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 23/40] nfsd: Add reference counting to state owners
Date: Mon, 21 Jul 2014 11:02:35 -0400 [thread overview]
Message-ID: <1405954972-28904-24-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>
The way stateowners are managed today is somewhat awkward. They need to
be explicitly destroyed, even though the stateids reference them. This
will be particularly problematic when we remove the client_mutex.
We may create a new stateowner and attempt to open a file or set a lock,
and have that fail. In the meantime, another RPC may come in that uses
that same stateowner and succeed. We can't have the first task tearing
down the stateowner in that situation.
To fix this, we need to change how stateowners are tracked altogether.
Refcount them and only destroy them once all stateids that reference
them have been destroyed. This patch starts by adding the refcounting
necessary to do that.
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
Signed-off-by: Jeff Layton <jlayton@primarydata.com>
---
fs/nfsd/nfs4state.c | 43 +++++++++++++++++++++++++++++--------------
fs/nfsd/state.h | 3 +++
2 files changed, 32 insertions(+), 14 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 8f724aa429ed..d96737f72c46 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -72,6 +72,7 @@ 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 void nfs4_put_stateowner(struct nfs4_stateowner *sop);
/* Locking: */
@@ -955,16 +956,10 @@ static void unhash_lockowner(struct nfs4_lockowner *lo)
}
}
-static void nfs4_free_lockowner(struct nfs4_lockowner *lo)
-{
- kfree(lo->lo_owner.so_owner.data);
- kmem_cache_free(lockowner_slab, lo);
-}
-
static void release_lockowner(struct nfs4_lockowner *lo)
{
unhash_lockowner(lo);
- nfs4_free_lockowner(lo);
+ nfs4_put_stateowner(&lo->lo_owner);
}
static void release_lockowner_if_empty(struct nfs4_lockowner *lo)
@@ -1034,18 +1029,12 @@ static void release_last_closed_stateid(struct nfs4_openowner *oo)
}
}
-static void nfs4_free_openowner(struct nfs4_openowner *oo)
-{
- kfree(oo->oo_owner.so_owner.data);
- kmem_cache_free(openowner_slab, oo);
-}
-
static void release_openowner(struct nfs4_openowner *oo)
{
unhash_openowner(oo);
list_del(&oo->oo_close_lru);
release_last_closed_stateid(oo);
- nfs4_free_openowner(oo);
+ nfs4_put_stateowner(&oo->oo_owner);
}
static inline int
@@ -2979,9 +2968,17 @@ static inline void *alloc_stateowner(struct kmem_cache *slab, struct xdr_netobj
INIT_LIST_HEAD(&sop->so_stateids);
sop->so_client = clp;
init_nfs4_replay(&sop->so_replay);
+ atomic_set(&sop->so_count, 1);
return sop;
}
+static void nfs4_put_stateowner(struct nfs4_stateowner *sop)
+{
+ if (!atomic_dec_and_test(&sop->so_count))
+ return;
+ sop->so_free(sop);
+}
+
static void hash_openowner(struct nfs4_openowner *oo, struct nfs4_client *clp, unsigned int strhashval)
{
struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
@@ -2990,6 +2987,14 @@ static void hash_openowner(struct nfs4_openowner *oo, struct nfs4_client *clp, u
list_add(&oo->oo_perclient, &clp->cl_openowners);
}
+static void nfs4_free_openowner(struct nfs4_stateowner *so)
+{
+ struct nfs4_openowner *oo = openowner(so);
+
+ kfree(oo->oo_owner.so_owner.data);
+ kmem_cache_free(openowner_slab, oo);
+}
+
static struct nfs4_openowner *
alloc_init_open_stateowner(unsigned int strhashval, struct nfsd4_open *open,
struct nfsd4_compound_state *cstate)
@@ -3000,6 +3005,7 @@ alloc_init_open_stateowner(unsigned int strhashval, struct nfsd4_open *open,
oo = alloc_stateowner(openowner_slab, &open->op_owner, clp);
if (!oo)
return NULL;
+ oo->oo_owner.so_free = nfs4_free_openowner;
oo->oo_owner.so_is_open_owner = 1;
oo->oo_owner.so_seqid = open->op_seqid;
oo->oo_flags = NFS4_OO_NEW;
@@ -4746,6 +4752,14 @@ find_lockowner_str(clientid_t *clid, struct xdr_netobj *owner,
return NULL;
}
+static void nfs4_free_lockowner(struct nfs4_stateowner *sop)
+{
+ struct nfs4_lockowner *lo = lockowner(sop);
+
+ kfree(lo->lo_owner.so_owner.data);
+ kmem_cache_free(lockowner_slab, lo);
+}
+
/*
* Alloc a lock owner structure.
* Called in nfsd4_lock - therefore, OPEN and OPEN_CONFIRM (if needed) has
@@ -4766,6 +4780,7 @@ alloc_init_lock_stateowner(unsigned int strhashval, struct nfs4_client *clp, str
/* It is the openowner seqid that will be incremented in encode in the
* case of new lockowners; so increment the lock seqid manually: */
lo->lo_owner.so_seqid = lock->lk_new_lock_seqid + 1;
+ lo->lo_owner.so_free = nfs4_free_lockowner;
list_add(&lo->lo_owner.so_strhash, &nn->ownerstr_hashtbl[strhashval]);
return lo;
}
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 5c30e6064749..3cb7283bc42a 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -339,10 +339,13 @@ struct nfs4_stateowner {
struct nfs4_client * so_client;
/* after increment in ENCODE_SEQID_OP_TAIL, represents the next
* sequence id expected from the client: */
+ atomic_t so_count;
u32 so_seqid;
struct xdr_netobj so_owner; /* open owner name */
struct nfs4_replay so_replay;
bool so_is_open_owner;
+
+ void (*so_free)(struct nfs4_stateowner *);
};
struct nfs4_openowner {
--
1.9.3
next prev 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 ` Jeff Layton [this message]
2014-07-27 13:58 ` [PATCH 23/40] nfsd: Add reference counting to state owners 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 ` [PATCH 32/40] nfsd: Protect adding/removing open state owners using client_lock Jeff Layton
2014-07-21 15:02 ` [PATCH 33/40] nfsd: Protect adding/removing 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-24-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).