linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.com>
To: "J. Bruce Fields" <bfields@fieldses.org>,
	Chuck Lever <chuck.lever@oracle.com>,
	Jeff Layton <jlayton@kernel.org>,
	Trond Myklebust <trond.myklebust@hammerspace.com>,
	Anna Schumaker <anna.schumaker@netapp.com>
Cc: Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 10/23] NFSv4: don't require lock for get_renew_cred or get_machine_cred
Date: Wed, 07 Nov 2018 15:12:31 +1100	[thread overview]
Message-ID: <154156395108.24086.10463547105269385122.stgit@noble> (raw)
In-Reply-To: <154156285766.24086.14262073575778354276.stgit@noble>

This lock is no longer necessary.

If nfs4_get_renew_cred() needs to hunt through the open-state
creds for a user cred, it still takes the lock to stablize
the rbtree, but otherwise there are no races.

Note that this completely removes the lock from nfs4_renew_state().
It appears that the original need for the locking here was removed
long ago, and there is no longer anything to protect.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 fs/nfs/nfs4_fs.h    |    6 +++---
 fs/nfs/nfs4proc.c   |    4 ++--
 fs/nfs/nfs4renewd.c |    5 +----
 fs/nfs/nfs4state.c  |   26 ++++++++++----------------
 4 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 8d59c9655ec4..0f393eb6b4e4 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -248,7 +248,7 @@ struct nfs4_add_xprt_data {
 
 struct nfs4_state_maintenance_ops {
 	int (*sched_state_renewal)(struct nfs_client *, struct rpc_cred *, unsigned);
-	struct rpc_cred * (*get_state_renewal_cred_locked)(struct nfs_client *);
+	struct rpc_cred * (*get_state_renewal_cred)(struct nfs_client *);
 	int (*renew_lease)(struct nfs_client *, struct rpc_cred *);
 };
 
@@ -449,8 +449,8 @@ extern void nfs4_set_lease_period(struct nfs_client *clp,
 
 /* nfs4state.c */
 struct rpc_cred *nfs4_get_clid_cred(struct nfs_client *clp);
-struct rpc_cred *nfs4_get_machine_cred_locked(struct nfs_client *clp);
-struct rpc_cred *nfs4_get_renew_cred_locked(struct nfs_client *clp);
+struct rpc_cred *nfs4_get_machine_cred(struct nfs_client *clp);
+struct rpc_cred *nfs4_get_renew_cred(struct nfs_client *clp);
 int nfs4_discover_server_trunking(struct nfs_client *clp,
 			struct nfs_client **);
 int nfs40_discover_server_trunking(struct nfs_client *clp,
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 867457d6dfbe..cafa155a053e 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -9606,14 +9606,14 @@ static const struct nfs4_state_recovery_ops nfs41_nograce_recovery_ops = {
 
 static const struct nfs4_state_maintenance_ops nfs40_state_renewal_ops = {
 	.sched_state_renewal = nfs4_proc_async_renew,
-	.get_state_renewal_cred_locked = nfs4_get_renew_cred_locked,
+	.get_state_renewal_cred = nfs4_get_renew_cred,
 	.renew_lease = nfs4_proc_renew,
 };
 
 #if defined(CONFIG_NFS_V4_1)
 static const struct nfs4_state_maintenance_ops nfs41_state_renewal_ops = {
 	.sched_state_renewal = nfs41_proc_async_sequence,
-	.get_state_renewal_cred_locked = nfs4_get_machine_cred_locked,
+	.get_state_renewal_cred = nfs4_get_machine_cred,
 	.renew_lease = nfs4_proc_sequence,
 };
 #endif
diff --git a/fs/nfs/nfs4renewd.c b/fs/nfs/nfs4renewd.c
index 1f8c2ae43a8d..8880cd958210 100644
--- a/fs/nfs/nfs4renewd.c
+++ b/fs/nfs/nfs4renewd.c
@@ -68,7 +68,6 @@ nfs4_renew_state(struct work_struct *work)
 	if (test_bit(NFS_CS_STOP_RENEW, &clp->cl_res_state))
 		goto out;
 
-	spin_lock(&clp->cl_lock);
 	lease = clp->cl_lease_time;
 	last = clp->cl_last_renewal;
 	now = jiffies;
@@ -79,8 +78,7 @@ nfs4_renew_state(struct work_struct *work)
 		renew_flags |= NFS4_RENEW_DELEGATION_CB;
 
 	if (renew_flags != 0) {
-		cred = ops->get_state_renewal_cred_locked(clp);
-		spin_unlock(&clp->cl_lock);
+		cred = ops->get_state_renewal_cred(clp);
 		if (cred == NULL) {
 			if (!(renew_flags & NFS4_RENEW_DELEGATION_CB)) {
 				set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state);
@@ -104,7 +102,6 @@ nfs4_renew_state(struct work_struct *work)
 	} else {
 		dprintk("%s: failed to call renewd. Reason: lease not expired \n",
 				__func__);
-		spin_unlock(&clp->cl_lock);
 	}
 	nfs4_schedule_state_renewal(clp);
 out_exp:
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 032d3ec929f3..dde17424c4f4 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -164,7 +164,7 @@ int nfs40_discover_server_trunking(struct nfs_client *clp,
 	return status;
 }
 
-struct rpc_cred *nfs4_get_machine_cred_locked(struct nfs_client *clp)
+struct rpc_cred *nfs4_get_machine_cred(struct nfs_client *clp)
 {
 	struct rpc_cred *cred = clp->cl_root_cred;
 
@@ -210,22 +210,23 @@ nfs4_get_renew_cred_server_locked(struct nfs_server *server)
 }
 
 /**
- * nfs4_get_renew_cred_locked - Acquire credential for a renew operation
+ * nfs4_get_renew_cred - Acquire credential for a renew operation
  * @clp: client state handle
  *
  * Returns an rpc_cred with reference count bumped, or NULL.
  * Caller must hold clp->cl_lock.
  */
-struct rpc_cred *nfs4_get_renew_cred_locked(struct nfs_client *clp)
+struct rpc_cred *nfs4_get_renew_cred(struct nfs_client *clp)
 {
 	struct rpc_cred *cred = NULL;
 	struct nfs_server *server;
 
 	/* Use machine credentials if available */
-	cred = nfs4_get_machine_cred_locked(clp);
+	cred = nfs4_get_machine_cred(clp);
 	if (cred != NULL)
 		goto out;
 
+	spin_lock(&clp->cl_lock);
 	rcu_read_lock();
 	list_for_each_entry_rcu(server, &clp->cl_superblocks, client_link) {
 		cred = nfs4_get_renew_cred_server_locked(server);
@@ -233,6 +234,7 @@ struct rpc_cred *nfs4_get_renew_cred_locked(struct nfs_client *clp)
 			break;
 	}
 	rcu_read_unlock();
+	spin_unlock(&clp->cl_lock);
 
 out:
 	return cred;
@@ -402,9 +404,7 @@ struct rpc_cred *nfs4_get_clid_cred(struct nfs_client *clp)
 {
 	struct rpc_cred *cred;
 
-	spin_lock(&clp->cl_lock);
-	cred = nfs4_get_machine_cred_locked(clp);
-	spin_unlock(&clp->cl_lock);
+	cred = nfs4_get_machine_cred(clp);
 	return cred;
 }
 
@@ -1907,9 +1907,7 @@ static int nfs4_check_lease(struct nfs_client *clp)
 	/* Is the client already known to have an expired lease? */
 	if (test_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state))
 		return 0;
-	spin_lock(&clp->cl_lock);
-	cred = ops->get_state_renewal_cred_locked(clp);
-	spin_unlock(&clp->cl_lock);
+	cred = ops->get_state_renewal_cred(clp);
 	if (cred == NULL) {
 		cred = nfs4_get_clid_cred(clp);
 		status = -ENOKEY;
@@ -2111,9 +2109,7 @@ static int nfs4_handle_migration(struct nfs_client *clp)
 	dprintk("%s: migration reported on \"%s\"\n", __func__,
 			clp->cl_hostname);
 
-	spin_lock(&clp->cl_lock);
-	cred = ops->get_state_renewal_cred_locked(clp);
-	spin_unlock(&clp->cl_lock);
+	cred = ops->get_state_renewal_cred(clp);
 	if (cred == NULL)
 		return -NFS4ERR_NOENT;
 
@@ -2159,9 +2155,7 @@ static int nfs4_handle_lease_moved(struct nfs_client *clp)
 	dprintk("%s: lease moved reported on \"%s\"\n", __func__,
 			clp->cl_hostname);
 
-	spin_lock(&clp->cl_lock);
-	cred = ops->get_state_renewal_cred_locked(clp);
-	spin_unlock(&clp->cl_lock);
+	cred = ops->get_state_renewal_cred(clp);
 	if (cred == NULL)
 		return -NFS4ERR_NOENT;
 



  parent reply	other threads:[~2018-11-07  4:14 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-07  4:12 [PATCH 00/23 - V4] NFS: Remove generic RPC credentials NeilBrown
2018-11-07  4:12 ` [PATCH 06/23] SUNRPC: remove groupinfo from struct auth_cred NeilBrown
2018-11-07  4:12 ` [PATCH 03/23] cred: export get_task_cred() NeilBrown
2018-11-07  4:12 ` [PATCH 04/23] cred: allow get_cred() and put_cred() to be given NULL NeilBrown
2018-11-07  4:12 ` [PATCH 05/23] SUNRPC: add 'struct cred *' to auth_cred and rpc_cred NeilBrown
2018-11-07  4:12 ` [PATCH 01/23] cred: add cred_fscmp() for comparing creds NeilBrown
2018-11-07  4:12 ` [PATCH 02/23] cred: add get_cred_rcu() NeilBrown
2018-11-07  4:12 ` [PATCH 21/23] SUNRPC: remove crbind rpc_cred operation NeilBrown
2018-11-07  4:12 ` [PATCH 11/23] SUNRPC: discard RPC_DO_ROOTOVERRIDE() NeilBrown
2018-11-07  4:12 ` [PATCH 22/23] SUNRPC: simplify auth_unix NeilBrown
2018-11-07 15:19   ` Chuck Lever
2018-11-08  1:41     ` NeilBrown
2018-11-08 15:54       ` Chuck Lever
2018-11-09  0:45         ` NeilBrown
2018-11-07  4:12 ` [PATCH 18/23] NFS: struct nfs_open_dir_context: convert rpc_cred pointer to cred NeilBrown
2018-11-07  4:12 ` [PATCH 20/23] SUNRPC: remove generic cred code NeilBrown
2018-11-07  4:12 ` [PATCH 23/23] SUNRPC discard cr_uid from struct rpc_cred NeilBrown
2018-11-07  4:12 ` NeilBrown [this message]
2018-11-07  4:12 ` [PATCH 19/23] NFS/NFSD/SUNRPC: replace generic creds with 'struct cred' NeilBrown
2018-11-07  4:12 ` [PATCH 12/23] NFS/SUNRPC: don't lookup machine credential until rpcauth_bindcred() NeilBrown
2018-11-07  4:12 ` [PATCH 17/23] NFS: change access cache to use 'struct cred' NeilBrown
2018-11-07  4:12 ` [PATCH 07/23] SUNRPC: remove uid and gid from struct auth_cred NeilBrown
2018-11-07  4:12 ` [PATCH 16/23] SUNRPC: remove RPCAUTH_AUTH_NO_CRKEY_TIMEOUT NeilBrown
2018-11-07  4:12 ` [PATCH 09/23] NFSv4: add cl_root_cred for use when machine cred is not available NeilBrown
2018-11-07  4:12 ` [PATCH 14/23] SUNRPC: add side channel to use non-generic cred for rpc call NeilBrown
2018-11-07  4:12 ` [PATCH 13/23] SUNRPC: introduce RPC_TASK_NULLCREDS to request auth_none NeilBrown
2018-11-07  4:12 ` [PATCH 08/23] SUNRPC: remove machine_cred field from struct auth_cred NeilBrown
2018-11-07  4:12 ` [PATCH 15/23] NFS: move credential expiry tracking out of SUNRPC into NFS NeilBrown
2018-11-29 23:19 ` [PATCH 00/23 - V4] NFS: Remove generic RPC credentials NeilBrown
2018-11-30 19:39   ` Schumaker, Anna
2018-12-03  0:30 [PATCH 00/23 - V5] " NeilBrown
2018-12-03  0:30 ` [PATCH 10/23] NFSv4: don't require lock for get_renew_cred or get_machine_cred NeilBrown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=154156395108.24086.10463547105269385122.stgit@noble \
    --to=neilb@suse.com \
    --cc=anna.schumaker@netapp.com \
    --cc=bfields@fieldses.org \
    --cc=chuck.lever@oracle.com \
    --cc=jlayton@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trond.myklebust@hammerspace.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).