NFSv4.1: Fix session initialisation races

Session initialisation is not complete until the lease manager has run. We need to ensure that both nfs4_init_session and nfs4_init_ds_session do so, and that they check for any resulting errors in clp->cl_cons_state. Only after this is done, can nfs4_ds_connect check the contents of clp->cl_exchange_flags. Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com> Cc: Andy Adamson <andros@netapp.com>

NFSv4.1: Fix session initialisation races
Session initialisation is not complete until the lease manager has run. We need to ensure that both nfs4_init_session and nfs4_init_ds_session do so, and that they check for any resulting errors in clp->cl_cons_state. Only after this is done, can nfs4_ds_connect check the contents of clp->cl_exchange_flags. Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com> Cc: Andy Adamson <andros@netapp.com>
Trond Myklebust
1 parent acdeb69d9c
Showing 4 changed files with 54 additions and 67 deletions Side-by-side Diff
fs/nfs/client.c
fs/nfs/internal.h
fs/nfs/nfs4filelayoutdev.c
fs/nfs/nfs4proc.c
@@ -592,22 +592,6 @@
 }
  
 /*
- * With sessions, the client is not marked ready until after a
- * successful EXCHANGE_ID and CREATE_SESSION.
- *
- * Map errors cl_cons_state errors to EPROTONOSUPPORT to indicate
- * other versions of NFS can be tried.
- */
-int nfs4_check_client_ready(struct nfs_client *clp)
-{
-	if (!nfs4_has_session(clp))
-		return 0;
-	if (clp->cl_cons_state < NFS_CS_READY)
-		return -EPROTONOSUPPORT;
-	return 0;
-}
-
-/*
  * Initialise the timeout values for a connection
  */
 static void nfs_init_timeout_values(struct rpc_timeout *to, int proto,
@@ -169,7 +169,6 @@
 					   struct nfs_fattr *,
 					   rpc_authflavor_t);
 extern void nfs_mark_client_ready(struct nfs_client *clp, int state);
-extern int nfs4_check_client_ready(struct nfs_client *clp);
 extern struct nfs_client *nfs4_set_ds_client(struct nfs_client* mds_clp,
 					     const struct sockaddr *ds_addr,
 					     int ds_addrlen, int ds_proto,
@@ -234,7 +233,7 @@
 extern struct rpc_procinfo nfs4_procedures[];
 #endif
  
-extern int nfs4_init_ds_session(struct nfs_client *clp);
+extern int nfs4_init_ds_session(struct nfs_client *, unsigned long);
  
 /* proc.c */
 void nfs_close_context(struct nfs_open_context *ctx, int is_sync);
@@ -203,28 +203,7 @@
 		goto out;
 	}
  
-	if ((clp->cl_exchange_flags & EXCHGID4_FLAG_MASK_PNFS) != 0) {
-		if (!is_ds_client(clp)) {
-			status = -ENODEV;
-			goto out_put;
-		}
-		ds->ds_clp = clp;
-		dprintk("%s [existing] server=%s\n", __func__,
-			ds->ds_remotestr);
-		goto out;
-	}
-
-	/*
-	 * Do not set NFS_CS_CHECK_LEASE_TIME instead set the DS lease to
-	 * be equal to the MDS lease. Renewal is scheduled in create_session.
-	 */
-	spin_lock(&mds_srv->nfs_client->cl_lock);
-	clp->cl_lease_time = mds_srv->nfs_client->cl_lease_time;
-	spin_unlock(&mds_srv->nfs_client->cl_lock);
-	clp->cl_last_renewal = jiffies;
-
-	/* New nfs_client */
-	status = nfs4_init_ds_session(clp);
+	status = nfs4_init_ds_session(clp, mds_srv->nfs_client->cl_lease_time);
 	if (status)
 		goto out_put;
  
@@ -5603,53 +5603,78 @@
 	return status;
 }
  
+/*
+ * With sessions, the client is not marked ready until after a
+ * successful EXCHANGE_ID and CREATE_SESSION.
+ *
+ * Map errors cl_cons_state errors to EPROTONOSUPPORT to indicate
+ * other versions of NFS can be tried.
+ */
+static int nfs41_check_session_ready(struct nfs_client *clp)
+{
+	int ret;
+	
+	if (clp->cl_cons_state == NFS_CS_SESSION_INITING) {
+		ret = nfs4_client_recover_expired_lease(clp);
+		if (ret)
+			return ret;
+	}
+	if (clp->cl_cons_state < NFS_CS_READY)
+		return -EPROTONOSUPPORT;
+	return 0;
+}
+
 int nfs4_init_session(struct nfs_server *server)
 {
 	struct nfs_client *clp = server->nfs_client;
 	struct nfs4_session *session;
 	unsigned int rsize, wsize;
-	int ret;
  
 	if (!nfs4_has_session(clp))
 		return 0;
  
 	session = clp->cl_session;
-	if (!test_and_clear_bit(NFS4_SESSION_INITING, &session->session_state))
-		return 0;
+	spin_lock(&clp->cl_lock);
+	if (test_and_clear_bit(NFS4_SESSION_INITING, &session->session_state)) {
  
-	rsize = server->rsize;
-	if (rsize == 0)
-		rsize = NFS_MAX_FILE_IO_SIZE;
-	wsize = server->wsize;
-	if (wsize == 0)
-		wsize = NFS_MAX_FILE_IO_SIZE;
+		rsize = server->rsize;
+		if (rsize == 0)
+			rsize = NFS_MAX_FILE_IO_SIZE;
+		wsize = server->wsize;
+		if (wsize == 0)
+			wsize = NFS_MAX_FILE_IO_SIZE;
  
-	session->fc_attrs.max_rqst_sz = wsize + nfs41_maxwrite_overhead;
-	session->fc_attrs.max_resp_sz = rsize + nfs41_maxread_overhead;
+		session->fc_attrs.max_rqst_sz = wsize + nfs41_maxwrite_overhead;
+		session->fc_attrs.max_resp_sz = rsize + nfs41_maxread_overhead;
+	}
+	spin_unlock(&clp->cl_lock);
  
-	ret = nfs4_recover_expired_lease(server);
-	if (!ret)
-		ret = nfs4_check_client_ready(clp);
-	return ret;
+	return nfs41_check_session_ready(clp);
 }
  
-int nfs4_init_ds_session(struct nfs_client *clp)
+int nfs4_init_ds_session(struct nfs_client *clp, unsigned long lease_time)
 {
 	struct nfs4_session *session = clp->cl_session;
 	int ret;
  
-	if (!test_and_clear_bit(NFS4_SESSION_INITING, &session->session_state))
-		return 0;
+	spin_lock(&clp->cl_lock);
+	if (test_and_clear_bit(NFS4_SESSION_INITING, &session->session_state)) {
+		/*
+		 * Do not set NFS_CS_CHECK_LEASE_TIME instead set the
+		 * DS lease to be equal to the MDS lease.
+		 */
+		clp->cl_lease_time = lease_time;
+		clp->cl_last_renewal = jiffies;
+	}
+	spin_unlock(&clp->cl_lock);
  
-	ret = nfs4_client_recover_expired_lease(clp);
-	if (!ret)
-		/* Test for the DS role */
-		if (!is_ds_client(clp))
-			ret = -ENODEV;
-	if (!ret)
-		ret = nfs4_check_client_ready(clp);
-	return ret;
-
+	ret = nfs41_check_session_ready(clp);
+	if (ret)
+		return ret;
+	/* Test for the DS role */
+	if (!is_ds_client(clp))
+		return -ENODEV;
+	return 0;
 }
 EXPORT_SYMBOL_GPL(nfs4_init_ds_session);
...	...	@@ -592,22 +592,6 @@
592	592	}
593	593
594	594	/*
595		- * With sessions, the client is not marked ready until after a
596		- * successful EXCHANGE_ID and CREATE_SESSION.
597		- *
598		- * Map errors cl_cons_state errors to EPROTONOSUPPORT to indicate
599		- * other versions of NFS can be tried.
600		- */
601		-int nfs4_check_client_ready(struct nfs_client *clp)
602		-{
603		- if (!nfs4_has_session(clp))
604		- return 0;
605		- if (clp->cl_cons_state < NFS_CS_READY)
606		- return -EPROTONOSUPPORT;
607		- return 0;
608		-}
609		-
610		-/*
611	595	* Initialise the timeout values for a connection
612	596	*/
613	597	static void nfs_init_timeout_values(struct rpc_timeout *to, int proto,
...	...	@@ -169,7 +169,6 @@
169	169	struct nfs_fattr *,
170	170	rpc_authflavor_t);
171	171	extern void nfs_mark_client_ready(struct nfs_client *clp, int state);
172		-extern int nfs4_check_client_ready(struct nfs_client *clp);
173	172	extern struct nfs_client nfs4_set_ds_client(struct nfs_client mds_clp,
174	173	const struct sockaddr *ds_addr,
175	174	int ds_addrlen, int ds_proto,
...	...	@@ -234,7 +233,7 @@
234	233	extern struct rpc_procinfo nfs4_procedures[];
235	234	#endif
236	235
237		-extern int nfs4_init_ds_session(struct nfs_client *clp);
	236	+extern int nfs4_init_ds_session(struct nfs_client *, unsigned long);
238	237
239	238	/* proc.c */
240	239	void nfs_close_context(struct nfs_open_context *ctx, int is_sync);
...	...	@@ -203,28 +203,7 @@
203	203	goto out;
204	204	}
205	205
206		- if ((clp->cl_exchange_flags & EXCHGID4_FLAG_MASK_PNFS) != 0) {
207		- if (!is_ds_client(clp)) {
208		- status = -ENODEV;
209		- goto out_put;
210		- }
211		- ds->ds_clp = clp;
212		- dprintk("%s [existing] server=%s\n", __func__,
213		- ds->ds_remotestr);
214		- goto out;
215		- }
216		-
217		- /*
218		- * Do not set NFS_CS_CHECK_LEASE_TIME instead set the DS lease to
219		- * be equal to the MDS lease. Renewal is scheduled in create_session.
220		- */
221		- spin_lock(&mds_srv->nfs_client->cl_lock);
222		- clp->cl_lease_time = mds_srv->nfs_client->cl_lease_time;
223		- spin_unlock(&mds_srv->nfs_client->cl_lock);
224		- clp->cl_last_renewal = jiffies;
225		-
226		- /* New nfs_client */
227		- status = nfs4_init_ds_session(clp);
	206	+ status = nfs4_init_ds_session(clp, mds_srv->nfs_client->cl_lease_time);
228	207	if (status)
229	208	goto out_put;
230	209
...	...	@@ -5603,53 +5603,78 @@
5603	5603	return status;
5604	5604	}
5605	5605
	5606	+/*
	5607	+ * With sessions, the client is not marked ready until after a
	5608	+ * successful EXCHANGE_ID and CREATE_SESSION.
	5609	+ *
	5610	+ * Map errors cl_cons_state errors to EPROTONOSUPPORT to indicate
	5611	+ * other versions of NFS can be tried.
	5612	+ */
	5613	+static int nfs41_check_session_ready(struct nfs_client *clp)
	5614	+{
	5615	+ int ret;
	5616	+
	5617	+ if (clp->cl_cons_state == NFS_CS_SESSION_INITING) {
	5618	+ ret = nfs4_client_recover_expired_lease(clp);
	5619	+ if (ret)
	5620	+ return ret;
	5621	+ }
	5622	+ if (clp->cl_cons_state < NFS_CS_READY)
	5623	+ return -EPROTONOSUPPORT;
	5624	+ return 0;
	5625	+}
	5626	+
5606	5627	int nfs4_init_session(struct nfs_server *server)
5607	5628	{
5608	5629	struct nfs_client *clp = server->nfs_client;
5609	5630	struct nfs4_session *session;
5610	5631	unsigned int rsize, wsize;
5611		- int ret;
5612	5632
5613	5633	if (!nfs4_has_session(clp))
5614	5634	return 0;
5615	5635
5616	5636	session = clp->cl_session;
5617		- if (!test_and_clear_bit(NFS4_SESSION_INITING, &session->session_state))
5618		- return 0;
	5637	+ spin_lock(&clp->cl_lock);
	5638	+ if (test_and_clear_bit(NFS4_SESSION_INITING, &session->session_state)) {
5619	5639
5620		- rsize = server->rsize;
5621		- if (rsize == 0)
5622		- rsize = NFS_MAX_FILE_IO_SIZE;
5623		- wsize = server->wsize;
5624		- if (wsize == 0)
5625		- wsize = NFS_MAX_FILE_IO_SIZE;
	5640	+ rsize = server->rsize;
	5641	+ if (rsize == 0)
	5642	+ rsize = NFS_MAX_FILE_IO_SIZE;
	5643	+ wsize = server->wsize;
	5644	+ if (wsize == 0)
	5645	+ wsize = NFS_MAX_FILE_IO_SIZE;
5626	5646
5627		- session->fc_attrs.max_rqst_sz = wsize + nfs41_maxwrite_overhead;
5628		- session->fc_attrs.max_resp_sz = rsize + nfs41_maxread_overhead;
	5647	+ session->fc_attrs.max_rqst_sz = wsize + nfs41_maxwrite_overhead;
	5648	+ session->fc_attrs.max_resp_sz = rsize + nfs41_maxread_overhead;
	5649	+ }
	5650	+ spin_unlock(&clp->cl_lock);
5629	5651
5630		- ret = nfs4_recover_expired_lease(server);
5631		- if (!ret)
5632		- ret = nfs4_check_client_ready(clp);
5633		- return ret;
	5652	+ return nfs41_check_session_ready(clp);
5634	5653	}
5635	5654
5636		-int nfs4_init_ds_session(struct nfs_client *clp)
	5655	+int nfs4_init_ds_session(struct nfs_client *clp, unsigned long lease_time)
5637	5656	{
5638	5657	struct nfs4_session *session = clp->cl_session;
5639	5658	int ret;
5640	5659
5641		- if (!test_and_clear_bit(NFS4_SESSION_INITING, &session->session_state))
5642		- return 0;
	5660	+ spin_lock(&clp->cl_lock);
	5661	+ if (test_and_clear_bit(NFS4_SESSION_INITING, &session->session_state)) {
	5662	+ /*
	5663	+ * Do not set NFS_CS_CHECK_LEASE_TIME instead set the
	5664	+ * DS lease to be equal to the MDS lease.
	5665	+ */
	5666	+ clp->cl_lease_time = lease_time;
	5667	+ clp->cl_last_renewal = jiffies;
	5668	+ }
	5669	+ spin_unlock(&clp->cl_lock);
5643	5670
5644		- ret = nfs4_client_recover_expired_lease(clp);
5645		- if (!ret)
5646		- /* Test for the DS role */
5647		- if (!is_ds_client(clp))
5648		- ret = -ENODEV;
5649		- if (!ret)
5650		- ret = nfs4_check_client_ready(clp);
5651		- return ret;
5652		-
	5671	+ ret = nfs41_check_session_ready(clp);
	5672	+ if (ret)
	5673	+ return ret;
	5674	+ /* Test for the DS role */
	5675	+ if (!is_ds_client(clp))
	5676	+ return -ENODEV;
	5677	+ return 0;
5653	5678	}
5654	5679	EXPORT_SYMBOL_GPL(nfs4_init_ds_session);
5655	5680