On Tue, 2010-12-21 at 23:00 -0500, Fred Isaman wrote: > This will be required to allow us to grab reference outside of i_lock. > While we are at it, make put_layout_hdr take the same argument as all the > related functions. > > Signed-off-by: Fred Isaman <iisaman@xxxxxxxxxx> > --- > fs/nfs/pnfs.c | 50 ++++++++++++++++++++++++++++---------------------- > fs/nfs/pnfs.h | 4 ++-- > 2 files changed, 30 insertions(+), 24 deletions(-) > > diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c > index 970cc3b..53a0184 100644 > --- a/fs/nfs/pnfs.c > +++ b/fs/nfs/pnfs.c > @@ -177,34 +177,40 @@ EXPORT_SYMBOL_GPL(pnfs_unregister_layoutdriver); > * pNFS client layout cache > */ > > +/* Need to hold i_lock if caller does not already hold reference */ > static void > -get_layout_hdr_locked(struct pnfs_layout_hdr *lo) > +get_layout_hdr(struct pnfs_layout_hdr *lo) > { > - assert_spin_locked(&lo->plh_inode->i_lock); > - lo->plh_refcount++; > + atomic_inc(&lo->plh_refcount); > } > > static void > -put_layout_hdr_locked(struct pnfs_layout_hdr *lo) > +destroy_layout_hdr(struct pnfs_layout_hdr *lo) > { > - assert_spin_locked(&lo->plh_inode->i_lock); > - BUG_ON(lo->plh_refcount == 0); > + dprintk("%s: freeing layout cache %p\n", __func__, lo); > + BUG_ON(!list_empty(&lo->plh_layouts)); > + NFS_I(lo->plh_inode)->layout = NULL; > + kfree(lo); > +} > > - lo->plh_refcount--; > - if (!lo->plh_refcount) { > - dprintk("%s: freeing layout cache %p\n", __func__, lo); > - BUG_ON(!list_empty(&lo->plh_layouts)); > - NFS_I(lo->plh_inode)->layout = NULL; > - kfree(lo); > - } > +static void > +put_layout_hdr_locked(struct pnfs_layout_hdr *lo) > +{ > + BUG_ON(atomic_read(&lo->plh_refcount) == 0); Isn't this debugged by now? > + if (atomic_dec_and_test(&lo->plh_refcount)) > + destroy_layout_hdr(lo); > } > > void > -put_layout_hdr(struct inode *inode) > +put_layout_hdr(struct pnfs_layout_hdr *lo) > { > - spin_lock(&inode->i_lock); > - put_layout_hdr_locked(NFS_I(inode)->layout); > - spin_unlock(&inode->i_lock); > + struct inode *inode = lo->plh_inode; > + > + BUG_ON(atomic_read(&lo->plh_refcount) == 0); Ditto.... > + if (atomic_dec_and_lock(&lo->plh_refcount, &inode->i_lock)) { > + destroy_layout_hdr(lo); > + spin_unlock(&inode->i_lock); > + } > } > > static void > @@ -224,7 +230,7 @@ static void free_lseg(struct pnfs_layout_segment *lseg) > BUG_ON(atomic_read(&lseg->pls_refcount) != 0); > NFS_SERVER(ino)->pnfs_curr_ld->free_lseg(lseg); > /* Matched by get_layout_hdr in pnfs_insert_layout */ > - put_layout_hdr(ino); > + put_layout_hdr(NFS_I(ino)->layout); > } > > /* The use of tmp_list is necessary because pnfs_curr_ld->free_lseg > @@ -481,7 +487,7 @@ pnfs_insert_layout(struct pnfs_layout_hdr *lo, > __func__, lseg, lseg->pls_range.iomode, > lseg->pls_range.offset, lseg->pls_range.length); > } > - get_layout_hdr_locked(lo); > + get_layout_hdr(lo); > > dprintk("%s:Return\n", __func__); > } > @@ -494,7 +500,7 @@ alloc_init_layout_hdr(struct inode *ino) > lo = kzalloc(sizeof(struct pnfs_layout_hdr), GFP_KERNEL); > if (!lo) > return NULL; > - lo->plh_refcount = 1; > + atomic_set(&lo->plh_refcount, 1); > INIT_LIST_HEAD(&lo->plh_layouts); > INIT_LIST_HEAD(&lo->plh_segs); > lo->plh_inode = ino; > @@ -609,7 +615,7 @@ pnfs_update_layout(struct inode *ino, > goto out_unlock; > atomic_inc(&lo->plh_outstanding); > > - get_layout_hdr_locked(lo); > + get_layout_hdr(lo); > if (list_empty(&lo->plh_segs)) { > /* The lo must be on the clp list if there is any > * chance of a CB_LAYOUTRECALL(FILE) coming in. > @@ -632,7 +638,7 @@ pnfs_update_layout(struct inode *ino, > spin_unlock(&ino->i_lock); > } > atomic_dec(&lo->plh_outstanding); > - put_layout_hdr(ino); > + put_layout_hdr(lo); > out: > dprintk("%s end, state 0x%lx lseg %p\n", __func__, > nfsi->layout->plh_flags, lseg); > diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h > index 698380d..8aaab56 100644 > --- a/fs/nfs/pnfs.h > +++ b/fs/nfs/pnfs.h > @@ -65,7 +65,7 @@ struct pnfs_layoutdriver_type { > }; > > struct pnfs_layout_hdr { > - unsigned long plh_refcount; > + atomic_t plh_refcount; > struct list_head plh_layouts; /* other client layouts */ > struct list_head plh_segs; /* layout segments list */ > nfs4_stateid plh_stateid; > @@ -147,7 +147,7 @@ void unset_pnfs_layoutdriver(struct nfs_server *); > int pnfs_layout_process(struct nfs4_layoutget *lgp); > void pnfs_destroy_layout(struct nfs_inode *); > void pnfs_destroy_all_layouts(struct nfs_client *); > -void put_layout_hdr(struct inode *inode); > +void put_layout_hdr(struct pnfs_layout_hdr *lo); > int pnfs_choose_layoutget_stateid(nfs4_stateid *dst, > struct pnfs_layout_hdr *lo, > struct nfs4_state *open_state); -- Trond Myklebust Linux NFS client maintainer NetApp Trond.Myklebust@xxxxxxxxxx www.netapp.com -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html