[pnfs] CVS: nfsv4

[Benny Halevy]

Marc, there are a few problems with this.

1. "nfsi->pnfs_layout_state |= NFS_INO_LAYOUT_FAILED;" happens unconditionally,
in all cases, including success. (I will submit a patch fixing that today)

2. I understand marking the client's nfs_file with such a flag when the server
returns a permanent error such as NFS4ERR_LAYOUTUNAVAILABLE but in other cases,
when the error is transient we must have a mechanism to clear the state, e.g.
put a timestamp on the file after which you'll start asking for layouts again
(e.g. in the NFS4ERR_LAYOUTTRYLATER case).  I summary, the server's error code
must be analyzed by the client to determine what to do.  IMO, we should consider
reverting what you did in this submit until we have finer grained error handling
for layoutget.

3. I noticed that when virtual_update_layout returns an error on the read path
the error is returned to the application as EIO.  This is wrong as the client
should fall back to doing NFSv4 I/O when it couldn't get a layout.  I'm working
on a patch for that too.

Benny

Marc Eshel (IBM) wrote:
> CVSROOT:	/cvs
> Module name:	nfsv4
> Changes by:	eshel at citi.	2006/12/24 19:01:59
> 
> Modified files:
> 	cvs/pnfs/fs/nfs: inode.c pnfs.c 
> 
> Log message:
> If for some reason the server does not return a layout the client keep asking
> on each read. Remember that get layout failed and don't request it again.
> 
> Index: inode.c
> ===================================================================
> RCS file: /cvs/nfsv4/cvs/pnfs/fs/nfs/inode.c,v
> retrieving revision 1.17
> diff -u -r1.17 inode.c
> --- inode.c	18 Dec 2006 17:18:43 -0000	1.17
> +++ inode.c	24 Dec 2006 23:54:04 -0000
> @@ -1084,6 +1084,7 @@
> #endif
> #ifdef CONFIG_NFS_V4 /* XXX CONFIG_PNFS */
> nfsi->nfs4_acl = NULL;
> +	nfsi->pnfs_layout_state = 0;
> nfsi->current_layout = NULL;
> nfsi->layoutcommit_ctx = NULL;
> #endif /* CONFIG_NFS_V4 */
> Index: pnfs.c
> ===================================================================
> RCS file: /cvs/nfsv4/cvs/pnfs/fs/nfs/pnfs.c,v
> retrieving revision 1.50
> diff -u -r1.50 pnfs.c
> --- pnfs.c	14 Dec 2006 10:47:40 -0000	1.50
> +++ pnfs.c	24 Dec 2006 23:54:05 -0000
> @@ -436,6 +436,11 @@
> }
> 
> res.layout.buf = NULL;
> +
> +        /* if get layout already failed once goto out */
> +        if (nfsi->pnfs_layout_state & NFS_INO_LAYOUT_FAILED)
> +		goto out;
> +
> if ((result = get_layout(ino, ctx, count, pos, iomode, &arg, &res))) {
> printk("\n%s: ERROR retrieving layout %d\n",__FUNCTION__,result);
> result =  -EIO;
> @@ -468,10 +473,15 @@
> 
> result = 0;
> out:
> +
> +        /* remember that get layout failed and don't try again */
> +        nfsi->pnfs_layout_state |= NFS_INO_LAYOUT_FAILED;
> +
> /* res.layout.buf kalloc'ed by the xdr decoder? */
> if (res.layout.buf)
> kfree(res.layout.buf);
> -	dprintk("%s end (err:%d)\n",__FUNCTION__,result);
> +	dprintk("%s end (err:%d) state %d\n",
> +		__FUNCTION__,result,nfsi->pnfs_layout_state);
> return result;
> }
> 
> _______________________________________________
> pNFS mailing list
> pNFS at linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs