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

[William A. (Andy) Adamson]

Hi Benny

oops!    I really meant to ask when free_nfs4_file() is called other than
from the put_nfs4_file. I think free_nfs4_file should only be called via
put_nfs4_file, which means you don't need to call nfs4_trylock_state in
free_nfs4_file() due to the BUG_ON_UNLOCKED_STATE() call in put_nfs4_file.

-->Andy

On 5/15/07, William A. (Andy) Adamson <andros at citi.umich.edu> 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);
> > }
> > @@ -157,12 +169,15 @@ free_nfs4_file(struct kref *kref)
> > static inline void
> > put_nfs4_file(struct nfs4_file *fi)
> > {
> > -       kref_put(&fi->fi_ref, free_nfs4_file);
> > +       BUG_ON_UNLOCKED_STATE();
> > +       if (fi)
> > +               kref_put(&fi->fi_ref, free_nfs4_file);
> > }
>
>
>
> I'm confused. When can put_nfs4_file be called when not under
> nfs4_lock_state?
>
> static inline void
> > get_nfs4_file(struct nfs4_file *fi)
> > {
> > +       BUG_ON_UNLOCKED_STATE();
> >         kref_get(&fi->fi_ref);
> > }
> >
> > @@ -538,6 +553,7 @@ alloc_client(struct xdr_netobj name)
> > static inline struct nfs4_client *
> > get_nfs4_client(struct nfs4_client *clp)
> > {
> > +       BUG_ON_UNLOCKED_STATE();
> >         kref_get(&clp->cl_ref);
> >         return clp;
> > }
> > @@ -1661,6 +1677,7 @@ find_file(struct inode *ino)
> >         unsigned int hashval = file_hashval(ino);
> >         struct nfs4_file *fp;
> >
> > +       BUG_ON_UNLOCKED_STATE();
> >         list_for_each_entry(fp, &file_hashtbl[hashval], fi_hash) {
> >                 if (fp->fi_inode == ino) {
> >                         get_nfs4_file(fp);
> > --
> > 1.5.1
> >
> >
> > _______________________________________________
> > pNFS mailing list
> > pNFS at linux-nfs.org
> > http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://linux-nfs.org/pipermail/pnfs/attachments/20070515/4af4cee3/attachment.htm