On Mon, Jul 12, 2010 at 2:29 PM, Benny Halevy <bhalevy@xxxxxxxxxxx> wrote: > On Jul. 12, 2010, 21:27 +0300, "William A. (Andy) Adamson" <androsadamson@xxxxxxxxx> wrote: >> On Mon, Jul 12, 2010 at 1:19 PM, Benny Halevy <bhalevy@xxxxxxxxxxx> wrote: >>> On Jul. 08, 2010, 1:34 +0300, andros@xxxxxxxxxx wrote: >>>> From: Andy Adamson <andros@xxxxxxxxxx> >>>> >>>> Signed-off-by: Andy Adamson <andros@xxxxxxxxxx> >>>> --- >>>> fs/nfs/nfs4proc.c | 2 ++ >>>> fs/nfs/pnfs.c | 13 +++++++++++++ >>>> fs/nfs/pnfs.h | 1 + >>>> 3 files changed, 16 insertions(+), 0 deletions(-) >>>> >>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c >>>> index 6acebc3..f763746 100644 >>>> --- a/fs/nfs/nfs4proc.c >>>> +++ b/fs/nfs/nfs4proc.c >>>> @@ -5565,6 +5565,8 @@ static void pnfs_layoutcommit_release(void *lcdata) >>>> struct pnfs_layoutcommit_data *data = >>>> (struct pnfs_layoutcommit_data *)lcdata; >>>> >>>> + /* Matched by get_layout in pnfs_layoutcommit_inode */ >>>> + put_layout(data->args.inode); >>>> put_rpccred(data->cred); >>>> pnfs_layoutcommit_free(lcdata); >>>> } >>>> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c >>>> index aa16e5d..d42c5da 100644 >>>> --- a/fs/nfs/pnfs.c >>>> +++ b/fs/nfs/pnfs.c >>>> @@ -354,6 +354,15 @@ put_layout_locked(struct pnfs_layout_type *lo) >>>> } >>>> >>>> void >>>> +put_layout(struct inode *inode) >>>> +{ >>>> + spin_lock(&inode->i_lock); >>>> + put_layout_locked(NFS_I(inode)->layout); >>>> + spin_unlock(&inode->i_lock); >>>> + >>>> +} >>>> + >>>> +void >>>> pnfs_layout_release(struct pnfs_layout_type *lo, >>>> struct nfs4_pnfs_layout_segment *range) >>>> { >>>> @@ -1598,6 +1607,9 @@ pnfs_layoutcommit_inode(struct inode *inode, int sync) >>>> __clear_bit(NFS_INO_LAYOUTCOMMIT, &nfsi->layout->pnfs_layout_state); >>>> pnfs_get_layout_stateid(&data->args.stateid, nfsi->layout); >>>> >>>> + /* Reference for layoutcommit matched in pnfs_layoutcommit_release */ >>>> + get_layout(NFS_I(inode)->layout); >>>> + >>> >>> So from the 30000 foot level, I need to remind myself what do >>> we need the refcount on the layout hdr (pnfs_layout_type) for? >> >>> Can we really use it detached from the inode? NO >>> Is it only for debugging to make catch the case that the inode >>> is released while there are references to the layout? >> >> When we migrate the filesystem, we need to reap the nfs_inode->layout >> while keeping the nfs_inode. >> > > But how does the refcount help us with that? > Don't we have to do this synchronously before reusing the nfs_inode? We need to drain all slots which could have async LAYOUTCOMMITs or async LAYOUTRETURNs still on the wire. The refcount keeps the nfs_inode->layout allocated for the LAYOUTCOMMIT return. -->Andy > > Benny > >> Same with server reboot, network partition, and use of a different replica. >> >> -->Andy >> >>> >>> Benny >>> >>>> spin_unlock(&inode->i_lock); >>>> >>>> /* Set up layout commit args */ >>>> @@ -1606,6 +1618,7 @@ pnfs_layoutcommit_inode(struct inode *inode, int sync) >>>> if (status) { >>>> /* The layout driver failed to setup the layoutcommit */ >>>> put_rpccred(data->cred); >>>> + put_layout(inode); >>>> goto out_free; >>>> } >>>> status = pnfs4_proc_layoutcommit(data, sync); >>>> diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h >>>> index 9b0fed4..e04b9d4 100644 >>>> --- a/fs/nfs/pnfs.h >>>> +++ b/fs/nfs/pnfs.h >>>> @@ -64,6 +64,7 @@ void pnfs_layout_release(struct pnfs_layout_type *, struct nfs4_pnfs_layout_segm >>>> void pnfs_set_layout_stateid(struct pnfs_layout_type *lo, >>>> const nfs4_stateid *stateid); >>>> void pnfs_destroy_layout(struct nfs_inode *); >>>> +void put_layout(struct inode *inode); >>>> >>>> #define PNFS_EXISTS_LDIO_OP(srv, opname) ((srv)->pnfs_curr_ld && \ >>>> (srv)->pnfs_curr_ld->ld_io_ops && \ >>> >>> -- >>> 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 >>> > -- 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