All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] nfsd: move fsnotify on client creation outside spinlock
@ 2021-06-02 20:06 J. Bruce Fields
  2021-06-03  0:10 ` NeilBrown
  0 siblings, 1 reply; 2+ messages in thread
From: J. Bruce Fields @ 2021-06-02 20:06 UTC (permalink / raw)
  To: linux-nfs

From: "J. Bruce Fields" <bfields@redhat.com>

This was causing a "sleeping function called from invalid context"
warning.

I don't think we need the set_and_test_bit() here; clients move from
unconfirmed to confirmed only once, under the client_lock.

The (conf == unconf) is a way to check whether we're in that confirming
case, hopefully that's not too obscure.

Fixes: 472d155a0631 "nfsd: report client confirmation status in "info" file"
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 99dfc668f605..c01ecb7b3fd3 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2824,11 +2824,8 @@ move_to_confirmed(struct nfs4_client *clp)
 	list_move(&clp->cl_idhash, &nn->conf_id_hashtbl[idhashval]);
 	rb_erase(&clp->cl_namenode, &nn->unconf_name_tree);
 	add_clp_to_name_tree(clp, &nn->conf_name_tree);
-	if (!test_and_set_bit(NFSD4_CLIENT_CONFIRMED, &clp->cl_flags)) {
-		trace_nfsd_clid_confirmed(&clp->cl_clientid);
-		if (clp->cl_nfsd_dentry && clp->cl_nfsd_info_dentry)
-			fsnotify_dentry(clp->cl_nfsd_info_dentry, FS_MODIFY);
-	}
+	set_bit(NFSD4_CLIENT_CONFIRMED, &clp->cl_flags);
+	trace_nfsd_clid_confirmed(&clp->cl_clientid);
 	renew_client_locked(clp);
 }
 
@@ -3487,6 +3484,8 @@ nfsd4_create_session(struct svc_rqst *rqstp,
 	/* cache solo and embedded create sessions under the client_lock */
 	nfsd4_cache_create_session(cr_ses, cs_slot, status);
 	spin_unlock(&nn->client_lock);
+	if (conf == unconf)
+		fsnotify_dentry(conf->cl_nfsd_info_dentry, FS_MODIFY);
 	/* init connection and backchannel */
 	nfsd4_init_conn(rqstp, conn, new);
 	nfsd4_put_session(new);
@@ -4095,6 +4094,8 @@ nfsd4_setclientid_confirm(struct svc_rqst *rqstp,
 	}
 	get_client_locked(conf);
 	spin_unlock(&nn->client_lock);
+	if (conf == unconf)
+		fsnotify_dentry(conf->cl_nfsd_info_dentry, FS_MODIFY);
 	nfsd4_probe_callback(conf);
 	spin_lock(&nn->client_lock);
 	put_client_renew_locked(conf);
-- 
2.31.1


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

* Re: [PATCH] nfsd: move fsnotify on client creation outside spinlock
  2021-06-02 20:06 [PATCH] nfsd: move fsnotify on client creation outside spinlock J. Bruce Fields
@ 2021-06-03  0:10 ` NeilBrown
  0 siblings, 0 replies; 2+ messages in thread
From: NeilBrown @ 2021-06-03  0:10 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: linux-nfs

On Thu, 03 Jun 2021, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <bfields@redhat.com>
> 
> This was causing a "sleeping function called from invalid context"
> warning.
> 
> I don't think we need the set_and_test_bit() here; clients move from
> unconfirmed to confirmed only once, under the client_lock.
> 
> The (conf == unconf) is a way to check whether we're in that confirming
> case, hopefully that's not too obscure.

It is a bit obscure, but I cannot see a cleaner way as it isn't too hard
to work out what the test means by looking back at the code.  So the
only concern is that some later change might cause the test to subtly
change meaning.  Probably not a big concern.

Anyway, thanks for fixing this up.

NeilBrown


> 
> Fixes: 472d155a0631 "nfsd: report client confirmation status in "info" file"
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> ---
>  fs/nfsd/nfs4state.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 99dfc668f605..c01ecb7b3fd3 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -2824,11 +2824,8 @@ move_to_confirmed(struct nfs4_client *clp)
>  	list_move(&clp->cl_idhash, &nn->conf_id_hashtbl[idhashval]);
>  	rb_erase(&clp->cl_namenode, &nn->unconf_name_tree);
>  	add_clp_to_name_tree(clp, &nn->conf_name_tree);
> -	if (!test_and_set_bit(NFSD4_CLIENT_CONFIRMED, &clp->cl_flags)) {
> -		trace_nfsd_clid_confirmed(&clp->cl_clientid);
> -		if (clp->cl_nfsd_dentry && clp->cl_nfsd_info_dentry)
> -			fsnotify_dentry(clp->cl_nfsd_info_dentry, FS_MODIFY);
> -	}
> +	set_bit(NFSD4_CLIENT_CONFIRMED, &clp->cl_flags);
> +	trace_nfsd_clid_confirmed(&clp->cl_clientid);
>  	renew_client_locked(clp);
>  }
>  
> @@ -3487,6 +3484,8 @@ nfsd4_create_session(struct svc_rqst *rqstp,
>  	/* cache solo and embedded create sessions under the client_lock */
>  	nfsd4_cache_create_session(cr_ses, cs_slot, status);
>  	spin_unlock(&nn->client_lock);
> +	if (conf == unconf)
> +		fsnotify_dentry(conf->cl_nfsd_info_dentry, FS_MODIFY);
>  	/* init connection and backchannel */
>  	nfsd4_init_conn(rqstp, conn, new);
>  	nfsd4_put_session(new);
> @@ -4095,6 +4094,8 @@ nfsd4_setclientid_confirm(struct svc_rqst *rqstp,
>  	}
>  	get_client_locked(conf);
>  	spin_unlock(&nn->client_lock);
> +	if (conf == unconf)
> +		fsnotify_dentry(conf->cl_nfsd_info_dentry, FS_MODIFY);
>  	nfsd4_probe_callback(conf);
>  	spin_lock(&nn->client_lock);
>  	put_client_renew_locked(conf);
> -- 
> 2.31.1
> 
> 

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

end of thread, other threads:[~2021-06-03  0:10 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-02 20:06 [PATCH] nfsd: move fsnotify on client creation outside spinlock J. Bruce Fields
2021-06-03  0:10 ` NeilBrown

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.