linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@primarydata.com>
To: bfields@fieldses.org
Cc: linux-nfs@vger.kernel.org, hch@infradead.org,
	Trond Myklebust <trond.myklebust@primarydata.com>
Subject: [PATCH 32/40] nfsd: Protect adding/removing open state owners using client_lock
Date: Mon, 21 Jul 2014 11:02:44 -0400	[thread overview]
Message-ID: <1405954972-28904-33-git-send-email-jlayton@primarydata.com> (raw)
In-Reply-To: <1405954972-28904-1-git-send-email-jlayton@primarydata.com>

From: Trond Myklebust <trond.myklebust@primarydata.com>

Once we remove client mutex protection, we'll need to ensure that
stateowner lookup and creation are atomic between concurrent compounds.
Ensure that alloc_init_open_stateowner checks the hashtable under the
client_lock before adding a new element.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfsd/nfs4state.c | 63 +++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 54 insertions(+), 9 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 0cf457d23e64..a3fae742315d 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -72,6 +72,9 @@ static u64 current_sessionid = 1;
 /* forward declarations */
 static int check_for_locks(struct nfs4_file *filp, struct nfs4_lockowner *lowner);
 static void nfs4_free_generic_stateid(struct nfs4_stid *stid);
+static struct nfs4_openowner *find_openstateowner_str_locked(
+		unsigned int hashval, struct nfsd4_open *open,
+		bool sessions, struct nfsd_net *nn);
 static void nfs4_put_stateowner(struct nfs4_stateowner *sop);
 
 /* Locking: */
@@ -1006,8 +1009,13 @@ static void release_open_stateid(struct nfs4_ol_stateid *stp)
 	put_generic_stateid(stp);
 }
 
-static void unhash_openowner(struct nfs4_openowner *oo)
+static void unhash_openowner_locked(struct nfs4_openowner *oo)
 {
+	struct nfsd_net *nn = net_generic(oo->oo_owner.so_client->net,
+						nfsd_net_id);
+
+	lockdep_assert_held(&nn->client_lock);
+
 	list_del_init(&oo->oo_owner.so_strhash);
 	list_del_init(&oo->oo_perclient);
 }
@@ -1026,18 +1034,29 @@ static void release_last_closed_stateid(struct nfs4_openowner *oo)
 static void release_openowner_stateids(struct nfs4_openowner *oo)
 {
 	struct nfs4_ol_stateid *stp;
+	struct nfsd_net *nn = net_generic(oo->oo_owner.so_client->net,
+						nfsd_net_id);
+
+	lockdep_assert_held(&nn->client_lock);
 
 	while (!list_empty(&oo->oo_owner.so_stateids)) {
 		stp = list_first_entry(&oo->oo_owner.so_stateids,
 				struct nfs4_ol_stateid, st_perstateowner);
+		spin_unlock(&nn->client_lock);
 		release_open_stateid(stp);
+		spin_lock(&nn->client_lock);
 	}
 }
 
 static void release_openowner(struct nfs4_openowner *oo)
 {
-	unhash_openowner(oo);
+	struct nfsd_net *nn = net_generic(oo->oo_owner.so_client->net,
+						nfsd_net_id);
+
+	spin_lock(&nn->client_lock);
+	unhash_openowner_locked(oo);
 	release_openowner_stateids(oo);
+	spin_unlock(&nn->client_lock);
 	release_last_closed_stateid(oo);
 	nfs4_put_stateowner(&oo->oo_owner);
 }
@@ -3019,8 +3038,11 @@ static void hash_openowner(struct nfs4_openowner *oo, struct nfs4_client *clp, u
 static void nfs4_unhash_openowner(struct nfs4_stateowner *so)
 {
 	struct nfs4_openowner *oo = openowner(so);
+	struct nfsd_net *nn = net_generic(so->so_client->net, nfsd_net_id);
 
-	unhash_openowner(oo);
+	spin_lock(&nn->client_lock);
+	unhash_openowner_locked(oo);
+	spin_unlock(&nn->client_lock);
 }
 
 static void nfs4_free_openowner(struct nfs4_stateowner *so)
@@ -3036,7 +3058,8 @@ alloc_init_open_stateowner(unsigned int strhashval, struct nfsd4_open *open,
 			   struct nfsd4_compound_state *cstate)
 {
 	struct nfs4_client *clp = cstate->clp;
-	struct nfs4_openowner *oo;
+	struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
+	struct nfs4_openowner *oo, *ret;
 
 	oo = alloc_stateowner(openowner_slab, &open->op_owner, clp);
 	if (!oo)
@@ -3051,7 +3074,15 @@ alloc_init_open_stateowner(unsigned int strhashval, struct nfsd4_open *open,
 	oo->oo_time = 0;
 	oo->oo_last_closed_stid = NULL;
 	INIT_LIST_HEAD(&oo->oo_close_lru);
-	hash_openowner(oo, clp, strhashval);
+	spin_lock(&nn->client_lock);
+	ret = find_openstateowner_str_locked(strhashval,
+			open, clp->cl_minorversion, nn);
+	if (ret == NULL) {
+		hash_openowner(oo, clp, strhashval);
+		ret = oo;
+	} else
+		nfs4_free_openowner(&oo->oo_owner);
+	spin_unlock(&nn->client_lock);
 	return oo;
 }
 
@@ -3125,13 +3156,15 @@ same_owner_str(struct nfs4_stateowner *sop, struct xdr_netobj *owner,
 }
 
 static struct nfs4_openowner *
-find_openstateowner_str(unsigned int hashval, struct nfsd4_open *open,
+find_openstateowner_str_locked(unsigned int hashval, struct nfsd4_open *open,
 			bool sessions, struct nfsd_net *nn)
 {
 	struct nfs4_stateowner *so;
 	struct nfs4_openowner *oo;
 	struct nfs4_client *clp;
 
+	lockdep_assert_held(&nn->client_lock);
+
 	list_for_each_entry(so, &nn->ownerstr_hashtbl[hashval], so_strhash) {
 		if (!so->so_is_open_owner)
 			continue;
@@ -3139,15 +3172,27 @@ find_openstateowner_str(unsigned int hashval, struct nfsd4_open *open,
 			oo = openowner(so);
 			clp = oo->oo_owner.so_client;
 			if ((bool)clp->cl_minorversion != sessions)
-				return NULL;
-			renew_client(oo->oo_owner.so_client);
-			atomic_inc(&oo->oo_owner.so_count);
+				break;
+			renew_client_locked(clp);
+			atomic_inc(&so->so_count);
 			return oo;
 		}
 	}
 	return NULL;
 }
 
+static struct nfs4_openowner *
+find_openstateowner_str(unsigned int hashval, struct nfsd4_open *open,
+			bool sessions, struct nfsd_net *nn)
+{
+	struct nfs4_openowner *oo;
+
+	spin_lock(&nn->client_lock);
+	oo = find_openstateowner_str_locked(hashval, open, sessions, nn);
+	spin_unlock(&nn->client_lock);
+	return oo;
+}
+
 /* search file_hashtbl[] for file */
 static struct nfs4_file *
 find_file_locked(struct inode *ino)
-- 
1.9.3


  parent reply	other threads:[~2014-07-21 15:03 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-21 15:02 [PATCH 00/40] nfsd: stateid and stateowner refcounting overhaul Jeff Layton
2014-07-21 15:02 ` [PATCH 01/40] nfsd4: use cl_lock to synchronize all stateid idr calls Jeff Layton
2014-07-27 13:21   ` Christoph Hellwig
2014-07-21 15:02 ` [PATCH 02/40] nfsd: Add reference counting to the lock and open stateids Jeff Layton
2014-07-27 13:23   ` Christoph Hellwig
2014-07-21 15:02 ` [PATCH 03/40] nfsd: Add a struct nfs4_file field to struct nfs4_stid Jeff Layton
2014-07-27 13:24   ` Christoph Hellwig
2014-07-21 15:02 ` [PATCH 04/40] nfsd: Replace nfs4_ol_stateid->st_file with the st_stid.sc_file Jeff Layton
2014-07-27 13:25   ` Christoph Hellwig
2014-07-21 15:02 ` [PATCH 05/40] nfsd: Ensure atomicity of stateid destruction and idr tree removal Jeff Layton
2014-07-27 13:28   ` Christoph Hellwig
2014-07-21 15:02 ` [PATCH 06/40] nfsd: Cleanup the freeing of stateids Jeff Layton
2014-07-27 13:35   ` Christoph Hellwig
2014-07-21 15:02 ` [PATCH 07/40] nfsd: do filp_close in sc_free callback for lock stateids Jeff Layton
2014-07-27 13:37   ` Christoph Hellwig
2014-07-21 15:02 ` [PATCH 08/40] nfsd: Add locking to protect the state owner lists Jeff Layton
2014-07-27 13:42   ` Christoph Hellwig
2014-07-29 11:42     ` Jeff Layton
2014-07-21 15:02 ` [PATCH 09/40] nfsd: clean up races in lock stateid searching and creation Jeff Layton
2014-07-27 13:44   ` Christoph Hellwig
2014-07-21 15:02 ` [PATCH 10/40] nfsd: Slight cleanup of find_stateid() Jeff Layton
2014-07-27 13:38   ` Christoph Hellwig
2014-07-21 15:02 ` [PATCH 11/40] nfsd: ensure atomicity in nfsd4_free_stateid and nfsd4_validate_stateid Jeff Layton
2014-07-27 13:46   ` Christoph Hellwig
2014-07-21 15:02 ` [PATCH 12/40] nfsd: Add reference counting to lock stateids Jeff Layton
2014-07-21 15:02 ` [PATCH 13/40] nfsd: nfsd4_locku() must reference the lock stateid Jeff Layton
2014-07-21 15:02 ` [PATCH 14/40] nfsd: Ensure that nfs4_open_delegation() references the delegation stateid Jeff Layton
2014-07-21 15:02 ` [PATCH 15/40] nfsd: nfsd4_process_open2() must reference " Jeff Layton
2014-07-21 15:02 ` [PATCH 16/40] nfsd: nfsd4_process_open2() must reference the open stateid Jeff Layton
2014-07-21 15:02 ` [PATCH 17/40] nfsd: Prepare nfsd4_close() for open stateid referencing Jeff Layton
2014-07-21 15:02 ` [PATCH 18/40] nfsd: nfsd4_open_confirm() must reference the open stateid Jeff Layton
2014-07-21 15:02 ` [PATCH 19/40] nfsd: Add reference counting to nfs4_preprocess_confirmed_seqid_op Jeff Layton
2014-07-21 15:02 ` [PATCH 20/40] nfsd: Migrate the stateid reference into nfs4_preprocess_seqid_op Jeff Layton
2014-07-21 15:02 ` [PATCH 21/40] nfsd: Migrate the stateid reference into nfs4_lookup_stateid() Jeff Layton
2014-07-21 15:02 ` [PATCH 22/40] nfsd: Migrate the stateid reference into nfs4_find_stateid_by_type() Jeff Layton
2014-07-21 15:02 ` [PATCH 23/40] nfsd: Add reference counting to state owners Jeff Layton
2014-07-27 13:58   ` Christoph Hellwig
2014-07-21 15:02 ` [PATCH 24/40] nfsd: Add a mutex to protect the NFSv4.0 open owner replay cache Jeff Layton
2014-07-21 15:02 ` [PATCH 25/40] nfsd: Keep a reference to the open stateid for the NFSv4.0 " Jeff Layton
2014-07-27 13:59   ` Christoph Hellwig
2014-07-21 15:02 ` [PATCH 26/40] nfsd: clean up lockowner refcounting when finding them Jeff Layton
2014-07-21 15:02 ` [PATCH 27/40] nfsd: add an operation for unhashing a stateowner Jeff Layton
2014-07-21 15:02 ` [PATCH 28/40] nfsd: Make lock stateid take a reference to the lockowner Jeff Layton
2014-07-21 15:02 ` [PATCH 29/40] nfsd: clean up refcounting for lockowners Jeff Layton
2014-07-21 15:02 ` [PATCH 30/40] nfsd: make openstateids hold references to their openowners Jeff Layton
2014-07-21 15:02 ` [PATCH 31/40] nfsd: don't allow CLOSE to proceed until refcount on stateid drops Jeff Layton
2014-07-21 15:02 ` Jeff Layton [this message]
2014-07-21 15:02 ` [PATCH 33/40] nfsd: Protect adding/removing lock owners using client_lock Jeff Layton
2014-07-21 15:02 ` [PATCH 34/40] nfsd: Move the open owner hash table into struct nfs4_client Jeff Layton
2014-07-21 15:02 ` [PATCH 35/40] nfsd: clean up and reorganize release_lockowner Jeff Layton
2014-07-21 15:02 ` [PATCH 36/40] nfsd: add locking to stateowner release Jeff Layton
2014-07-21 15:02 ` [PATCH 37/40] nfsd: optimize destroy_lockowner cl_lock thrashing Jeff Layton
2014-07-21 15:02 ` [PATCH 38/40] nfsd: close potential race in nfsd4_free_stateid Jeff Layton
2014-07-21 15:02 ` [PATCH 39/40] nfsd: reduce cl_lock thrashing in release_openowner Jeff Layton
2014-07-21 15:02 ` [PATCH 40/40] nfsd: don't thrash the cl_lock while freeing an open stateid Jeff Layton

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=1405954972-28904-33-git-send-email-jlayton@primarydata.com \
    --to=jlayton@primarydata.com \
    --cc=bfields@fieldses.org \
    --cc=hch@infradead.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trond.myklebust@primarydata.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).