diff mbox series

nfsd: move fsnotify on client creation outside spinlock

Message ID 20210602200639.GC6995@fieldses.org (mailing list archive)
State New
Headers show
Series nfsd: move fsnotify on client creation outside spinlock | expand

Commit Message

Bruce Fields June 2, 2021, 8:06 p.m. UTC
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(-)

Comments

NeilBrown June 3, 2021, 12:10 a.m. UTC | #1
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
> 
>
diff mbox series

Patch

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