All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] NFSv4.1: Fix session initialisation races
@ 2012-05-23 17:32 Trond Myklebust
  2012-05-23 17:32 ` [PATCH 2/3] NFSv4: Fix a race in the net namespace mount notification Trond Myklebust
  2012-05-23 18:37 ` [PATCH 1/3] NFSv4.1: Fix session initialisation races Adamson, Andy
  0 siblings, 2 replies; 10+ messages in thread
From: Trond Myklebust @ 2012-05-23 17:32 UTC (permalink / raw)
  To: linux-nfs; +Cc: Andy Adamson

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>
---
 fs/nfs/client.c            |   16 ---------
 fs/nfs/internal.h          |    3 +-
 fs/nfs/nfs4filelayoutdev.c |   23 +------------
 fs/nfs/nfs4proc.c          |   77 ++++++++++++++++++++++++++++---------------
 4 files changed, 52 insertions(+), 67 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 60f7e4e..78970a1 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -593,22 +593,6 @@ void nfs_mark_client_ready(struct nfs_client *clp, int state)
 }
 
 /*
- * 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,
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index b777bda..00b66de 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -168,7 +168,6 @@ extern struct nfs_server *nfs_clone_server(struct nfs_server *,
 					   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);
@@ -237,7 +236,7 @@ extern const u32 nfs41_maxwrite_overhead;
 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);
diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c
index c9cff9a..0764c98 100644
--- a/fs/nfs/nfs4filelayoutdev.c
+++ b/fs/nfs/nfs4filelayoutdev.c
@@ -176,28 +176,7 @@ nfs4_ds_connect(struct nfs_server *mds_srv, struct nfs4_pnfs_ds *ds)
 		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;
 
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 99650aa..272c4ad 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5605,53 +5605,76 @@ int nfs4_proc_destroy_session(struct nfs4_session *session)
 	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;
+	
+	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;
-
-	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;
+	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 = 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);
 
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/3] NFSv4: Fix a race in the net namespace mount notification
  2012-05-23 17:32 [PATCH 1/3] NFSv4.1: Fix session initialisation races Trond Myklebust
@ 2012-05-23 17:32 ` Trond Myklebust
  2012-05-23 17:32   ` [PATCH 3/3] NFS: Add memory barriers to the nfs_client->cl_cons_state initialisation Trond Myklebust
  2012-05-23 18:58   ` [PATCH 2/3] NFSv4: Fix a race in the net namespace mount notification Stanislav Kinsbursky
  2012-05-23 18:37 ` [PATCH 1/3] NFSv4.1: Fix session initialisation races Adamson, Andy
  1 sibling, 2 replies; 10+ messages in thread
From: Trond Myklebust @ 2012-05-23 17:32 UTC (permalink / raw)
  To: linux-nfs; +Cc: Stanislav Kinsbursky

Since the struct nfs_client gets added to the global nfs_client_list
before it is initialised, it is possible that rpc_pipefs_event can
end up trying to create idmapper entries on such a thing.

The solution is to have the mount notification wait for the
initialisation of each nfs_client to complete, and then to
skip any entries for which the it failed.

Reported-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: Stanislav Kinsbursky <skinsbursky@parallels.com>
---
 fs/nfs/client.c   |   16 +++++++++++++---
 fs/nfs/idmap.c    |   15 +++++++++++++++
 fs/nfs/internal.h |    1 +
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 78970a1..e6070ea 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -504,6 +504,17 @@ static struct nfs_client *nfs_match_client(const struct nfs_client_initdata *dat
 	return NULL;
 }
 
+static bool nfs_client_init_is_complete(const struct nfs_client *clp)
+{
+	return clp->cl_cons_state != NFS_CS_INITING;
+}
+
+int nfs_wait_client_init_complete(const struct nfs_client *clp)
+{
+	return wait_event_killable(nfs_client_active_wq,
+			nfs_client_init_is_complete(clp));
+}
+
 /*
  * Look up a client by IP address and protocol version
  * - creates a new record if one doesn't yet exist
@@ -564,8 +575,7 @@ found_client:
 	if (new)
 		nfs_free_client(new);
 
-	error = wait_event_killable(nfs_client_active_wq,
-				clp->cl_cons_state < NFS_CS_INITING);
+	error = nfs_wait_client_init_complete(clp);
 	if (error < 0) {
 		nfs_put_client(clp);
 		return ERR_PTR(-ERESTARTSYS);
@@ -1317,7 +1327,7 @@ static int nfs4_init_client_minor_version(struct nfs_client *clp)
 		 * so that the client back channel can find the
 		 * nfs_client struct
 		 */
-		clp->cl_cons_state = NFS_CS_SESSION_INITING;
+		nfs_mark_client_ready(clp, NFS_CS_SESSION_INITING);
 	}
 #endif /* CONFIG_NFS_V4_1 */
 
diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
index 3e8edbe..6ca949b 100644
--- a/fs/nfs/idmap.c
+++ b/fs/nfs/idmap.c
@@ -530,9 +530,24 @@ static struct nfs_client *nfs_get_client_for_event(struct net *net, int event)
 	struct nfs_net *nn = net_generic(net, nfs_net_id);
 	struct dentry *cl_dentry;
 	struct nfs_client *clp;
+	int err;
 
+restart:
 	spin_lock(&nn->nfs_client_lock);
 	list_for_each_entry(clp, &nn->nfs_client_list, cl_share_link) {
+		/* Wait for initialisation to finish */
+		if (clp->cl_cons_state == NFS_CS_INITING) {
+			atomic_inc(&clp->cl_count);
+			spin_unlock(&nn->nfs_client_lock);
+			err = nfs_wait_client_init_complete(clp);
+			nfs_put_client(clp);
+			if (err)
+				return NULL;
+			goto restart;
+		}
+		/* Skip nfs_clients that failed to initialise */
+		if (clp->cl_cons_state < 0)
+			continue;
 		if (clp->rpc_ops != &nfs_v4_clientops)
 			continue;
 		cl_dentry = clp->cl_idmap->idmap_pipe->dentry;
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 00b66de..a23daa9 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -167,6 +167,7 @@ extern struct nfs_server *nfs_clone_server(struct nfs_server *,
 					   struct nfs_fh *,
 					   struct nfs_fattr *,
 					   rpc_authflavor_t);
+extern int nfs_wait_client_init_complete(const struct nfs_client *clp);
 extern void nfs_mark_client_ready(struct nfs_client *clp, int state);
 extern struct nfs_client *nfs4_set_ds_client(struct nfs_client* mds_clp,
 					     const struct sockaddr *ds_addr,
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/3] NFS: Add memory barriers to the nfs_client->cl_cons_state initialisation
  2012-05-23 17:32 ` [PATCH 2/3] NFSv4: Fix a race in the net namespace mount notification Trond Myklebust
@ 2012-05-23 17:32   ` Trond Myklebust
  2012-05-24  4:36     ` Stanislav Kinsbursky
  2012-05-23 18:58   ` [PATCH 2/3] NFSv4: Fix a race in the net namespace mount notification Stanislav Kinsbursky
  1 sibling, 1 reply; 10+ messages in thread
From: Trond Myklebust @ 2012-05-23 17:32 UTC (permalink / raw)
  To: linux-nfs

Ensure that a process that uses the nfs_client->cl_cons_state test
for whether the initialisation process is finished does not read
stale data.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/client.c |    5 +++++
 fs/nfs/idmap.c  |    1 +
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index e6070ea..4be85f9 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -456,6 +456,8 @@ static bool nfs4_cb_match_client(const struct sockaddr *addr,
 	    clp->cl_cons_state == NFS_CS_SESSION_INITING))
 		return false;
 
+	smp_rmb();
+
 	/* Match the version and minorversion */
 	if (clp->rpc_ops->version != 4 ||
 	    clp->cl_minorversion != minorversion)
@@ -587,6 +589,8 @@ found_client:
 		return ERR_PTR(error);
 	}
 
+	smp_rmb();
+
 	BUG_ON(clp->cl_cons_state != NFS_CS_READY);
 
 	dprintk("--> nfs_get_client() = %p [share]\n", clp);
@@ -598,6 +602,7 @@ found_client:
  */
 void nfs_mark_client_ready(struct nfs_client *clp, int state)
 {
+	smp_wmb();
 	clp->cl_cons_state = state;
 	wake_up_all(&nfs_client_active_wq);
 }
diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
index 6ca949b..7d4e8dd 100644
--- a/fs/nfs/idmap.c
+++ b/fs/nfs/idmap.c
@@ -548,6 +548,7 @@ restart:
 		/* Skip nfs_clients that failed to initialise */
 		if (clp->cl_cons_state < 0)
 			continue;
+		smp_rmb();
 		if (clp->rpc_ops != &nfs_v4_clientops)
 			continue;
 		cl_dentry = clp->cl_idmap->idmap_pipe->dentry;
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] NFSv4.1: Fix session initialisation races
  2012-05-23 17:32 [PATCH 1/3] NFSv4.1: Fix session initialisation races Trond Myklebust
  2012-05-23 17:32 ` [PATCH 2/3] NFSv4: Fix a race in the net namespace mount notification Trond Myklebust
@ 2012-05-23 18:37 ` Adamson, Andy
  2012-05-23 19:14   ` Myklebust, Trond
  1 sibling, 1 reply; 10+ messages in thread
From: Adamson, Andy @ 2012-05-23 18:37 UTC (permalink / raw)
  To: Myklebust, Trond; +Cc: <linux-nfs@vger.kernel.org>



On May 23, 2012, at 1:32 PM, Trond Myklebust wrote:

> 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>
> ---
> fs/nfs/client.c            |   16 ---------
> fs/nfs/internal.h          |    3 +-
> fs/nfs/nfs4filelayoutdev.c |   23 +------------
> fs/nfs/nfs4proc.c          |   77 ++++++++++++++++++++++++++++---------------
> 4 files changed, 52 insertions(+), 67 deletions(-)
> 
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 60f7e4e..78970a1 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -593,22 +593,6 @@ void nfs_mark_client_ready(struct nfs_client *clp, int state)
> }
> 
> /*
> - * 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,
> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> index b777bda..00b66de 100644
> --- a/fs/nfs/internal.h
> +++ b/fs/nfs/internal.h
> @@ -168,7 +168,6 @@ extern struct nfs_server *nfs_clone_server(struct nfs_server *,
> 					   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);
> @@ -237,7 +236,7 @@ extern const u32 nfs41_maxwrite_overhead;
> 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);
> diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c
> index c9cff9a..0764c98 100644
> --- a/fs/nfs/nfs4filelayoutdev.c
> +++ b/fs/nfs/nfs4filelayoutdev.c
> @@ -176,28 +176,7 @@ nfs4_ds_connect(struct nfs_server *mds_srv, struct nfs4_pnfs_ds *ds)
> 		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;
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 99650aa..272c4ad 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -5605,53 +5605,76 @@ int nfs4_proc_destroy_session(struct nfs4_session *session)
> 	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;
> +	
> +	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;
> -
> -	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;
> +	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 = nfs41_check_session_ready(clp);

If I read the code correctly, on an existing client,  this call needlessly waits for the state manager to finish whatever it is doing. As the state manager could be operating on a different file system than the DS belongs to,  and recovering a bunch of state,  it might be worth while to avoid.  That was the intention of checking the cl_exchange_flags in nfs4_ds_connect.
In fact, on an existing client, nfs4_init_ds_session does not need to be called.

-->Andy

> +	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);
> 
> -- 
> 1.7.7.6
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] NFSv4: Fix a race in the net namespace mount notification
  2012-05-23 17:32 ` [PATCH 2/3] NFSv4: Fix a race in the net namespace mount notification Trond Myklebust
  2012-05-23 17:32   ` [PATCH 3/3] NFS: Add memory barriers to the nfs_client->cl_cons_state initialisation Trond Myklebust
@ 2012-05-23 18:58   ` Stanislav Kinsbursky
  1 sibling, 0 replies; 10+ messages in thread
From: Stanislav Kinsbursky @ 2012-05-23 18:58 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: linux-nfs

Looks good to me.

Acked-by: Stanislav Kinsbursky <skinsbursky@parallels.com>

23.05.2012 21:32, Trond Myklebust написал:
> Since the struct nfs_client gets added to the global nfs_client_list
> before it is initialised, it is possible that rpc_pipefs_event can
> end up trying to create idmapper entries on such a thing.
>
> The solution is to have the mount notification wait for the
> initialisation of each nfs_client to complete, and then to
> skip any entries for which the it failed.
>
> Reported-by: Stanislav Kinsbursky<skinsbursky@parallels.com>
> Signed-off-by: Trond Myklebust<Trond.Myklebust@netapp.com>
> Cc: Stanislav Kinsbursky<skinsbursky@parallels.com>
> ---
>   fs/nfs/client.c   |   16 +++++++++++++---
>   fs/nfs/idmap.c    |   15 +++++++++++++++
>   fs/nfs/internal.h |    1 +
>   3 files changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 78970a1..e6070ea 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -504,6 +504,17 @@ static struct nfs_client *nfs_match_client(const struct nfs_client_initdata *dat
>   	return NULL;
>   }
>
> +static bool nfs_client_init_is_complete(const struct nfs_client *clp)
> +{
> +	return clp->cl_cons_state != NFS_CS_INITING;
> +}
> +
> +int nfs_wait_client_init_complete(const struct nfs_client *clp)
> +{
> +	return wait_event_killable(nfs_client_active_wq,
> +			nfs_client_init_is_complete(clp));
> +}
> +
>   /*
>    * Look up a client by IP address and protocol version
>    * - creates a new record if one doesn't yet exist
> @@ -564,8 +575,7 @@ found_client:
>   	if (new)
>   		nfs_free_client(new);
>
> -	error = wait_event_killable(nfs_client_active_wq,
> -				clp->cl_cons_state<  NFS_CS_INITING);
> +	error = nfs_wait_client_init_complete(clp);
>   	if (error<  0) {
>   		nfs_put_client(clp);
>   		return ERR_PTR(-ERESTARTSYS);
> @@ -1317,7 +1327,7 @@ static int nfs4_init_client_minor_version(struct nfs_client *clp)
>   		 * so that the client back channel can find the
>   		 * nfs_client struct
>   		 */
> -		clp->cl_cons_state = NFS_CS_SESSION_INITING;
> +		nfs_mark_client_ready(clp, NFS_CS_SESSION_INITING);
>   	}
>   #endif /* CONFIG_NFS_V4_1 */
>
> diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
> index 3e8edbe..6ca949b 100644
> --- a/fs/nfs/idmap.c
> +++ b/fs/nfs/idmap.c
> @@ -530,9 +530,24 @@ static struct nfs_client *nfs_get_client_for_event(struct net *net, int event)
>   	struct nfs_net *nn = net_generic(net, nfs_net_id);
>   	struct dentry *cl_dentry;
>   	struct nfs_client *clp;
> +	int err;
>
> +restart:
>   	spin_lock(&nn->nfs_client_lock);
>   	list_for_each_entry(clp,&nn->nfs_client_list, cl_share_link) {
> +		/* Wait for initialisation to finish */
> +		if (clp->cl_cons_state == NFS_CS_INITING) {
> +			atomic_inc(&clp->cl_count);
> +			spin_unlock(&nn->nfs_client_lock);
> +			err = nfs_wait_client_init_complete(clp);
> +			nfs_put_client(clp);
> +			if (err)
> +				return NULL;
> +			goto restart;
> +		}
> +		/* Skip nfs_clients that failed to initialise */
> +		if (clp->cl_cons_state<  0)
> +			continue;
>   		if (clp->rpc_ops !=&nfs_v4_clientops)
>   			continue;
>   		cl_dentry = clp->cl_idmap->idmap_pipe->dentry;
> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> index 00b66de..a23daa9 100644
> --- a/fs/nfs/internal.h
> +++ b/fs/nfs/internal.h
> @@ -167,6 +167,7 @@ extern struct nfs_server *nfs_clone_server(struct nfs_server *,
>   					   struct nfs_fh *,
>   					   struct nfs_fattr *,
>   					   rpc_authflavor_t);
> +extern int nfs_wait_client_init_complete(const struct nfs_client *clp);
>   extern void nfs_mark_client_ready(struct nfs_client *clp, int state);
>   extern struct nfs_client *nfs4_set_ds_client(struct nfs_client* mds_clp,
>   					     const struct sockaddr *ds_addr,


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] NFSv4.1: Fix session initialisation races
  2012-05-23 18:37 ` [PATCH 1/3] NFSv4.1: Fix session initialisation races Adamson, Andy
@ 2012-05-23 19:14   ` Myklebust, Trond
  2012-05-23 19:20     ` Myklebust, Trond
  0 siblings, 1 reply; 10+ messages in thread
From: Myklebust, Trond @ 2012-05-23 19:14 UTC (permalink / raw)
  To: Adamson, Andy; +Cc: <linux-nfs@vger.kernel.org>

T24gV2VkLCAyMDEyLTA1LTIzIGF0IDE4OjM3ICswMDAwLCBBZGFtc29uLCBBbmR5IHdyb3RlOg0K
PiANCj4gT24gTWF5IDIzLCAyMDEyLCBhdCAxOjMyIFBNLCBUcm9uZCBNeWtsZWJ1c3Qgd3JvdGU6
DQo+IA0KPiA+IFNlc3Npb24gaW5pdGlhbGlzYXRpb24gaXMgbm90IGNvbXBsZXRlIHVudGlsIHRo
ZSBsZWFzZSBtYW5hZ2VyDQo+ID4gaGFzIHJ1bi4gV2UgbmVlZCB0byBlbnN1cmUgdGhhdCBib3Ro
IG5mczRfaW5pdF9zZXNzaW9uIGFuZA0KPiA+IG5mczRfaW5pdF9kc19zZXNzaW9uIGRvIHNvLCBh
bmQgdGhhdCB0aGV5IGNoZWNrIGZvciBhbnkgcmVzdWx0aW5nDQo+ID4gZXJyb3JzIGluIGNscC0+
Y2xfY29uc19zdGF0ZS4NCj4gPiANCj4gPiBPbmx5IGFmdGVyIHRoaXMgaXMgZG9uZSwgY2FuIG5m
czRfZHNfY29ubmVjdCBjaGVjayB0aGUgY29udGVudHMNCj4gPiBvZiBjbHAtPmNsX2V4Y2hhbmdl
X2ZsYWdzLg0KPiA+IA0KPiA+IFNpZ25lZC1vZmYtYnk6IFRyb25kIE15a2xlYnVzdCA8VHJvbmQu
TXlrbGVidXN0QG5ldGFwcC5jb20+DQo+ID4gQ2M6IEFuZHkgQWRhbXNvbiA8YW5kcm9zQG5ldGFw
cC5jb20+DQo+ID4gLS0tDQo+ID4gZnMvbmZzL2NsaWVudC5jICAgICAgICAgICAgfCAgIDE2IC0t
LS0tLS0tLQ0KPiA+IGZzL25mcy9pbnRlcm5hbC5oICAgICAgICAgIHwgICAgMyArLQ0KPiA+IGZz
L25mcy9uZnM0ZmlsZWxheW91dGRldi5jIHwgICAyMyArLS0tLS0tLS0tLS0tDQo+ID4gZnMvbmZz
L25mczRwcm9jLmMgICAgICAgICAgfCAgIDc3ICsrKysrKysrKysrKysrKysrKysrKysrKysrKyst
LS0tLS0tLS0tLS0tLS0NCj4gPiA0IGZpbGVzIGNoYW5nZWQsIDUyIGluc2VydGlvbnMoKyksIDY3
IGRlbGV0aW9ucygtKQ0KPiA+IA0KPiA+IGRpZmYgLS1naXQgYS9mcy9uZnMvY2xpZW50LmMgYi9m
cy9uZnMvY2xpZW50LmMNCj4gPiBpbmRleCA2MGY3ZTRlLi43ODk3MGExIDEwMDY0NA0KPiA+IC0t
LSBhL2ZzL25mcy9jbGllbnQuYw0KPiA+ICsrKyBiL2ZzL25mcy9jbGllbnQuYw0KPiA+IEBAIC01
OTMsMjIgKzU5Myw2IEBAIHZvaWQgbmZzX21hcmtfY2xpZW50X3JlYWR5KHN0cnVjdCBuZnNfY2xp
ZW50ICpjbHAsIGludCBzdGF0ZSkNCj4gPiB9DQo+ID4gDQo+ID4gLyoNCj4gPiAtICogV2l0aCBz
ZXNzaW9ucywgdGhlIGNsaWVudCBpcyBub3QgbWFya2VkIHJlYWR5IHVudGlsIGFmdGVyIGENCj4g
PiAtICogc3VjY2Vzc2Z1bCBFWENIQU5HRV9JRCBhbmQgQ1JFQVRFX1NFU1NJT04uDQo+ID4gLSAq
DQo+ID4gLSAqIE1hcCBlcnJvcnMgY2xfY29uc19zdGF0ZSBlcnJvcnMgdG8gRVBST1RPTk9TVVBQ
T1JUIHRvIGluZGljYXRlDQo+ID4gLSAqIG90aGVyIHZlcnNpb25zIG9mIE5GUyBjYW4gYmUgdHJp
ZWQuDQo+ID4gLSAqLw0KPiA+IC1pbnQgbmZzNF9jaGVja19jbGllbnRfcmVhZHkoc3RydWN0IG5m
c19jbGllbnQgKmNscCkNCj4gPiAtew0KPiA+IC0JaWYgKCFuZnM0X2hhc19zZXNzaW9uKGNscCkp
DQo+ID4gLQkJcmV0dXJuIDA7DQo+ID4gLQlpZiAoY2xwLT5jbF9jb25zX3N0YXRlIDwgTkZTX0NT
X1JFQURZKQ0KPiA+IC0JCXJldHVybiAtRVBST1RPTk9TVVBQT1JUOw0KPiA+IC0JcmV0dXJuIDA7
DQo+ID4gLX0NCj4gPiAtDQo+ID4gLS8qDQo+ID4gICogSW5pdGlhbGlzZSB0aGUgdGltZW91dCB2
YWx1ZXMgZm9yIGEgY29ubmVjdGlvbg0KPiA+ICAqLw0KPiA+IHN0YXRpYyB2b2lkIG5mc19pbml0
X3RpbWVvdXRfdmFsdWVzKHN0cnVjdCBycGNfdGltZW91dCAqdG8sIGludCBwcm90bywNCj4gPiBk
aWZmIC0tZ2l0IGEvZnMvbmZzL2ludGVybmFsLmggYi9mcy9uZnMvaW50ZXJuYWwuaA0KPiA+IGlu
ZGV4IGI3NzdiZGEuLjAwYjY2ZGUgMTAwNjQ0DQo+ID4gLS0tIGEvZnMvbmZzL2ludGVybmFsLmgN
Cj4gPiArKysgYi9mcy9uZnMvaW50ZXJuYWwuaA0KPiA+IEBAIC0xNjgsNyArMTY4LDYgQEAgZXh0
ZXJuIHN0cnVjdCBuZnNfc2VydmVyICpuZnNfY2xvbmVfc2VydmVyKHN0cnVjdCBuZnNfc2VydmVy
ICosDQo+ID4gCQkJCQkgICBzdHJ1Y3QgbmZzX2ZhdHRyICosDQo+ID4gCQkJCQkgICBycGNfYXV0
aGZsYXZvcl90KTsNCj4gPiBleHRlcm4gdm9pZCBuZnNfbWFya19jbGllbnRfcmVhZHkoc3RydWN0
IG5mc19jbGllbnQgKmNscCwgaW50IHN0YXRlKTsNCj4gPiAtZXh0ZXJuIGludCBuZnM0X2NoZWNr
X2NsaWVudF9yZWFkeShzdHJ1Y3QgbmZzX2NsaWVudCAqY2xwKTsNCj4gPiBleHRlcm4gc3RydWN0
IG5mc19jbGllbnQgKm5mczRfc2V0X2RzX2NsaWVudChzdHJ1Y3QgbmZzX2NsaWVudCogbWRzX2Ns
cCwNCj4gPiAJCQkJCSAgICAgY29uc3Qgc3RydWN0IHNvY2thZGRyICpkc19hZGRyLA0KPiA+IAkJ
CQkJICAgICBpbnQgZHNfYWRkcmxlbiwgaW50IGRzX3Byb3RvKTsNCj4gPiBAQCAtMjM3LDcgKzIz
Niw3IEBAIGV4dGVybiBjb25zdCB1MzIgbmZzNDFfbWF4d3JpdGVfb3ZlcmhlYWQ7DQo+ID4gZXh0
ZXJuIHN0cnVjdCBycGNfcHJvY2luZm8gbmZzNF9wcm9jZWR1cmVzW107DQo+ID4gI2VuZGlmDQo+
ID4gDQo+ID4gLWV4dGVybiBpbnQgbmZzNF9pbml0X2RzX3Nlc3Npb24oc3RydWN0IG5mc19jbGll
bnQgKmNscCk7DQo+ID4gK2V4dGVybiBpbnQgbmZzNF9pbml0X2RzX3Nlc3Npb24oc3RydWN0IG5m
c19jbGllbnQgKiwgdW5zaWduZWQgbG9uZyk7DQo+ID4gDQo+ID4gLyogcHJvYy5jICovDQo+ID4g
dm9pZCBuZnNfY2xvc2VfY29udGV4dChzdHJ1Y3QgbmZzX29wZW5fY29udGV4dCAqY3R4LCBpbnQg
aXNfc3luYyk7DQo+ID4gZGlmZiAtLWdpdCBhL2ZzL25mcy9uZnM0ZmlsZWxheW91dGRldi5jIGIv
ZnMvbmZzL25mczRmaWxlbGF5b3V0ZGV2LmMNCj4gPiBpbmRleCBjOWNmZjlhLi4wNzY0Yzk4IDEw
MDY0NA0KPiA+IC0tLSBhL2ZzL25mcy9uZnM0ZmlsZWxheW91dGRldi5jDQo+ID4gKysrIGIvZnMv
bmZzL25mczRmaWxlbGF5b3V0ZGV2LmMNCj4gPiBAQCAtMTc2LDI4ICsxNzYsNyBAQCBuZnM0X2Rz
X2Nvbm5lY3Qoc3RydWN0IG5mc19zZXJ2ZXIgKm1kc19zcnYsIHN0cnVjdCBuZnM0X3BuZnNfZHMg
KmRzKQ0KPiA+IAkJZ290byBvdXQ7DQo+ID4gCX0NCj4gPiANCj4gPiAtCWlmICgoY2xwLT5jbF9l
eGNoYW5nZV9mbGFncyAmIEVYQ0hHSUQ0X0ZMQUdfTUFTS19QTkZTKSAhPSAwKSB7DQo+ID4gLQkJ
aWYgKCFpc19kc19jbGllbnQoY2xwKSkgew0KPiA+IC0JCQlzdGF0dXMgPSAtRU5PREVWOw0KPiA+
IC0JCQlnb3RvIG91dF9wdXQ7DQo+ID4gLQkJfQ0KPiA+IC0JCWRzLT5kc19jbHAgPSBjbHA7DQo+
ID4gLQkJZHByaW50aygiJXMgW2V4aXN0aW5nXSBzZXJ2ZXI9JXNcbiIsIF9fZnVuY19fLA0KPiA+
IC0JCQlkcy0+ZHNfcmVtb3Rlc3RyKTsNCj4gPiAtCQlnb3RvIG91dDsNCj4gPiAtCX0NCj4gPiAt
DQo+ID4gLQkvKg0KPiA+IC0JICogRG8gbm90IHNldCBORlNfQ1NfQ0hFQ0tfTEVBU0VfVElNRSBp
bnN0ZWFkIHNldCB0aGUgRFMgbGVhc2UgdG8NCj4gPiAtCSAqIGJlIGVxdWFsIHRvIHRoZSBNRFMg
bGVhc2UuIFJlbmV3YWwgaXMgc2NoZWR1bGVkIGluIGNyZWF0ZV9zZXNzaW9uLg0KPiA+IC0JICov
DQo+ID4gLQlzcGluX2xvY2soJm1kc19zcnYtPm5mc19jbGllbnQtPmNsX2xvY2spOw0KPiA+IC0J
Y2xwLT5jbF9sZWFzZV90aW1lID0gbWRzX3Nydi0+bmZzX2NsaWVudC0+Y2xfbGVhc2VfdGltZTsN
Cj4gPiAtCXNwaW5fdW5sb2NrKCZtZHNfc3J2LT5uZnNfY2xpZW50LT5jbF9sb2NrKTsNCj4gPiAt
CWNscC0+Y2xfbGFzdF9yZW5ld2FsID0gamlmZmllczsNCj4gPiAtDQo+ID4gLQkvKiBOZXcgbmZz
X2NsaWVudCAqLw0KPiA+IC0Jc3RhdHVzID0gbmZzNF9pbml0X2RzX3Nlc3Npb24oY2xwKTsNCj4g
PiArCXN0YXR1cyA9IG5mczRfaW5pdF9kc19zZXNzaW9uKGNscCwgbWRzX3Nydi0+bmZzX2NsaWVu
dC0+Y2xfbGVhc2VfdGltZSk7DQo+ID4gCWlmIChzdGF0dXMpDQo+ID4gCQlnb3RvIG91dF9wdXQ7
DQo+ID4gDQo+ID4gZGlmZiAtLWdpdCBhL2ZzL25mcy9uZnM0cHJvYy5jIGIvZnMvbmZzL25mczRw
cm9jLmMNCj4gPiBpbmRleCA5OTY1MGFhLi4yNzJjNGFkIDEwMDY0NA0KPiA+IC0tLSBhL2ZzL25m
cy9uZnM0cHJvYy5jDQo+ID4gKysrIGIvZnMvbmZzL25mczRwcm9jLmMNCj4gPiBAQCAtNTYwNSw1
MyArNTYwNSw3NiBAQCBpbnQgbmZzNF9wcm9jX2Rlc3Ryb3lfc2Vzc2lvbihzdHJ1Y3QgbmZzNF9z
ZXNzaW9uICpzZXNzaW9uKQ0KPiA+IAlyZXR1cm4gc3RhdHVzOw0KPiA+IH0NCj4gPiANCj4gPiAr
LyoNCj4gPiArICogV2l0aCBzZXNzaW9ucywgdGhlIGNsaWVudCBpcyBub3QgbWFya2VkIHJlYWR5
IHVudGlsIGFmdGVyIGENCj4gPiArICogc3VjY2Vzc2Z1bCBFWENIQU5HRV9JRCBhbmQgQ1JFQVRF
X1NFU1NJT04uDQo+ID4gKyAqDQo+ID4gKyAqIE1hcCBlcnJvcnMgY2xfY29uc19zdGF0ZSBlcnJv
cnMgdG8gRVBST1RPTk9TVVBQT1JUIHRvIGluZGljYXRlDQo+ID4gKyAqIG90aGVyIHZlcnNpb25z
IG9mIE5GUyBjYW4gYmUgdHJpZWQuDQo+ID4gKyAqLw0KPiA+ICtzdGF0aWMgaW50IG5mczQxX2No
ZWNrX3Nlc3Npb25fcmVhZHkoc3RydWN0IG5mc19jbGllbnQgKmNscCkNCj4gPiArew0KPiA+ICsJ
aW50IHJldDsNCj4gPiArCQ0KPiA+ICsJcmV0ID0gbmZzNF9jbGllbnRfcmVjb3Zlcl9leHBpcmVk
X2xlYXNlKGNscCk7DQo+ID4gKwlpZiAocmV0KQ0KPiA+ICsJCXJldHVybiByZXQ7DQo+ID4gKwlp
ZiAoY2xwLT5jbF9jb25zX3N0YXRlIDwgTkZTX0NTX1JFQURZKQ0KPiA+ICsJCXJldHVybiAtRVBS
T1RPTk9TVVBQT1JUOw0KPiA+ICsJcmV0dXJuIDA7DQo+ID4gK30NCj4gPiArDQo+ID4gaW50IG5m
czRfaW5pdF9zZXNzaW9uKHN0cnVjdCBuZnNfc2VydmVyICpzZXJ2ZXIpDQo+ID4gew0KPiA+IAlz
dHJ1Y3QgbmZzX2NsaWVudCAqY2xwID0gc2VydmVyLT5uZnNfY2xpZW50Ow0KPiA+IAlzdHJ1Y3Qg
bmZzNF9zZXNzaW9uICpzZXNzaW9uOw0KPiA+IAl1bnNpZ25lZCBpbnQgcnNpemUsIHdzaXplOw0K
PiA+IC0JaW50IHJldDsNCj4gPiANCj4gPiAJaWYgKCFuZnM0X2hhc19zZXNzaW9uKGNscCkpDQo+
ID4gCQlyZXR1cm4gMDsNCj4gPiANCj4gPiAJc2Vzc2lvbiA9IGNscC0+Y2xfc2Vzc2lvbjsNCj4g
PiAtCWlmICghdGVzdF9hbmRfY2xlYXJfYml0KE5GUzRfU0VTU0lPTl9JTklUSU5HLCAmc2Vzc2lv
bi0+c2Vzc2lvbl9zdGF0ZSkpDQo+ID4gLQkJcmV0dXJuIDA7DQo+ID4gKwlzcGluX2xvY2soJmNs
cC0+Y2xfbG9jayk7DQo+ID4gKwlpZiAodGVzdF9hbmRfY2xlYXJfYml0KE5GUzRfU0VTU0lPTl9J
TklUSU5HLCAmc2Vzc2lvbi0+c2Vzc2lvbl9zdGF0ZSkpIHsNCj4gPiANCj4gPiAtCXJzaXplID0g
c2VydmVyLT5yc2l6ZTsNCj4gPiAtCWlmIChyc2l6ZSA9PSAwKQ0KPiA+IC0JCXJzaXplID0gTkZT
X01BWF9GSUxFX0lPX1NJWkU7DQo+ID4gLQl3c2l6ZSA9IHNlcnZlci0+d3NpemU7DQo+ID4gLQlp
ZiAod3NpemUgPT0gMCkNCj4gPiAtCQl3c2l6ZSA9IE5GU19NQVhfRklMRV9JT19TSVpFOw0KPiA+
ICsJCXJzaXplID0gc2VydmVyLT5yc2l6ZTsNCj4gPiArCQlpZiAocnNpemUgPT0gMCkNCj4gPiAr
CQkJcnNpemUgPSBORlNfTUFYX0ZJTEVfSU9fU0laRTsNCj4gPiArCQl3c2l6ZSA9IHNlcnZlci0+
d3NpemU7DQo+ID4gKwkJaWYgKHdzaXplID09IDApDQo+ID4gKwkJCXdzaXplID0gTkZTX01BWF9G
SUxFX0lPX1NJWkU7DQo+ID4gDQo+ID4gLQlzZXNzaW9uLT5mY19hdHRycy5tYXhfcnFzdF9zeiA9
IHdzaXplICsgbmZzNDFfbWF4d3JpdGVfb3ZlcmhlYWQ7DQo+ID4gLQlzZXNzaW9uLT5mY19hdHRy
cy5tYXhfcmVzcF9zeiA9IHJzaXplICsgbmZzNDFfbWF4cmVhZF9vdmVyaGVhZDsNCj4gPiArCQlz
ZXNzaW9uLT5mY19hdHRycy5tYXhfcnFzdF9zeiA9IHdzaXplICsgbmZzNDFfbWF4d3JpdGVfb3Zl
cmhlYWQ7DQo+ID4gKwkJc2Vzc2lvbi0+ZmNfYXR0cnMubWF4X3Jlc3Bfc3ogPSByc2l6ZSArIG5m
czQxX21heHJlYWRfb3ZlcmhlYWQ7DQo+ID4gKwl9DQo+ID4gKwlzcGluX3VubG9jaygmY2xwLT5j
bF9sb2NrKTsNCj4gPiANCj4gPiAtCXJldCA9IG5mczRfcmVjb3Zlcl9leHBpcmVkX2xlYXNlKHNl
cnZlcik7DQo+ID4gLQlpZiAoIXJldCkNCj4gPiAtCQlyZXQgPSBuZnM0X2NoZWNrX2NsaWVudF9y
ZWFkeShjbHApOw0KPiA+IC0JcmV0dXJuIHJldDsNCj4gPiArCXJldHVybiBuZnM0MV9jaGVja19z
ZXNzaW9uX3JlYWR5KGNscCk7DQo+ID4gfQ0KPiA+IA0KPiA+IC1pbnQgbmZzNF9pbml0X2RzX3Nl
c3Npb24oc3RydWN0IG5mc19jbGllbnQgKmNscCkNCj4gPiAraW50IG5mczRfaW5pdF9kc19zZXNz
aW9uKHN0cnVjdCBuZnNfY2xpZW50ICpjbHAsIHVuc2lnbmVkIGxvbmcgbGVhc2VfdGltZSkNCj4g
PiB7DQo+ID4gCXN0cnVjdCBuZnM0X3Nlc3Npb24gKnNlc3Npb24gPSBjbHAtPmNsX3Nlc3Npb247
DQo+ID4gCWludCByZXQ7DQo+ID4gDQo+ID4gLQlpZiAoIXRlc3RfYW5kX2NsZWFyX2JpdChORlM0
X1NFU1NJT05fSU5JVElORywgJnNlc3Npb24tPnNlc3Npb25fc3RhdGUpKQ0KPiA+IC0JCXJldHVy
biAwOw0KPiA+IC0NCj4gPiAtCXJldCA9IG5mczRfY2xpZW50X3JlY292ZXJfZXhwaXJlZF9sZWFz
ZShjbHApOw0KPiA+IC0JaWYgKCFyZXQpDQo+ID4gLQkJLyogVGVzdCBmb3IgdGhlIERTIHJvbGUg
Ki8NCj4gPiAtCQlpZiAoIWlzX2RzX2NsaWVudChjbHApKQ0KPiA+IC0JCQlyZXQgPSAtRU5PREVW
Ow0KPiA+IC0JaWYgKCFyZXQpDQo+ID4gLQkJcmV0ID0gbmZzNF9jaGVja19jbGllbnRfcmVhZHko
Y2xwKTsNCj4gPiAtCXJldHVybiByZXQ7DQo+ID4gKwlzcGluX2xvY2soJmNscC0+Y2xfbG9jayk7
DQo+ID4gKwlpZiAodGVzdF9hbmRfY2xlYXJfYml0KE5GUzRfU0VTU0lPTl9JTklUSU5HLCAmc2Vz
c2lvbi0+c2Vzc2lvbl9zdGF0ZSkpIHsNCj4gPiArCQkvKg0KPiA+ICsJCSAqIERvIG5vdCBzZXQg
TkZTX0NTX0NIRUNLX0xFQVNFX1RJTUUgaW5zdGVhZCBzZXQgdGhlDQo+ID4gKwkJICogRFMgbGVh
c2UgdG8gYmUgZXF1YWwgdG8gdGhlIE1EUyBsZWFzZS4NCj4gPiArCQkgKi8NCj4gPiArCQljbHAt
PmNsX2xlYXNlX3RpbWUgPSBsZWFzZV90aW1lOw0KPiA+ICsJCWNscC0+Y2xfbGFzdF9yZW5ld2Fs
ID0gamlmZmllczsNCj4gPiArCX0NCj4gPiArCXNwaW5fdW5sb2NrKCZjbHAtPmNsX2xvY2spOw0K
PiA+IA0KPiA+ICsJcmV0ID0gbmZzNDFfY2hlY2tfc2Vzc2lvbl9yZWFkeShjbHApOw0KPiANCj4g
SWYgSSByZWFkIHRoZSBjb2RlIGNvcnJlY3RseSwgb24gYW4gZXhpc3RpbmcgY2xpZW50LCAgdGhp
cyBjYWxsIG5lZWRsZXNzbHkgd2FpdHMgZm9yIHRoZSBzdGF0ZSBtYW5hZ2VyIHRvIGZpbmlzaCB3
aGF0ZXZlciBpdCBpcyBkb2luZy4gQXMgdGhlIHN0YXRlIG1hbmFnZXIgY291bGQgYmUgb3BlcmF0
aW5nIG9uIGEgZGlmZmVyZW50IGZpbGUgc3lzdGVtIHRoYW4gdGhlIERTIGJlbG9uZ3MgdG8sICBh
bmQgcmVjb3ZlcmluZyBhIGJ1bmNoIG9mIHN0YXRlLCAgaXQgbWlnaHQgYmUgd29ydGggd2hpbGUg
dG8gYXZvaWQuICBUaGF0IHdhcyB0aGUgaW50ZW50aW9uIG9mIGNoZWNraW5nIHRoZSBjbF9leGNo
YW5nZV9mbGFncyBpbiBuZnM0X2RzX2Nvbm5lY3QuDQo+IEluIGZhY3QsIG9uIGFuIGV4aXN0aW5n
IGNsaWVudCwgbmZzNF9pbml0X2RzX3Nlc3Npb24gZG9lcyBub3QgbmVlZCB0byBiZSBjYWxsZWQu
DQoNClllcyBpdCBkb2VzLiBSaWdodCBub3cgeW91IGhhdmUgbm8gZ3VhcmFudGVlIHRoYXQgdGhl
IHN0YXRlIG1hbmFnZXIgaXMNCmZpbmlzaGVkIHNldHRpbmcgdXAgdGhlIHNlc3Npb24sIGFuZCBz
byBubyBndWFyYW50ZWUgdGhhdCBpdCB3aWxsDQpzdWNjZWVkLg0KDQpXZSBjYW4gcGVyaGFwcyBz
a2lwIHRoZSBjYWxsIHRvIG5mczRfcmVjb3Zlcl9leHBpcmVkX2xlYXNlKCkgaWYgKGFuZA0Kb25s
eSBpZikgY2xwLT5jbF9jb25zX3N0YXRlIDw9IE5GU19DU19SRUFEWSwgYnV0IG90aGVyd2lzZSBp
dCBkb2VzIG5lZWQNCnRvIGNvbXBsZXRlLg0KDQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXgg
TkZTIGNsaWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0KVHJvbmQuTXlrbGVidXN0QG5ldGFwcC5j
b20NCnd3dy5uZXRhcHAuY29tDQoNCg==

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] NFSv4.1: Fix session initialisation races
  2012-05-23 19:14   ` Myklebust, Trond
@ 2012-05-23 19:20     ` Myklebust, Trond
  2012-05-23 19:25       ` Adamson, Andy
  0 siblings, 1 reply; 10+ messages in thread
From: Myklebust, Trond @ 2012-05-23 19:20 UTC (permalink / raw)
  To: Adamson, Andy; +Cc: <linux-nfs@vger.kernel.org>

T24gV2VkLCAyMDEyLTA1LTIzIGF0IDE5OjE0ICswMDAwLCBNeWtsZWJ1c3QsIFRyb25kIHdyb3Rl
Og0KPiBPbiBXZWQsIDIwMTItMDUtMjMgYXQgMTg6MzcgKzAwMDAsIEFkYW1zb24sIEFuZHkgd3Jv
dGU6DQo+ID4gDQo+ID4gT24gTWF5IDIzLCAyMDEyLCBhdCAxOjMyIFBNLCBUcm9uZCBNeWtsZWJ1
c3Qgd3JvdGU6DQo+ID4gDQo+ID4gPiBTZXNzaW9uIGluaXRpYWxpc2F0aW9uIGlzIG5vdCBjb21w
bGV0ZSB1bnRpbCB0aGUgbGVhc2UgbWFuYWdlcg0KPiA+ID4gaGFzIHJ1bi4gV2UgbmVlZCB0byBl
bnN1cmUgdGhhdCBib3RoIG5mczRfaW5pdF9zZXNzaW9uIGFuZA0KPiA+ID4gbmZzNF9pbml0X2Rz
X3Nlc3Npb24gZG8gc28sIGFuZCB0aGF0IHRoZXkgY2hlY2sgZm9yIGFueSByZXN1bHRpbmcNCj4g
PiA+IGVycm9ycyBpbiBjbHAtPmNsX2NvbnNfc3RhdGUuDQo+ID4gPiANCj4gPiA+IE9ubHkgYWZ0
ZXIgdGhpcyBpcyBkb25lLCBjYW4gbmZzNF9kc19jb25uZWN0IGNoZWNrIHRoZSBjb250ZW50cw0K
PiA+ID4gb2YgY2xwLT5jbF9leGNoYW5nZV9mbGFncy4NCj4gPiA+IA0KPiA+ID4gU2lnbmVkLW9m
Zi1ieTogVHJvbmQgTXlrbGVidXN0IDxUcm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbT4NCj4gPiA+
IENjOiBBbmR5IEFkYW1zb24gPGFuZHJvc0BuZXRhcHAuY29tPg0KPiA+ID4gLS0tDQo+ID4gPiBm
cy9uZnMvY2xpZW50LmMgICAgICAgICAgICB8ICAgMTYgLS0tLS0tLS0tDQo+ID4gPiBmcy9uZnMv
aW50ZXJuYWwuaCAgICAgICAgICB8ICAgIDMgKy0NCj4gPiA+IGZzL25mcy9uZnM0ZmlsZWxheW91
dGRldi5jIHwgICAyMyArLS0tLS0tLS0tLS0tDQo+ID4gPiBmcy9uZnMvbmZzNHByb2MuYyAgICAg
ICAgICB8ICAgNzcgKysrKysrKysrKysrKysrKysrKysrKysrKysrKy0tLS0tLS0tLS0tLS0tLQ0K
PiA+ID4gNCBmaWxlcyBjaGFuZ2VkLCA1MiBpbnNlcnRpb25zKCspLCA2NyBkZWxldGlvbnMoLSkN
Cj4gPiA+IA0KPiA+ID4gZGlmZiAtLWdpdCBhL2ZzL25mcy9jbGllbnQuYyBiL2ZzL25mcy9jbGll
bnQuYw0KPiA+ID4gaW5kZXggNjBmN2U0ZS4uNzg5NzBhMSAxMDA2NDQNCj4gPiA+IC0tLSBhL2Zz
L25mcy9jbGllbnQuYw0KPiA+ID4gKysrIGIvZnMvbmZzL2NsaWVudC5jDQo+ID4gPiBAQCAtNTkz
LDIyICs1OTMsNiBAQCB2b2lkIG5mc19tYXJrX2NsaWVudF9yZWFkeShzdHJ1Y3QgbmZzX2NsaWVu
dCAqY2xwLCBpbnQgc3RhdGUpDQo+ID4gPiB9DQo+ID4gPiANCj4gPiA+IC8qDQo+ID4gPiAtICog
V2l0aCBzZXNzaW9ucywgdGhlIGNsaWVudCBpcyBub3QgbWFya2VkIHJlYWR5IHVudGlsIGFmdGVy
IGENCj4gPiA+IC0gKiBzdWNjZXNzZnVsIEVYQ0hBTkdFX0lEIGFuZCBDUkVBVEVfU0VTU0lPTi4N
Cj4gPiA+IC0gKg0KPiA+ID4gLSAqIE1hcCBlcnJvcnMgY2xfY29uc19zdGF0ZSBlcnJvcnMgdG8g
RVBST1RPTk9TVVBQT1JUIHRvIGluZGljYXRlDQo+ID4gPiAtICogb3RoZXIgdmVyc2lvbnMgb2Yg
TkZTIGNhbiBiZSB0cmllZC4NCj4gPiA+IC0gKi8NCj4gPiA+IC1pbnQgbmZzNF9jaGVja19jbGll
bnRfcmVhZHkoc3RydWN0IG5mc19jbGllbnQgKmNscCkNCj4gPiA+IC17DQo+ID4gPiAtCWlmICgh
bmZzNF9oYXNfc2Vzc2lvbihjbHApKQ0KPiA+ID4gLQkJcmV0dXJuIDA7DQo+ID4gPiAtCWlmIChj
bHAtPmNsX2NvbnNfc3RhdGUgPCBORlNfQ1NfUkVBRFkpDQo+ID4gPiAtCQlyZXR1cm4gLUVQUk9U
T05PU1VQUE9SVDsNCj4gPiA+IC0JcmV0dXJuIDA7DQo+ID4gPiAtfQ0KPiA+ID4gLQ0KPiA+ID4g
LS8qDQo+ID4gPiAgKiBJbml0aWFsaXNlIHRoZSB0aW1lb3V0IHZhbHVlcyBmb3IgYSBjb25uZWN0
aW9uDQo+ID4gPiAgKi8NCj4gPiA+IHN0YXRpYyB2b2lkIG5mc19pbml0X3RpbWVvdXRfdmFsdWVz
KHN0cnVjdCBycGNfdGltZW91dCAqdG8sIGludCBwcm90bywNCj4gPiA+IGRpZmYgLS1naXQgYS9m
cy9uZnMvaW50ZXJuYWwuaCBiL2ZzL25mcy9pbnRlcm5hbC5oDQo+ID4gPiBpbmRleCBiNzc3YmRh
Li4wMGI2NmRlIDEwMDY0NA0KPiA+ID4gLS0tIGEvZnMvbmZzL2ludGVybmFsLmgNCj4gPiA+ICsr
KyBiL2ZzL25mcy9pbnRlcm5hbC5oDQo+ID4gPiBAQCAtMTY4LDcgKzE2OCw2IEBAIGV4dGVybiBz
dHJ1Y3QgbmZzX3NlcnZlciAqbmZzX2Nsb25lX3NlcnZlcihzdHJ1Y3QgbmZzX3NlcnZlciAqLA0K
PiA+ID4gCQkJCQkgICBzdHJ1Y3QgbmZzX2ZhdHRyICosDQo+ID4gPiAJCQkJCSAgIHJwY19hdXRo
Zmxhdm9yX3QpOw0KPiA+ID4gZXh0ZXJuIHZvaWQgbmZzX21hcmtfY2xpZW50X3JlYWR5KHN0cnVj
dCBuZnNfY2xpZW50ICpjbHAsIGludCBzdGF0ZSk7DQo+ID4gPiAtZXh0ZXJuIGludCBuZnM0X2No
ZWNrX2NsaWVudF9yZWFkeShzdHJ1Y3QgbmZzX2NsaWVudCAqY2xwKTsNCj4gPiA+IGV4dGVybiBz
dHJ1Y3QgbmZzX2NsaWVudCAqbmZzNF9zZXRfZHNfY2xpZW50KHN0cnVjdCBuZnNfY2xpZW50KiBt
ZHNfY2xwLA0KPiA+ID4gCQkJCQkgICAgIGNvbnN0IHN0cnVjdCBzb2NrYWRkciAqZHNfYWRkciwN
Cj4gPiA+IAkJCQkJICAgICBpbnQgZHNfYWRkcmxlbiwgaW50IGRzX3Byb3RvKTsNCj4gPiA+IEBA
IC0yMzcsNyArMjM2LDcgQEAgZXh0ZXJuIGNvbnN0IHUzMiBuZnM0MV9tYXh3cml0ZV9vdmVyaGVh
ZDsNCj4gPiA+IGV4dGVybiBzdHJ1Y3QgcnBjX3Byb2NpbmZvIG5mczRfcHJvY2VkdXJlc1tdOw0K
PiA+ID4gI2VuZGlmDQo+ID4gPiANCj4gPiA+IC1leHRlcm4gaW50IG5mczRfaW5pdF9kc19zZXNz
aW9uKHN0cnVjdCBuZnNfY2xpZW50ICpjbHApOw0KPiA+ID4gK2V4dGVybiBpbnQgbmZzNF9pbml0
X2RzX3Nlc3Npb24oc3RydWN0IG5mc19jbGllbnQgKiwgdW5zaWduZWQgbG9uZyk7DQo+ID4gPiAN
Cj4gPiA+IC8qIHByb2MuYyAqLw0KPiA+ID4gdm9pZCBuZnNfY2xvc2VfY29udGV4dChzdHJ1Y3Qg
bmZzX29wZW5fY29udGV4dCAqY3R4LCBpbnQgaXNfc3luYyk7DQo+ID4gPiBkaWZmIC0tZ2l0IGEv
ZnMvbmZzL25mczRmaWxlbGF5b3V0ZGV2LmMgYi9mcy9uZnMvbmZzNGZpbGVsYXlvdXRkZXYuYw0K
PiA+ID4gaW5kZXggYzljZmY5YS4uMDc2NGM5OCAxMDA2NDQNCj4gPiA+IC0tLSBhL2ZzL25mcy9u
ZnM0ZmlsZWxheW91dGRldi5jDQo+ID4gPiArKysgYi9mcy9uZnMvbmZzNGZpbGVsYXlvdXRkZXYu
Yw0KPiA+ID4gQEAgLTE3NiwyOCArMTc2LDcgQEAgbmZzNF9kc19jb25uZWN0KHN0cnVjdCBuZnNf
c2VydmVyICptZHNfc3J2LCBzdHJ1Y3QgbmZzNF9wbmZzX2RzICpkcykNCj4gPiA+IAkJZ290byBv
dXQ7DQo+ID4gPiAJfQ0KPiA+ID4gDQo+ID4gPiAtCWlmICgoY2xwLT5jbF9leGNoYW5nZV9mbGFn
cyAmIEVYQ0hHSUQ0X0ZMQUdfTUFTS19QTkZTKSAhPSAwKSB7DQo+ID4gPiAtCQlpZiAoIWlzX2Rz
X2NsaWVudChjbHApKSB7DQo+ID4gPiAtCQkJc3RhdHVzID0gLUVOT0RFVjsNCj4gPiA+IC0JCQln
b3RvIG91dF9wdXQ7DQo+ID4gPiAtCQl9DQo+ID4gPiAtCQlkcy0+ZHNfY2xwID0gY2xwOw0KPiA+
ID4gLQkJZHByaW50aygiJXMgW2V4aXN0aW5nXSBzZXJ2ZXI9JXNcbiIsIF9fZnVuY19fLA0KPiA+
ID4gLQkJCWRzLT5kc19yZW1vdGVzdHIpOw0KPiA+ID4gLQkJZ290byBvdXQ7DQo+ID4gPiAtCX0N
Cj4gPiA+IC0NCj4gPiA+IC0JLyoNCj4gPiA+IC0JICogRG8gbm90IHNldCBORlNfQ1NfQ0hFQ0tf
TEVBU0VfVElNRSBpbnN0ZWFkIHNldCB0aGUgRFMgbGVhc2UgdG8NCj4gPiA+IC0JICogYmUgZXF1
YWwgdG8gdGhlIE1EUyBsZWFzZS4gUmVuZXdhbCBpcyBzY2hlZHVsZWQgaW4gY3JlYXRlX3Nlc3Np
b24uDQo+ID4gPiAtCSAqLw0KPiA+ID4gLQlzcGluX2xvY2soJm1kc19zcnYtPm5mc19jbGllbnQt
PmNsX2xvY2spOw0KPiA+ID4gLQljbHAtPmNsX2xlYXNlX3RpbWUgPSBtZHNfc3J2LT5uZnNfY2xp
ZW50LT5jbF9sZWFzZV90aW1lOw0KPiA+ID4gLQlzcGluX3VubG9jaygmbWRzX3Nydi0+bmZzX2Ns
aWVudC0+Y2xfbG9jayk7DQo+ID4gPiAtCWNscC0+Y2xfbGFzdF9yZW5ld2FsID0gamlmZmllczsN
Cj4gPiA+IC0NCj4gPiA+IC0JLyogTmV3IG5mc19jbGllbnQgKi8NCj4gPiA+IC0Jc3RhdHVzID0g
bmZzNF9pbml0X2RzX3Nlc3Npb24oY2xwKTsNCj4gPiA+ICsJc3RhdHVzID0gbmZzNF9pbml0X2Rz
X3Nlc3Npb24oY2xwLCBtZHNfc3J2LT5uZnNfY2xpZW50LT5jbF9sZWFzZV90aW1lKTsNCj4gPiA+
IAlpZiAoc3RhdHVzKQ0KPiA+ID4gCQlnb3RvIG91dF9wdXQ7DQo+ID4gPiANCj4gPiA+IGRpZmYg
LS1naXQgYS9mcy9uZnMvbmZzNHByb2MuYyBiL2ZzL25mcy9uZnM0cHJvYy5jDQo+ID4gPiBpbmRl
eCA5OTY1MGFhLi4yNzJjNGFkIDEwMDY0NA0KPiA+ID4gLS0tIGEvZnMvbmZzL25mczRwcm9jLmMN
Cj4gPiA+ICsrKyBiL2ZzL25mcy9uZnM0cHJvYy5jDQo+ID4gPiBAQCAtNTYwNSw1MyArNTYwNSw3
NiBAQCBpbnQgbmZzNF9wcm9jX2Rlc3Ryb3lfc2Vzc2lvbihzdHJ1Y3QgbmZzNF9zZXNzaW9uICpz
ZXNzaW9uKQ0KPiA+ID4gCXJldHVybiBzdGF0dXM7DQo+ID4gPiB9DQo+ID4gPiANCj4gPiA+ICsv
Kg0KPiA+ID4gKyAqIFdpdGggc2Vzc2lvbnMsIHRoZSBjbGllbnQgaXMgbm90IG1hcmtlZCByZWFk
eSB1bnRpbCBhZnRlciBhDQo+ID4gPiArICogc3VjY2Vzc2Z1bCBFWENIQU5HRV9JRCBhbmQgQ1JF
QVRFX1NFU1NJT04uDQo+ID4gPiArICoNCj4gPiA+ICsgKiBNYXAgZXJyb3JzIGNsX2NvbnNfc3Rh
dGUgZXJyb3JzIHRvIEVQUk9UT05PU1VQUE9SVCB0byBpbmRpY2F0ZQ0KPiA+ID4gKyAqIG90aGVy
IHZlcnNpb25zIG9mIE5GUyBjYW4gYmUgdHJpZWQuDQo+ID4gPiArICovDQo+ID4gPiArc3RhdGlj
IGludCBuZnM0MV9jaGVja19zZXNzaW9uX3JlYWR5KHN0cnVjdCBuZnNfY2xpZW50ICpjbHApDQo+
ID4gPiArew0KPiA+ID4gKwlpbnQgcmV0Ow0KPiA+ID4gKwkNCj4gPiA+ICsJcmV0ID0gbmZzNF9j
bGllbnRfcmVjb3Zlcl9leHBpcmVkX2xlYXNlKGNscCk7DQo+ID4gPiArCWlmIChyZXQpDQo+ID4g
PiArCQlyZXR1cm4gcmV0Ow0KPiA+ID4gKwlpZiAoY2xwLT5jbF9jb25zX3N0YXRlIDwgTkZTX0NT
X1JFQURZKQ0KPiA+ID4gKwkJcmV0dXJuIC1FUFJPVE9OT1NVUFBPUlQ7DQo+ID4gPiArCXJldHVy
biAwOw0KPiA+ID4gK30NCj4gPiA+ICsNCj4gPiA+IGludCBuZnM0X2luaXRfc2Vzc2lvbihzdHJ1
Y3QgbmZzX3NlcnZlciAqc2VydmVyKQ0KPiA+ID4gew0KPiA+ID4gCXN0cnVjdCBuZnNfY2xpZW50
ICpjbHAgPSBzZXJ2ZXItPm5mc19jbGllbnQ7DQo+ID4gPiAJc3RydWN0IG5mczRfc2Vzc2lvbiAq
c2Vzc2lvbjsNCj4gPiA+IAl1bnNpZ25lZCBpbnQgcnNpemUsIHdzaXplOw0KPiA+ID4gLQlpbnQg
cmV0Ow0KPiA+ID4gDQo+ID4gPiAJaWYgKCFuZnM0X2hhc19zZXNzaW9uKGNscCkpDQo+ID4gPiAJ
CXJldHVybiAwOw0KPiA+ID4gDQo+ID4gPiAJc2Vzc2lvbiA9IGNscC0+Y2xfc2Vzc2lvbjsNCj4g
PiA+IC0JaWYgKCF0ZXN0X2FuZF9jbGVhcl9iaXQoTkZTNF9TRVNTSU9OX0lOSVRJTkcsICZzZXNz
aW9uLT5zZXNzaW9uX3N0YXRlKSkNCj4gPiA+IC0JCXJldHVybiAwOw0KPiA+ID4gKwlzcGluX2xv
Y2soJmNscC0+Y2xfbG9jayk7DQo+ID4gPiArCWlmICh0ZXN0X2FuZF9jbGVhcl9iaXQoTkZTNF9T
RVNTSU9OX0lOSVRJTkcsICZzZXNzaW9uLT5zZXNzaW9uX3N0YXRlKSkgew0KPiA+ID4gDQo+ID4g
PiAtCXJzaXplID0gc2VydmVyLT5yc2l6ZTsNCj4gPiA+IC0JaWYgKHJzaXplID09IDApDQo+ID4g
PiAtCQlyc2l6ZSA9IE5GU19NQVhfRklMRV9JT19TSVpFOw0KPiA+ID4gLQl3c2l6ZSA9IHNlcnZl
ci0+d3NpemU7DQo+ID4gPiAtCWlmICh3c2l6ZSA9PSAwKQ0KPiA+ID4gLQkJd3NpemUgPSBORlNf
TUFYX0ZJTEVfSU9fU0laRTsNCj4gPiA+ICsJCXJzaXplID0gc2VydmVyLT5yc2l6ZTsNCj4gPiA+
ICsJCWlmIChyc2l6ZSA9PSAwKQ0KPiA+ID4gKwkJCXJzaXplID0gTkZTX01BWF9GSUxFX0lPX1NJ
WkU7DQo+ID4gPiArCQl3c2l6ZSA9IHNlcnZlci0+d3NpemU7DQo+ID4gPiArCQlpZiAod3NpemUg
PT0gMCkNCj4gPiA+ICsJCQl3c2l6ZSA9IE5GU19NQVhfRklMRV9JT19TSVpFOw0KPiA+ID4gDQo+
ID4gPiAtCXNlc3Npb24tPmZjX2F0dHJzLm1heF9ycXN0X3N6ID0gd3NpemUgKyBuZnM0MV9tYXh3
cml0ZV9vdmVyaGVhZDsNCj4gPiA+IC0Jc2Vzc2lvbi0+ZmNfYXR0cnMubWF4X3Jlc3Bfc3ogPSBy
c2l6ZSArIG5mczQxX21heHJlYWRfb3ZlcmhlYWQ7DQo+ID4gPiArCQlzZXNzaW9uLT5mY19hdHRy
cy5tYXhfcnFzdF9zeiA9IHdzaXplICsgbmZzNDFfbWF4d3JpdGVfb3ZlcmhlYWQ7DQo+ID4gPiAr
CQlzZXNzaW9uLT5mY19hdHRycy5tYXhfcmVzcF9zeiA9IHJzaXplICsgbmZzNDFfbWF4cmVhZF9v
dmVyaGVhZDsNCj4gPiA+ICsJfQ0KPiA+ID4gKwlzcGluX3VubG9jaygmY2xwLT5jbF9sb2NrKTsN
Cj4gPiA+IA0KPiA+ID4gLQlyZXQgPSBuZnM0X3JlY292ZXJfZXhwaXJlZF9sZWFzZShzZXJ2ZXIp
Ow0KPiA+ID4gLQlpZiAoIXJldCkNCj4gPiA+IC0JCXJldCA9IG5mczRfY2hlY2tfY2xpZW50X3Jl
YWR5KGNscCk7DQo+ID4gPiAtCXJldHVybiByZXQ7DQo+ID4gPiArCXJldHVybiBuZnM0MV9jaGVj
a19zZXNzaW9uX3JlYWR5KGNscCk7DQo+ID4gPiB9DQo+ID4gPiANCj4gPiA+IC1pbnQgbmZzNF9p
bml0X2RzX3Nlc3Npb24oc3RydWN0IG5mc19jbGllbnQgKmNscCkNCj4gPiA+ICtpbnQgbmZzNF9p
bml0X2RzX3Nlc3Npb24oc3RydWN0IG5mc19jbGllbnQgKmNscCwgdW5zaWduZWQgbG9uZyBsZWFz
ZV90aW1lKQ0KPiA+ID4gew0KPiA+ID4gCXN0cnVjdCBuZnM0X3Nlc3Npb24gKnNlc3Npb24gPSBj
bHAtPmNsX3Nlc3Npb247DQo+ID4gPiAJaW50IHJldDsNCj4gPiA+IA0KPiA+ID4gLQlpZiAoIXRl
c3RfYW5kX2NsZWFyX2JpdChORlM0X1NFU1NJT05fSU5JVElORywgJnNlc3Npb24tPnNlc3Npb25f
c3RhdGUpKQ0KPiA+ID4gLQkJcmV0dXJuIDA7DQo+ID4gPiAtDQo+ID4gPiAtCXJldCA9IG5mczRf
Y2xpZW50X3JlY292ZXJfZXhwaXJlZF9sZWFzZShjbHApOw0KPiA+ID4gLQlpZiAoIXJldCkNCj4g
PiA+IC0JCS8qIFRlc3QgZm9yIHRoZSBEUyByb2xlICovDQo+ID4gPiAtCQlpZiAoIWlzX2RzX2Ns
aWVudChjbHApKQ0KPiA+ID4gLQkJCXJldCA9IC1FTk9ERVY7DQo+ID4gPiAtCWlmICghcmV0KQ0K
PiA+ID4gLQkJcmV0ID0gbmZzNF9jaGVja19jbGllbnRfcmVhZHkoY2xwKTsNCj4gPiA+IC0JcmV0
dXJuIHJldDsNCj4gPiA+ICsJc3Bpbl9sb2NrKCZjbHAtPmNsX2xvY2spOw0KPiA+ID4gKwlpZiAo
dGVzdF9hbmRfY2xlYXJfYml0KE5GUzRfU0VTU0lPTl9JTklUSU5HLCAmc2Vzc2lvbi0+c2Vzc2lv
bl9zdGF0ZSkpIHsNCj4gPiA+ICsJCS8qDQo+ID4gPiArCQkgKiBEbyBub3Qgc2V0IE5GU19DU19D
SEVDS19MRUFTRV9USU1FIGluc3RlYWQgc2V0IHRoZQ0KPiA+ID4gKwkJICogRFMgbGVhc2UgdG8g
YmUgZXF1YWwgdG8gdGhlIE1EUyBsZWFzZS4NCj4gPiA+ICsJCSAqLw0KPiA+ID4gKwkJY2xwLT5j
bF9sZWFzZV90aW1lID0gbGVhc2VfdGltZTsNCj4gPiA+ICsJCWNscC0+Y2xfbGFzdF9yZW5ld2Fs
ID0gamlmZmllczsNCj4gPiA+ICsJfQ0KPiA+ID4gKwlzcGluX3VubG9jaygmY2xwLT5jbF9sb2Nr
KTsNCj4gPiA+IA0KPiA+ID4gKwlyZXQgPSBuZnM0MV9jaGVja19zZXNzaW9uX3JlYWR5KGNscCk7
DQo+ID4gDQo+ID4gSWYgSSByZWFkIHRoZSBjb2RlIGNvcnJlY3RseSwgb24gYW4gZXhpc3Rpbmcg
Y2xpZW50LCAgdGhpcyBjYWxsIG5lZWRsZXNzbHkgd2FpdHMgZm9yIHRoZSBzdGF0ZSBtYW5hZ2Vy
IHRvIGZpbmlzaCB3aGF0ZXZlciBpdCBpcyBkb2luZy4gQXMgdGhlIHN0YXRlIG1hbmFnZXIgY291
bGQgYmUgb3BlcmF0aW5nIG9uIGEgZGlmZmVyZW50IGZpbGUgc3lzdGVtIHRoYW4gdGhlIERTIGJl
bG9uZ3MgdG8sICBhbmQgcmVjb3ZlcmluZyBhIGJ1bmNoIG9mIHN0YXRlLCAgaXQgbWlnaHQgYmUg
d29ydGggd2hpbGUgdG8gYXZvaWQuICBUaGF0IHdhcyB0aGUgaW50ZW50aW9uIG9mIGNoZWNraW5n
IHRoZSBjbF9leGNoYW5nZV9mbGFncyBpbiBuZnM0X2RzX2Nvbm5lY3QuDQo+ID4gSW4gZmFjdCwg
b24gYW4gZXhpc3RpbmcgY2xpZW50LCBuZnM0X2luaXRfZHNfc2Vzc2lvbiBkb2VzIG5vdCBuZWVk
IHRvIGJlIGNhbGxlZC4NCj4gDQo+IFllcyBpdCBkb2VzLiBSaWdodCBub3cgeW91IGhhdmUgbm8g
Z3VhcmFudGVlIHRoYXQgdGhlIHN0YXRlIG1hbmFnZXIgaXMNCj4gZmluaXNoZWQgc2V0dGluZyB1
cCB0aGUgc2Vzc2lvbiwgYW5kIHNvIG5vIGd1YXJhbnRlZSB0aGF0IGl0IHdpbGwNCj4gc3VjY2Vl
ZC4NCj4gDQo+IFdlIGNhbiBwZXJoYXBzIHNraXAgdGhlIGNhbGwgdG8gbmZzNF9yZWNvdmVyX2V4
cGlyZWRfbGVhc2UoKSBpZiAoYW5kDQo+IG9ubHkgaWYpIGNscC0+Y2xfY29uc19zdGF0ZSA8PSBO
RlNfQ1NfUkVBRFksIGJ1dCBvdGhlcndpc2UgaXQgZG9lcyBuZWVkDQo+IHRvIGNvbXBsZXRlLg0K
DQpJT1c6IFNvbWV0aGluZyBsaWtlIHRoZSBmb2xsb3dpbmcgY2hhbmdlIHRvIG5mczQxX2NoZWNr
X3Nlc3Npb25fcmVhZHkoKToNCg0KZGlmZiAtLWdpdCBhL2ZzL25mcy9uZnM0cHJvYy5jIGIvZnMv
bmZzL25mczRwcm9jLmMNCmluZGV4IDU2NTEwNWIuLmM4NTYyOTggMTAwNjQ0DQotLS0gYS9mcy9u
ZnMvbmZzNHByb2MuYw0KKysrIGIvZnMvbmZzL25mczRwcm9jLmMNCkBAIC01NjE0LDkgKzU2MTQs
MTEgQEAgc3RhdGljIGludCBuZnM0MV9jaGVja19zZXNzaW9uX3JlYWR5KHN0cnVjdCBuZnNfY2xp
ZW50ICpjbHApDQogew0KIAlpbnQgcmV0Ow0KIAkNCi0JcmV0ID0gbmZzNF9jbGllbnRfcmVjb3Zl
cl9leHBpcmVkX2xlYXNlKGNscCk7DQotCWlmIChyZXQpDQotCQlyZXR1cm4gcmV0Ow0KKwlpZiAo
Y2xwLT5jbF9jb25zX3N0YXRlID09IE5GU19DU19TRVNTSU9OX0lOSVRJTkcpIHsNCisJCXJldCA9
IG5mczRfY2xpZW50X3JlY292ZXJfZXhwaXJlZF9sZWFzZShjbHApOw0KKwkJaWYgKHJldCkNCisJ
CQlyZXR1cm4gcmV0Ow0KKwl9DQogCWlmIChjbHAtPmNsX2NvbnNfc3RhdGUgPCBORlNfQ1NfUkVB
RFkpDQogCQlyZXR1cm4gLUVQUk9UT05PU1VQUE9SVDsNCiAJcmV0dXJuIDA7DQoNCg0KLS0gDQpU
cm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRhaW5lcg0KDQpOZXRBcHANClRy
b25kLk15a2xlYnVzdEBuZXRhcHAuY29tDQp3d3cubmV0YXBwLmNvbQ0KDQo=

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] NFSv4.1: Fix session initialisation races
  2012-05-23 19:20     ` Myklebust, Trond
@ 2012-05-23 19:25       ` Adamson, Andy
  0 siblings, 0 replies; 10+ messages in thread
From: Adamson, Andy @ 2012-05-23 19:25 UTC (permalink / raw)
  To: Myklebust, Trond; +Cc: Adamson, Andy, <linux-nfs@vger.kernel.org>


On May 23, 2012, at 3:20 PM, Myklebust, Trond wrote:

> On Wed, 2012-05-23 at 19:14 +0000, Myklebust, Trond wrote:
>> On Wed, 2012-05-23 at 18:37 +0000, Adamson, Andy wrote:
>>> 
>>> On May 23, 2012, at 1:32 PM, Trond Myklebust wrote:
>>> 
>>>> 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>
>>>> ---
>>>> fs/nfs/client.c            |   16 ---------
>>>> fs/nfs/internal.h          |    3 +-
>>>> fs/nfs/nfs4filelayoutdev.c |   23 +------------
>>>> fs/nfs/nfs4proc.c          |   77 ++++++++++++++++++++++++++++---------------
>>>> 4 files changed, 52 insertions(+), 67 deletions(-)
>>>> 
>>>> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
>>>> index 60f7e4e..78970a1 100644
>>>> --- a/fs/nfs/client.c
>>>> +++ b/fs/nfs/client.c
>>>> @@ -593,22 +593,6 @@ void nfs_mark_client_ready(struct nfs_client *clp, int state)
>>>> }
>>>> 
>>>> /*
>>>> - * 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,
>>>> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
>>>> index b777bda..00b66de 100644
>>>> --- a/fs/nfs/internal.h
>>>> +++ b/fs/nfs/internal.h
>>>> @@ -168,7 +168,6 @@ extern struct nfs_server *nfs_clone_server(struct nfs_server *,
>>>> 					   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);
>>>> @@ -237,7 +236,7 @@ extern const u32 nfs41_maxwrite_overhead;
>>>> 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);
>>>> diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c
>>>> index c9cff9a..0764c98 100644
>>>> --- a/fs/nfs/nfs4filelayoutdev.c
>>>> +++ b/fs/nfs/nfs4filelayoutdev.c
>>>> @@ -176,28 +176,7 @@ nfs4_ds_connect(struct nfs_server *mds_srv, struct nfs4_pnfs_ds *ds)
>>>> 		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;
>>>> 
>>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>>>> index 99650aa..272c4ad 100644
>>>> --- a/fs/nfs/nfs4proc.c
>>>> +++ b/fs/nfs/nfs4proc.c
>>>> @@ -5605,53 +5605,76 @@ int nfs4_proc_destroy_session(struct nfs4_session *session)
>>>> 	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;
>>>> +	
>>>> +	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;
>>>> -
>>>> -	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;
>>>> +	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 = nfs41_check_session_ready(clp);
>>> 
>>> If I read the code correctly, on an existing client,  this call needlessly waits for the state manager to finish whatever it is doing. As the state manager could be operating on a different file system than the DS belongs to,  and recovering a bunch of state,  it might be worth while to avoid.  That was the intention of checking the cl_exchange_flags in nfs4_ds_connect.
>>> In fact, on an existing client, nfs4_init_ds_session does not need to be called.
>> 
>> Yes it does. Right now you have no guarantee that the state manager is
>> finished setting up the session, and so no guarantee that it will
>> succeed.
>> 
>> We can perhaps skip the call to nfs4_recover_expired_lease() if (and
>> only if) clp->cl_cons_state <= NFS_CS_READY, but otherwise it does need
>> to complete.
> 
> IOW: Something like the following change to nfs41_check_session_ready():
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 565105b..c856298 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -5614,9 +5614,11 @@ static int nfs41_check_session_ready(struct nfs_client *clp)
> {
> 	int ret;
> 	
> -	ret = nfs4_client_recover_expired_lease(clp);
> -	if (ret)
> -		return 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;
> 

Yes - that addresses my concern. Nice cleanup of the ds connection code.

-->Andy

> 
> -- 
> Trond Myklebust
> Linux NFS client maintainer
> 
> NetApp
> Trond.Myklebust@netapp.com
> www.netapp.com
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] NFS: Add memory barriers to the nfs_client->cl_cons_state initialisation
  2012-05-23 17:32   ` [PATCH 3/3] NFS: Add memory barriers to the nfs_client->cl_cons_state initialisation Trond Myklebust
@ 2012-05-24  4:36     ` Stanislav Kinsbursky
  2012-05-24 11:59       ` Myklebust, Trond
  0 siblings, 1 reply; 10+ messages in thread
From: Stanislav Kinsbursky @ 2012-05-24  4:36 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: linux-nfs

23.05.2012 21:32, Trond Myklebust написал:
> Ensure that a process that uses the nfs_client->cl_cons_state test
> for whether the initialisation process is finished does not read
> stale data.
>
> Signed-off-by: Trond Myklebust<Trond.Myklebust@netapp.com>
> ---
>   fs/nfs/client.c |    5 +++++
>   fs/nfs/idmap.c  |    1 +
>   2 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index e6070ea..4be85f9 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -456,6 +456,8 @@ static bool nfs4_cb_match_client(const struct sockaddr *addr,
>   	    clp->cl_cons_state == NFS_CS_SESSION_INITING))
>   		return false;
>
> +	smp_rmb();
> +
>   	/* Match the version and minorversion */
>   	if (clp->rpc_ops->version != 4 ||
>   	    clp->cl_minorversion != minorversion)
> @@ -587,6 +589,8 @@ found_client:
>   		return ERR_PTR(error);
>   	}
>
> +	smp_rmb();
> +
>   	BUG_ON(clp->cl_cons_state != NFS_CS_READY);
>
>   	dprintk("-->  nfs_get_client() = %p [share]\n", clp);
> @@ -598,6 +602,7 @@ found_client:
>    */
>   void nfs_mark_client_ready(struct nfs_client *clp, int state)
>   {
> +	smp_wmb();

BTW, why barrier is before assignment?

>   	clp->cl_cons_state = state;
>   	wake_up_all(&nfs_client_active_wq);
>   }
> diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
> index 6ca949b..7d4e8dd 100644
> --- a/fs/nfs/idmap.c
> +++ b/fs/nfs/idmap.c
> @@ -548,6 +548,7 @@ restart:
>   		/* Skip nfs_clients that failed to initialise */
>   		if (clp->cl_cons_state<  0)
>   			continue;
> +		smp_rmb();
>   		if (clp->rpc_ops !=&nfs_v4_clientops)
>   			continue;
>   		cl_dentry = clp->cl_idmap->idmap_pipe->dentry;


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] NFS: Add memory barriers to the nfs_client->cl_cons_state initialisation
  2012-05-24  4:36     ` Stanislav Kinsbursky
@ 2012-05-24 11:59       ` Myklebust, Trond
  0 siblings, 0 replies; 10+ messages in thread
From: Myklebust, Trond @ 2012-05-24 11:59 UTC (permalink / raw)
  To: Stanislav Kinsbursky; +Cc: linux-nfs

T24gVGh1LCAyMDEyLTA1LTI0IGF0IDA4OjM2ICswNDAwLCBTdGFuaXNsYXYgS2luc2J1cnNreSB3
cm90ZToNCj4gMjMuMDUuMjAxMiAyMTozMiwgVHJvbmQgTXlrbGVidXN0INC90LDQv9C40YHQsNC7
Og0KPiA+IEVuc3VyZSB0aGF0IGEgcHJvY2VzcyB0aGF0IHVzZXMgdGhlIG5mc19jbGllbnQtPmNs
X2NvbnNfc3RhdGUgdGVzdA0KPiA+IGZvciB3aGV0aGVyIHRoZSBpbml0aWFsaXNhdGlvbiBwcm9j
ZXNzIGlzIGZpbmlzaGVkIGRvZXMgbm90IHJlYWQNCj4gPiBzdGFsZSBkYXRhLg0KPiA+DQo+ID4g
U2lnbmVkLW9mZi1ieTogVHJvbmQgTXlrbGVidXN0PFRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29t
Pg0KPiA+IC0tLQ0KPiA+ICAgZnMvbmZzL2NsaWVudC5jIHwgICAgNSArKysrKw0KPiA+ICAgZnMv
bmZzL2lkbWFwLmMgIHwgICAgMSArDQo+ID4gICAyIGZpbGVzIGNoYW5nZWQsIDYgaW5zZXJ0aW9u
cygrKSwgMCBkZWxldGlvbnMoLSkNCj4gPg0KPiA+IGRpZmYgLS1naXQgYS9mcy9uZnMvY2xpZW50
LmMgYi9mcy9uZnMvY2xpZW50LmMNCj4gPiBpbmRleCBlNjA3MGVhLi40YmU4NWY5IDEwMDY0NA0K
PiA+IC0tLSBhL2ZzL25mcy9jbGllbnQuYw0KPiA+ICsrKyBiL2ZzL25mcy9jbGllbnQuYw0KPiA+
IEBAIC00NTYsNiArNDU2LDggQEAgc3RhdGljIGJvb2wgbmZzNF9jYl9tYXRjaF9jbGllbnQoY29u
c3Qgc3RydWN0IHNvY2thZGRyICphZGRyLA0KPiA+ICAgCSAgICBjbHAtPmNsX2NvbnNfc3RhdGUg
PT0gTkZTX0NTX1NFU1NJT05fSU5JVElORykpDQo+ID4gICAJCXJldHVybiBmYWxzZTsNCj4gPg0K
PiA+ICsJc21wX3JtYigpOw0KPiA+ICsNCj4gPiAgIAkvKiBNYXRjaCB0aGUgdmVyc2lvbiBhbmQg
bWlub3J2ZXJzaW9uICovDQo+ID4gICAJaWYgKGNscC0+cnBjX29wcy0+dmVyc2lvbiAhPSA0IHx8
DQo+ID4gICAJICAgIGNscC0+Y2xfbWlub3J2ZXJzaW9uICE9IG1pbm9ydmVyc2lvbikNCj4gPiBA
QCAtNTg3LDYgKzU4OSw4IEBAIGZvdW5kX2NsaWVudDoNCj4gPiAgIAkJcmV0dXJuIEVSUl9QVFIo
ZXJyb3IpOw0KPiA+ICAgCX0NCj4gPg0KPiA+ICsJc21wX3JtYigpOw0KPiA+ICsNCj4gPiAgIAlC
VUdfT04oY2xwLT5jbF9jb25zX3N0YXRlICE9IE5GU19DU19SRUFEWSk7DQo+ID4NCj4gPiAgIAlk
cHJpbnRrKCItLT4gIG5mc19nZXRfY2xpZW50KCkgPSAlcCBbc2hhcmVdXG4iLCBjbHApOw0KPiA+
IEBAIC01OTgsNiArNjAyLDcgQEAgZm91bmRfY2xpZW50Og0KPiA+ICAgICovDQo+ID4gICB2b2lk
IG5mc19tYXJrX2NsaWVudF9yZWFkeShzdHJ1Y3QgbmZzX2NsaWVudCAqY2xwLCBpbnQgc3RhdGUp
DQo+ID4gICB7DQo+ID4gKwlzbXBfd21iKCk7DQo+IA0KPiBCVFcsIHdoeSBiYXJyaWVyIGlzIGJl
Zm9yZSBhc3NpZ25tZW50Pw0KDQpTbyB0aGF0IHdlIHdyaXRlIHRoZSBpbml0aWFsaXNhdGlvbiBj
aGFuZ2VzIHRvIHRoZSBzdHJ1Y3QgbmZzX2NsaWVudA0KYmVmb3JlIGNoYW5naW5nIHRoZSB2YWx1
ZSBvZiBjbHAtPmNsX2NvbnNfc3RhdGUuIE90aGVyd2lzZSwgYW5vdGhlcg0KcHJvY2Vzc29yIG1p
Z2h0IHNlZSB0aGUgY2hhbmdlIHRvIGNscC0+Y2xfY29uc19zdGF0ZSBiZWZvcmUgdGhleSBzZWUg
dGhlDQpyZXN0IG9mIHRoZSBpbml0aWFsaXNhdGlvbiBvZiBjbHAuDQoNCk5vdGUgdGhhdCB0aGVy
ZSBpcyBhIHNlY29uZCB3cml0ZSBiYXJyaWVyIGluIHRoZSAnd2FrZV91cF9hbGwnIGNhbGwgdG8N
CmVuc3VyZSB0aGF0IHNsZWVwaW5nIHByb2Nlc3NlcyB0aGF0IGFyZSBhY3R1YWxseSB3b2tlbiB1
cCB3aWxsIHNlZSB0aGUNCmNoYW5nZSB0byBjbHAtPmNsX2NvbnNfc3RhdGUuDQoNCj4gPiAgIAlj
bHAtPmNsX2NvbnNfc3RhdGUgPSBzdGF0ZTsNCj4gPiAgIAl3YWtlX3VwX2FsbCgmbmZzX2NsaWVu
dF9hY3RpdmVfd3EpOw0KPiA+ICAgfQ0KPiA+IGRpZmYgLS1naXQgYS9mcy9uZnMvaWRtYXAuYyBi
L2ZzL25mcy9pZG1hcC5jDQo+ID4gaW5kZXggNmNhOTQ5Yi4uN2Q0ZThkZCAxMDA2NDQNCj4gPiAt
LS0gYS9mcy9uZnMvaWRtYXAuYw0KPiA+ICsrKyBiL2ZzL25mcy9pZG1hcC5jDQo+ID4gQEAgLTU0
OCw2ICs1NDgsNyBAQCByZXN0YXJ0Og0KPiA+ICAgCQkvKiBTa2lwIG5mc19jbGllbnRzIHRoYXQg
ZmFpbGVkIHRvIGluaXRpYWxpc2UgKi8NCj4gPiAgIAkJaWYgKGNscC0+Y2xfY29uc19zdGF0ZTwg
IDApDQo+ID4gICAJCQljb250aW51ZTsNCj4gPiArCQlzbXBfcm1iKCk7DQo+ID4gICAJCWlmIChj
bHAtPnJwY19vcHMgIT0mbmZzX3Y0X2NsaWVudG9wcykNCj4gPiAgIAkJCWNvbnRpbnVlOw0KPiA+
ICAgCQljbF9kZW50cnkgPSBjbHAtPmNsX2lkbWFwLT5pZG1hcF9waXBlLT5kZW50cnk7DQo+IA0K
DQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVyDQoNCk5l
dEFwcA0KVHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20NCnd3dy5uZXRhcHAuY29tDQoNCg==

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2012-05-24 11:59 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-23 17:32 [PATCH 1/3] NFSv4.1: Fix session initialisation races Trond Myklebust
2012-05-23 17:32 ` [PATCH 2/3] NFSv4: Fix a race in the net namespace mount notification Trond Myklebust
2012-05-23 17:32   ` [PATCH 3/3] NFS: Add memory barriers to the nfs_client->cl_cons_state initialisation Trond Myklebust
2012-05-24  4:36     ` Stanislav Kinsbursky
2012-05-24 11:59       ` Myklebust, Trond
2012-05-23 18:58   ` [PATCH 2/3] NFSv4: Fix a race in the net namespace mount notification Stanislav Kinsbursky
2012-05-23 18:37 ` [PATCH 1/3] NFSv4.1: Fix session initialisation races Adamson, Andy
2012-05-23 19:14   ` Myklebust, Trond
2012-05-23 19:20     ` Myklebust, Trond
2012-05-23 19:25       ` Adamson, Andy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.