[pnfs] [PATCH 2/6] pnfs: Remove check for RECALL_FILE from layout return

[Andy Adamson]

On Sun, 2008-07-27 at 20:08 +0300, Benny Halevy wrote:
> On Wed, 23 Jul 2008 15:05:10 -0400 andros at netapp.com wrote:
> > From: Andy Adamson <andros at localhost.localdomain>
> > 
> > There is a valid inode passed in for RECALL_FILE and RECALL_FSID.
> > RECALL_ALL is not handled.
> 
> Where isn't RECALL_ALL handled?
> pnfs_recall_layout should send a final layoutreturn
> for RETURN_ALL using the first inode that was found
> on clp->cl_lo_inodes (on this path:
> pnfs_cb_layoutrecall->nfs_layoutrecall_find_inode
> then pnfs_cb_layoutrecall->pnfs_async_return_layout
> ->kthread_run(pnfs_recall_layout)

You are right, RECALL_ALL is handled.

> 
> 
> > 
> > Signed-off-by: Andy Adamson<andros at netapp.com>
> > ---
> >  fs/nfs/nfs4proc.c |    8 +++-----
> >  fs/nfs/pnfs.c     |   22 +++++++---------------
> >  2 files changed, 10 insertions(+), 20 deletions(-)
> > 
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index 379f98c..6febc16 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -5353,11 +5353,9 @@ static void nfs4_pnfs_layoutreturn_release(void *calldata)
> >  	dprintk("--> %s nfsi->current_layout %p\n", __func__,
> >  					nfsi->current_layout);
> >  
> > -	if (lrp->args.return_type == RECALL_FILE) {
> > -		lo = nfsi->current_layout;
> > -		BUG_ON(!lo);
> > -		pnfs_layout_release(lo);
> > -	}
> 
> Better carry this pointer in struct nfs4_pnfs_layoutreturn
> similar to what I did in commit 7da96b7670ec6b349ade95e623a3e6db98d6d7d1
> "pnfs: carry reference to pnfs_layout_type in nfs4_pnfs_layoutget"
> 
> 
> > +	lo = nfsi->current_layout;
> > +	BUG_ON(!lo);
> > +	pnfs_layout_release(lo);
> >  	kfree(calldata);
> >  	dprintk("<-- %s\n", __func__);
> >  }
> > diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> > index 47e00ea..81291f7 100644
> > --- a/fs/nfs/pnfs.c
> > +++ b/fs/nfs/pnfs.c
> > @@ -604,10 +604,7 @@ return_layout(struct inode *ino, struct nfs4_pnfs_layout_segment *range,
> >  	lrp->args.lseg = *range;
> >  	lrp->args.inode = ino;
> >  
> > -	/* FIX-ME: We don't hold a reference to lo if type != RECALL_FILE
> > -	 * and lo is NULL. Need to get_lock_current_layout for all types ?*/
> > -	if (lo)
> > -		pnfs_get_layout_stateid(&lrp->args.stateid, lo);
> > +	pnfs_get_layout_stateid(&lrp->args.stateid, lo);
> >  
> >  	status = server->nfs_client->rpc_ops->pnfs_layoutreturn(lrp);
> >  out:
> > @@ -633,18 +630,13 @@ _pnfs_return_layout(struct inode *ino, struct nfs4_pnfs_layout_segment *range,
> >  		arg.offset = 0;
> >  		arg.length = ~0;
> >  	}
> > -	if (type == RECALL_FILE) {
> > -		lo = get_lock_current_layout(nfsi);
> > -		if (lo == NULL) {
> > -			status = -EIO;
> > -			goto out;
> > -		}
> > -		pnfs_free_layout(lo, &arg);
> > -		/* unlock w/o put rebalanced by eventual call to
> > -		 * pnfs_layout_release
> > -		 */
> > -		spin_unlock(&nfsi->lo_lock);
> > +	lo = get_lock_current_layout(nfsi);
> > +	if (lo == NULL) {
> > +		status = -EIO;
> > +		goto out;
> >  	}
> > +	pnfs_free_layout(lo, &arg);
> > +	spin_unlock(&nfsi->lo_lock);
> >  
> >  	status = return_layout(ino, &arg, type, lo);
> >  out: