[pnfs] [PATCH 5/12] cleanup nfsd state lock

[J. Bruce Fields]

On Tue, May 15, 2007 at 01:27:57PM -0400, William A. (Andy) Adamson wrote:
> On 5/7/07, Benny Halevy <bhalevy at panasas.com> wrote:
> >
> >
> >
> >From 8ad07658b24efa9a284f9edc38c54c95f80b4bde Mon Sep 17 00:00:00 2001
> >From: Benny Halevy <bhalevy at ns1.bhalevy.com>
> >Date: Mon, 7 May 2007 16:58:42 +0300
> >Subject: [PATCH] cleanup nfsd state lock
> >
> >make sure nfs4 file and client are got and freed
> >under the state lock
> >get... code asserts that the lock is taken (with BUG_ON)
> >free... code just takes the lock if needed
> >---
> >fs/nfsd/nfs4state.c |   19 ++++++++++++++++++-
> >1 files changed, 18 insertions(+), 1 deletions(-)
> >
> >diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> >index d1355b2..71c9ec3 100644
> >--- a/fs/nfsd/nfs4state.c
> >+++ b/fs/nfsd/nfs4state.c
> >@@ -108,6 +108,12 @@ static kmem_cache_t *file_slab = NULL;
> >static kmem_cache_t *stateid_slab = NULL;
> >static kmem_cache_t *deleg_slab = NULL;
> >
> >+static int
> >+nfs4_trylock_state(void)
> >+{
> >+       return mutex_trylock(&client_mutex);
> >+}
> >+
> >void
> >nfs4_lock_state(void)
> >{
> >@@ -120,6 +126,8 @@ nfs4_unlock_state(void)
> >        mutex_unlock(&client_mutex);
> >}
> >
> >+#define BUG_ON_UNLOCKED_STATE() BUG_ON(nfs4_trylock_state());
> >+
> >static inline u32
> >opaque_hashval(const void *ptr, int nbytes)
> >{
> >@@ -148,8 +156,12 @@ static struct list_head del_recall_lru;
> >static void
> >free_nfs4_file(struct kref *kref)
> >{
> >+       int did_lock;
> >        struct nfs4_file *fp = container_of(kref, struct nfs4_file,
> >fi_ref);
> >+       did_lock = nfs4_trylock_state();
> >        list_del(&fp->fi_hash);
> >+       if (did_lock)
> >+               nfs4_unlock_state();
> >        iput(fp->fi_inode);
> >        kmem_cache_free(file_slab, fp);

Also couldn't did_lock get set just because somebody else (not
necessarily the caller) set the lock?

If we really need to be able to call this both under and not under the
lock, I'd rather if possible just provide two variants:

	__free_nfs4_file(struct kref *kref)
	{
		...
	}

	free_nfs4_file(struct kref *kref)
	{
		nfs4_lock_state();
		__free_nfs4_file();
		nfs4_unlock_state();
	}

--b.