Re: [PATCH v2 8/9] pnfs: lift retry logic from send_layoutget to pnfs_update_layout

[Jeff Layton <jlayton@poochiereds.net>]

On Wed, 2016-05-11 at 06:21 -0400, Jeff Layton wrote:
> If we get back something like NFS4ERR_OLD_STATEID, that will be
> translated into -EAGAIN, and the do/while loop in send_layoutget
> will drive the call again.
> 
> This is not quite what we want, I think. An error like that is a
> sign that something has changed. That something could have been a
> concurrent LAYOUTGET that would give us a usable lseg.
> 
> Lift the retry logic into pnfs_update_layout instead. That allows
> us to redo the layout search, and may spare us from having to issue
> an RPC.
> 
> Signed-off-by: Jeff Layton <jeff.layton@primarydata.com>
> ---
>  fs/nfs/pnfs.c | 67 ++++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 34 insertions(+), 33 deletions(-)
> 
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index 5f6ed295acb5..ed3ab3e81f38 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -839,7 +839,6 @@ send_layoutget(struct pnfs_layout_hdr *lo,
>  	struct inode *ino = lo->plh_inode;
>  	struct nfs_server *server = NFS_SERVER(ino);
>  	struct nfs4_layoutget *lgp;
> -	struct pnfs_layout_segment *lseg;
>  	loff_t i_size;
>  
>  	dprintk("--> %s\n", __func__);
> @@ -849,42 +848,30 @@ send_layoutget(struct pnfs_layout_hdr *lo,
>  	 * store in lseg. If we race with a concurrent seqid morphing
>  	 * op, then re-send the LAYOUTGET.
>  	 */
> -	do {
> -		lgp = kzalloc(sizeof(*lgp), gfp_flags);
> -		if (lgp == NULL)
> -			return NULL;
> -

Just spotted this bug. The above should return ERR_PTR(-ENOMEM). Fixed
in my nfs-4.7 branch.

> -		i_size = i_size_read(ino);
> -
> -		lgp->args.minlength = PAGE_SIZE;
> -		if (lgp->args.minlength > range->length)
> -			lgp->args.minlength = range->length;
> -		if (range->iomode == IOMODE_READ) {
> -			if (range->offset >= i_size)
> -				lgp->args.minlength = 0;
> -			else if (i_size - range->offset < lgp->args.minlength)
> -				lgp->args.minlength = i_size - range->offset;
> -		}
> -		lgp->args.maxcount = PNFS_LAYOUT_MAXSIZE;
> -		pnfs_copy_range(&lgp->args.range, range);
> -		lgp->args.type = server->pnfs_curr_ld->id;
> -		lgp->args.inode = ino;
> -		lgp->args.ctx = get_nfs_open_context(ctx);
> -		lgp->gfp_flags = gfp_flags;
> -		lgp->cred = lo->plh_lc_cred;
> +	lgp = kzalloc(sizeof(*lgp), gfp_flags);
> +	if (lgp == NULL)
> +		return NULL;
>  
> -		lseg = nfs4_proc_layoutget(lgp, gfp_flags);
> -	} while (lseg == ERR_PTR(-EAGAIN));
> +	i_size = i_size_read(ino);
>  
> -	if (IS_ERR(lseg)) {
> -		if (!nfs_error_is_fatal(PTR_ERR(lseg)))
> -			lseg = NULL;
> -	} else {
> -		pnfs_layout_clear_fail_bit(lo,
> -				pnfs_iomode_to_fail_bit(range->iomode));
> +	lgp->args.minlength = PAGE_SIZE;
> +	if (lgp->args.minlength > range->length)
> +		lgp->args.minlength = range->length;
> +	if (range->iomode == IOMODE_READ) {
> +	if (range->offset >= i_size)
> +			lgp->args.minlength = 0;
> +		else if (i_size - range->offset < lgp->args.minlength)
> +			lgp->args.minlength = i_size - range->offset;
>  	}
> +	lgp->args.maxcount = PNFS_LAYOUT_MAXSIZE;
> +	pnfs_copy_range(&lgp->args.range, range);
> +	lgp->args.type = server->pnfs_curr_ld->id;
> +	lgp->args.inode = ino;
> +	lgp->args.ctx = get_nfs_open_context(ctx);
> +	lgp->gfp_flags = gfp_flags;
> +	lgp->cred = lo->plh_lc_cred;
>  
> -	return lseg;
> +	return nfs4_proc_layoutget(lgp, gfp_flags);
>  }
>  
>  static void pnfs_clear_layoutcommit(struct inode *inode,
> @@ -1646,6 +1633,20 @@ lookup_again:
>  		arg.length = PAGE_ALIGN(arg.length);
>  
>  	lseg = send_layoutget(lo, ctx, &arg, gfp_flags);
> +	if (IS_ERR(lseg)) {
> +		if (lseg == ERR_PTR(-EAGAIN)) {
> +			if (first)
> +				pnfs_clear_first_layoutget(lo);
> +			pnfs_put_layout_hdr(lo);
> +			goto lookup_again;
> +		}
> +
> +		if (!nfs_error_is_fatal(PTR_ERR(lseg)))
> +			lseg = NULL;
> +	} else {
> +		pnfs_layout_clear_fail_bit(lo, pnfs_iomode_to_fail_bit(iomode));
> +	}
> +
>  	atomic_dec(&lo->plh_outstanding);
>  	trace_pnfs_update_layout(ino, pos, count, iomode, lo,
>  				 PNFS_UPDATE_LAYOUT_SEND_LAYOUTGET);
-- 
Jeff Layton <jlayton@poochiereds.net>