[PATCH 0/6] pnfs DLM cluster only use read iomode layouts Version 2

[PATCH 0/6] pnfs DLM cluster only use read iomode layouts Version 2

[andros]

Responded to comments on error code fixes.

Applies against 2.6.33-rc6 pnfs-all branch.

In a DLM cluster, writing to a node other than the node where the open call
occurred (where meta data is cached) will have performance implications when
the write causes meta data changes that need to be propagated to the open call
node.

Return NFS4ERR_BADIOMODE for LAYOUTGET requests with LAYOUTIOMODE4_RW iomode.

0001-pnfsd-fix-file-system-API-layout_get-error-codes.patch
0002-pnfsd-fix-NFS4ERR_BADIOMODE-in-layoutget.patch
0003-pnfsd-DLM-file-layout-only-support-read-iomode-layou.patch
0004-pnfsd-fix-DLM-file-layout-no-device-return.patch

Allow the client to fail a RW iomode layout and still grab a RO iomode layout.
0005-pnfs-set-failed-layout-bit-per-iomode.patch

fix compile errors
0006-pnfsd-fix-compile-errors-when-CONFIG_PNFSD-is-not-se.patch

Testing: Connectathon tests pass. Basic test shows RW iomode layout for
'bigfile' fails with NFS4ERR_BADIOMODE with writes going through the MDS,
and RO layout for 'bigfile' is obtained with reads going to the DS.


-->Andy

[PATCH 1/6] pnfsd: fix file system API layout_get error codes

[andros]

From: Andy Adamson <andros@netapp.com>

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfsd/nfs4pnfsd.c |   36 ++++++++++++++++++++++--------------
 1 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index 816e2f0..75bddd8 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -868,23 +868,31 @@ nfs4_pnfs_get_layout(struct nfsd4_pnfs_layoutget *lgp,
 		res.lg_seg.offset, res.lg_seg.length);
 
 	if (status) {
+		/*
+		 * The allowable error codes for the layout_get pNFS export
+		 * operations vector function (from the file system) can be
+		 * expanded as needed to include other errors defined for
+		 * the RFC 5561 LAYOUTGET operation.
+		 */
 		switch (status) {
-		case -ETOOSMALL:
-			status = nfserr_toosmall;
-			break;
-		case -ENOMEM:
-		case -EAGAIN:
-		case -EINTR:
-			status = nfserr_layouttrylater;
-			break;
-		case -ENOENT:
-			status = nfserr_badlayout;
-			break;
-		case -E2BIG:
-			status = nfserr_toosmall;
+		case nfserr_acces:
+		case nfserr_badiomode:
+			/* No support for LAYOUTIOMODE4_RW layouts */
+		case nfserr_badlayout:
+			/* No layout matching loga_minlength rules */
+		case nfserr_inval:
+		case nfserr_io:
+		case nfserr_layouttrylater:
+		case nfserr_layoutunavailable:
+		case nfserr_locked:
+		case nfserr_nospc:
+		case nfserr_recallconflict:
+		case nfserr_serverfault:
+		case nfserr_toosmall:
+			/* Requested layout to big for loga_maxcount */
 			break;
 		default:
-			status = nfserr_layoutunavailable;
+			BUG();
 		}
 		goto out_freelayout;
 	}
-- 
1.6.6

[PATCH 2/6] pnfsd: fix NFS4ERR_BADIOMODE in layoutget

[andros]

From: Andy Adamson <andros@netapp.com>

If an invalid iomode, or an iomode of LAYOUTIOMODE4_ANY is specified, the
metadata server MUST return NFS4ERR_BADIOMODE.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfsd/nfs4proc.c |   11 ++---------
 1 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 769628f..575e1b6 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1114,21 +1114,14 @@ nfsd4_layoutget(struct svc_rqst *rqstp,
 	if (status)
 		goto out;
 
-	status = nfserr_inval;
+	status = nfserr_badiomode;
 	if (lgp->lg_seg.iomode != IOMODE_READ &&
-	    lgp->lg_seg.iomode != IOMODE_RW &&
-	    lgp->lg_seg.iomode != IOMODE_ANY) {
+	    lgp->lg_seg.iomode != IOMODE_RW) {
 		dprintk("pNFS %s: invalid iomode %d\n", __func__,
 			lgp->lg_seg.iomode);
 		goto out;
 	}
 
-	status = nfserr_badiomode;
-	if (lgp->lg_seg.iomode == IOMODE_ANY) {
-		dprintk("pNFS %s: IOMODE_ANY is not allowed\n", __func__);
-		goto out;
-	}
-
 	/* Set up arguments so layout can be retrieved at encode time */
 	lgp->lg_fhp = current_fh;
 	copy_clientid((clientid_t *)&lgp->lg_seg.clientid, cstate->session);
-- 
1.6.6

[PATCH 3/6] pnfsd: DLM file layout only support read iomode layouts

[andros]

From: Andy Adamson <andros@netapp.com>

In a DLM cluster, writing to a node other than the node where the open call
occurred (where meta data is cached) will have performance implications when
the write causes meta data changes that need to be propagated to the open call
node.

DlM clusters support only LAYOUTIOMODE4_READ layouts. Writes will go through
the MDS.

Return NFS4ERR_BADIOMODE for LAYOUTGET requests with LAYOUTIOMODE4_RW iomode.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfsd/nfs4pnfsdlm.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/fs/nfsd/nfs4pnfsdlm.c b/fs/nfsd/nfs4pnfsdlm.c
index 293bf95..8cc7c84 100644
--- a/fs/nfsd/nfs4pnfsdlm.c
+++ b/fs/nfsd/nfs4pnfsdlm.c
@@ -28,6 +28,7 @@
 #include <linux/nfsd/nfs4layoutxdr.h>
 
 #include "nfsfh.h"
+#include "nfsd.h"
 
 #define NFSDDBG_FACILITY                NFSDDBG_PROC
 
@@ -331,6 +332,10 @@ static int nfsd4_pnfs_dlm_layoutget(struct inode *inode,
 
 	dprintk("%s: LAYOUT_GET\n", __func__);
 
+	/* DLM exported file systems only support layouts for READ */
+	if (res->lg_seg.iomode == IOMODE_RW)
+		return nfserr_badiomode;
+
 	index = dlm_ino_hash(inode);
 	dprintk("%s first stripe index %d i_ino %lu\n", __func__, index,
 		inode->i_ino);
-- 
1.6.6

[PATCH 4/6] pnfsd: fix DLM file layout no device return

[andros]

From: Andy Adamson <andros@netapp.com>

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfsd/nfs4pnfsdlm.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4pnfsdlm.c b/fs/nfsd/nfs4pnfsdlm.c
index 8cc7c84..83c4698 100644
--- a/fs/nfsd/nfs4pnfsdlm.c
+++ b/fs/nfsd/nfs4pnfsdlm.c
@@ -316,7 +316,7 @@ static int dlm_ino_hash(struct inode *ino)
 	 */
 	de = nfsd4_find_pnfs_dlm_device(ino->i_sb->s_bdev->bd_disk->disk_name);
 	if (!de)
-		return -EINVAL;
+		return -1;
 	hash_mask = de->num_ds - 1;
 	return ino->i_ino & hash_mask;
 }
@@ -340,7 +340,7 @@ static int nfsd4_pnfs_dlm_layoutget(struct inode *inode,
 	dprintk("%s first stripe index %d i_ino %lu\n", __func__, index,
 		inode->i_ino);
 	if (index < 0)
-		return index;
+		return nfserr_layoutunavailable;
 
 	res->lg_seg.layout_type = LAYOUT_NFSV4_FILES;
 	/* Always give out whole file layouts */
-- 
1.6.6

[PATCH 5/6] pnfs: set failed layout bit per iomode

[andros]

From: Andy Adamson <andros@netapp.com>

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/pnfs.c          |    6 +++---
 fs/nfs/pnfs.h          |    6 ++++++
 include/linux/nfs_fs.h |    5 +++--
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 3a05422..596cfce 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -1119,11 +1119,11 @@ pnfs_update_layout(struct inode *ino,
 	}
 
 	/* if get layout already failed once goto out */
-	if (test_bit(NFS_INO_LAYOUT_FAILED, &nfsi->pnfs_layout_state)) {
+	if (test_bit(lo_fail_bit(iomode), &nfsi->pnfs_layout_state)) {
 		if (unlikely(nfsi->pnfs_layout_suspend &&
 		    get_seconds() >= nfsi->pnfs_layout_suspend)) {
 			dprintk("%s: layout_get resumed\n", __func__);
-			clear_bit(NFS_INO_LAYOUT_FAILED,
+			clear_bit(lo_fail_bit(iomode),
 				  &nfsi->pnfs_layout_state);
 			nfsi->pnfs_layout_suspend = 0;
 		} else {
@@ -1247,7 +1247,7 @@ pnfs_get_layout_done(struct nfs4_pnfs_layoutget *lgp, int rpc_status)
 get_out:
 	/* remember that get layout failed and suspend trying */
 	nfsi->pnfs_layout_suspend = suspend;
-	set_bit(NFS_INO_LAYOUT_FAILED, &nfsi->pnfs_layout_state);
+	set_bit(lo_fail_bit(lgp->args.lseg.iomode), &nfsi->pnfs_layout_state);
 	dprintk("%s: layout_get suspended until %ld\n",
 		__func__, suspend);
 out:
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index 379e67d..312c4ef 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -94,6 +94,12 @@ void pnfs_set_layout_stateid(struct pnfs_layout_type *lo,
 				     (srv)->pnfs_curr_ld->ld_policy_ops && \
 				     (srv)->pnfs_curr_ld->ld_policy_ops->opname)
 
+static inline int lo_fail_bit(u32 iomode)
+{
+	return iomode == IOMODE_RW ?
+			 NFS_INO_RW_LAYOUT_FAILED : NFS_INO_RO_LAYOUT_FAILED;
+}
+
 /* Return true if a layout driver is being used for this mountpoint */
 static inline int pnfs_enabled_sb(struct nfs_server *nfss)
 {
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index fc0990a..dd57a6a 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -201,8 +201,9 @@ struct nfs_inode {
 	struct list_head	lo_inodes;
 
 	unsigned long pnfs_layout_state;
-#define NFS_INO_LAYOUT_FAILED	0	/* get layout failed, stop trying */
-#define NFS_INO_LAYOUT_ALLOC	1	/* bit lock for layout allocation */
+#define NFS_INO_RO_LAYOUT_FAILED 0 	/* get ro layout failed stop trying */
+#define NFS_INO_RW_LAYOUT_FAILED 1 	/* get rw layout failed stop trying */
+#define NFS_INO_LAYOUT_ALLOC     2	/* bit lock for layout allocation */
 	time_t pnfs_layout_suspend;
 	wait_queue_head_t lo_waitq;
 	spinlock_t lo_lock;
-- 
1.6.6

[PATCH 6/6] pnfsd: fix compile errors when CONFIG_PNFSD is not set

[andros]

From: Andy Adamson <andros@netapp.com>

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfsd/nfs4pnfsd.c   |    4 ++++
 fs/nfsd/nfs4pnfsdlm.c |    4 ++++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index 75bddd8..44a1edd 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -21,6 +21,8 @@
  *
  *****************************************************************************/
 
+#if defined(CONFIG_PNFSD)
+
 #include "pnfsd.h"
 
 #define NFSDDBG_FACILITY                NFSDDBG_PROC
@@ -1676,3 +1678,5 @@ int nfsd_device_notify_cb(struct super_block *sb,
 		__func__, status, notify_num);
 	return status;
 }
+
+#endif /* CONFIG_PNFSD */
diff --git a/fs/nfsd/nfs4pnfsdlm.c b/fs/nfsd/nfs4pnfsdlm.c
index 83c4698..8e8a5a8 100644
--- a/fs/nfsd/nfs4pnfsdlm.c
+++ b/fs/nfsd/nfs4pnfsdlm.c
@@ -21,6 +21,8 @@
  *
  ******************************************************************************/
 
+#if defined(CONFIG_PNFSD)
+
 #include <linux/nfs4.h>
 #include <linux/nfsd/const.h>
 #include <linux/nfsd/debug.h>
@@ -402,3 +404,5 @@ const struct pnfs_export_operations pnfs_dlm_export_ops = {
 	.layout_get = nfsd4_pnfs_dlm_layoutget,
 };
 EXPORT_SYMBOL(pnfs_dlm_export_ops);
+
+#endif /* CONFIG_PNFSD */
-- 
1.6.6

Re: [pnfs] [PATCH 6/6] pnfsd: fix compile errors when CONFIG_PNFSD is not set

[Benny Halevy]

I don't see any problems with CONFIG_PNFSD=n
both fs/nfsd/nfs4pnfsd.c and fs/nfsd/nfs4pnfsdlm.c are built only conditionally
with PNFSD is configured.

fs/nfsd/Makefile:
nfsd-$(CONFIG_PNFSD)	+= nfs4pnfsd.o nfs4pnfsdlm.o nfs4pnfsds.o

Benny

On Feb. 09, 2010, 19:42 +0200, andros@netapp.com wrote:
> From: Andy Adamson <andros@netapp.com>
> 
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
>  fs/nfsd/nfs4pnfsd.c   |    4 ++++
>  fs/nfsd/nfs4pnfsdlm.c |    4 ++++
>  2 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
> index 75bddd8..44a1edd 100644
> --- a/fs/nfsd/nfs4pnfsd.c
> +++ b/fs/nfsd/nfs4pnfsd.c
> @@ -21,6 +21,8 @@
>   *
>   *****************************************************************************/
>  
> +#if defined(CONFIG_PNFSD)
> +
>  #include "pnfsd.h"
>  
>  #define NFSDDBG_FACILITY                NFSDDBG_PROC
> @@ -1676,3 +1678,5 @@ int nfsd_device_notify_cb(struct super_block *sb,
>  		__func__, status, notify_num);
>  	return status;
>  }
> +
> +#endif /* CONFIG_PNFSD */
> diff --git a/fs/nfsd/nfs4pnfsdlm.c b/fs/nfsd/nfs4pnfsdlm.c
> index 83c4698..8e8a5a8 100644
> --- a/fs/nfsd/nfs4pnfsdlm.c
> +++ b/fs/nfsd/nfs4pnfsdlm.c
> @@ -21,6 +21,8 @@
>   *
>   ******************************************************************************/
>  
> +#if defined(CONFIG_PNFSD)
> +
>  #include <linux/nfs4.h>
>  #include <linux/nfsd/const.h>
>  #include <linux/nfsd/debug.h>
> @@ -402,3 +404,5 @@ const struct pnfs_export_operations pnfs_dlm_export_ops = {
>  	.layout_get = nfsd4_pnfs_dlm_layoutget,
>  };
>  EXPORT_SYMBOL(pnfs_dlm_export_ops);
> +
> +#endif /* CONFIG_PNFSD */

Re: [pnfs] [PATCH 6/6] pnfsd: fix compile errors when CONFIG_PNFSD is not set

[Andy Adamson]

On Feb 9, 2010, at 1:00 PM, Benny Halevy wrote:

> I don't see any problems with CONFIG_PNFSD=n
> both fs/nfsd/nfs4pnfsd.c and fs/nfsd/nfs4pnfsdlm.c are built only  
> conditionally
> with PNFSD is configured.

Yep - I was building directly e.g.

% make fs/nfsd/nfs4pnfsd.o

Which of course doesn't happen :)

-->Andy

>
> fs/nfsd/Makefile:
> nfsd-$(CONFIG_PNFSD)	+= nfs4pnfsd.o nfs4pnfsdlm.o nfs4pnfsds.o
>
> Benny
>
> On Feb. 09, 2010, 19:42 +0200, andros@netapp.com wrote:
>> From: Andy Adamson <andros@netapp.com>
>>
>> Signed-off-by: Andy Adamson <andros@netapp.com>
>> ---
>> fs/nfsd/nfs4pnfsd.c   |    4 ++++
>> fs/nfsd/nfs4pnfsdlm.c |    4 ++++
>> 2 files changed, 8 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
>> index 75bddd8..44a1edd 100644
>> --- a/fs/nfsd/nfs4pnfsd.c
>> +++ b/fs/nfsd/nfs4pnfsd.c
>> @@ -21,6 +21,8 @@
>>  *
>>   
>> *****************************************************************************/
>>
>> +#if defined(CONFIG_PNFSD)
>> +
>> #include "pnfsd.h"
>>
>> #define NFSDDBG_FACILITY                NFSDDBG_PROC
>> @@ -1676,3 +1678,5 @@ int nfsd_device_notify_cb(struct super_block  
>> *sb,
>> 		__func__, status, notify_num);
>> 	return status;
>> }
>> +
>> +#endif /* CONFIG_PNFSD */
>> diff --git a/fs/nfsd/nfs4pnfsdlm.c b/fs/nfsd/nfs4pnfsdlm.c
>> index 83c4698..8e8a5a8 100644
>> --- a/fs/nfsd/nfs4pnfsdlm.c
>> +++ b/fs/nfsd/nfs4pnfsdlm.c
>> @@ -21,6 +21,8 @@
>>  *
>>   
>> ******************************************************************************/
>>
>> +#if defined(CONFIG_PNFSD)
>> +
>> #include <linux/nfs4.h>
>> #include <linux/nfsd/const.h>
>> #include <linux/nfsd/debug.h>
>> @@ -402,3 +404,5 @@ const struct pnfs_export_operations  
>> pnfs_dlm_export_ops = {
>> 	.layout_get = nfsd4_pnfs_dlm_layoutget,
>> };
>> EXPORT_SYMBOL(pnfs_dlm_export_ops);
>> +
>> +#endif /* CONFIG_PNFSD */

Re: [pnfs] [PATCH 2/6] pnfsd: fix NFS4ERR_BADIOMODE in layoutget

[Benny Halevy]

Thanks!

I'm committing everything but patch 6/6 for now.
Please check again the !CONFIG_PNFSD case.
I don't see any problem.

Benny

On Feb. 09, 2010, 19:42 +0200, andros@netapp.com wrote:
> From: Andy Adamson <andros@netapp.com>
> 
> If an invalid iomode, or an iomode of LAYOUTIOMODE4_ANY is specified, the
> metadata server MUST return NFS4ERR_BADIOMODE.
> 
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
>  fs/nfsd/nfs4proc.c |   11 ++---------
>  1 files changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 769628f..575e1b6 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1114,21 +1114,14 @@ nfsd4_layoutget(struct svc_rqst *rqstp,
>  	if (status)
>  		goto out;
>  
> -	status = nfserr_inval;
> +	status = nfserr_badiomode;
>  	if (lgp->lg_seg.iomode != IOMODE_READ &&
> -	    lgp->lg_seg.iomode != IOMODE_RW &&
> -	    lgp->lg_seg.iomode != IOMODE_ANY) {
> +	    lgp->lg_seg.iomode != IOMODE_RW) {
>  		dprintk("pNFS %s: invalid iomode %d\n", __func__,
>  			lgp->lg_seg.iomode);
>  		goto out;
>  	}
>  
> -	status = nfserr_badiomode;
> -	if (lgp->lg_seg.iomode == IOMODE_ANY) {
> -		dprintk("pNFS %s: IOMODE_ANY is not allowed\n", __func__);
> -		goto out;
> -	}
> -
>  	/* Set up arguments so layout can be retrieved at encode time */
>  	lgp->lg_fhp = current_fh;
>  	copy_clientid((clientid_t *)&lgp->lg_seg.clientid, cstate->session);

Re: [pnfs] [PATCH 2/6] pnfsd: fix NFS4ERR_BADIOMODE in layoutget

[Andy Adamson]

On Feb 9, 2010, at 1:03 PM, Benny Halevy wrote:

> Thanks!
>
> I'm committing everything but patch 6/6 for now.
> Please check again the !CONFIG_PNFSD case.
> I don't see any problem.

OK - Thanks

-->Andy

>
> Benny
>
> On Feb. 09, 2010, 19:42 +0200, andros@netapp.com wrote:
>> From: Andy Adamson <andros@netapp.com>
>>
>> If an invalid iomode, or an iomode of LAYOUTIOMODE4_ANY is  
>> specified, the
>> metadata server MUST return NFS4ERR_BADIOMODE.
>>
>> Signed-off-by: Andy Adamson <andros@netapp.com>
>> ---
>> fs/nfsd/nfs4proc.c |   11 ++---------
>> 1 files changed, 2 insertions(+), 9 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>> index 769628f..575e1b6 100644
>> --- a/fs/nfsd/nfs4proc.c
>> +++ b/fs/nfsd/nfs4proc.c
>> @@ -1114,21 +1114,14 @@ nfsd4_layoutget(struct svc_rqst *rqstp,
>> 	if (status)
>> 		goto out;
>>
>> -	status = nfserr_inval;
>> +	status = nfserr_badiomode;
>> 	if (lgp->lg_seg.iomode != IOMODE_READ &&
>> -	    lgp->lg_seg.iomode != IOMODE_RW &&
>> -	    lgp->lg_seg.iomode != IOMODE_ANY) {
>> +	    lgp->lg_seg.iomode != IOMODE_RW) {
>> 		dprintk("pNFS %s: invalid iomode %d\n", __func__,
>> 			lgp->lg_seg.iomode);
>> 		goto out;
>> 	}
>>
>> -	status = nfserr_badiomode;
>> -	if (lgp->lg_seg.iomode == IOMODE_ANY) {
>> -		dprintk("pNFS %s: IOMODE_ANY is not allowed\n", __func__);
>> -		goto out;
>> -	}
>> -
>> 	/* Set up arguments so layout can be retrieved at encode time */
>> 	lgp->lg_fhp = current_fh;
>> 	copy_clientid((clientid_t *)&lgp->lg_seg.clientid, cstate->session);

Re: [pnfs] [PATCH 2/6] pnfsd: fix NFS4ERR_BADIOMODE in layoutget

[Boaz Harrosh]

On 02/09/2010 08:03 PM, Benny Halevy wrote:
> Thanks!
> 
> I'm committing everything but patch 6/6 for now.
> Please check again the !CONFIG_PNFSD case.
> I don't see any problem.
> 
> Benny
> 

Don't we have other places that need a patch like the one I sent
to exofs?

Boaz

[PATCH] SQUASHME: pnfsd-exofs: Change layoutget return codes

[Boaz Harrosh]

Dependent on patch from Andy:
	[PATCH 1/6] pnfsd: fix file system API layout_get error codes

Change codes to nfs4.1 codes

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 fs/exofs/export.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/exofs/export.c b/fs/exofs/export.c
index be3d54e..7a59614 100644
--- a/fs/exofs/export.c
+++ b/fs/exofs/export.c
@@ -95,13 +95,13 @@ static int exofs_layout_get(
 	/* skip opaque size, will be filled-in later */
 	start = exp_xdr_reserve_qwords(xdr, 1);
 	if (!start) {
-		err = -E2BIG;
+		err = nfserr_toosmall;
 		goto err;
 	}
 
 	creds = kcalloc(el->s_numdevs, sizeof(*creds), GFP_KERNEL);
 	if (!creds) {
-		err = -ENOMEM;
+		err = nfserr_layouttrylater;
 		goto err;
 	}
 
@@ -134,8 +134,10 @@ static int exofs_layout_get(
 	layout.olo_comps = creds;
 
 	err = pnfs_osd_xdr_encode_layout(xdr, &layout);
-	if (err)
+	if (err) {
+		err = nfserr_toosmall; /*FIXME: Change osd_xdr error codes */
 		goto err;
+	}
 
 	exp_xdr_encode_opaque_len(start, xdr->p);
 
@@ -146,7 +148,7 @@ static int exofs_layout_get(
 	spin_unlock(&oi->i_layout_lock);
 
 	if (in_recall)
-		err = -EAGAIN;
+		err = nfserr_layouttrylater;
 
 err:
 	kfree(creds);
-- 
1.6.6

Re: [pnfs] [PATCH] SQUASHME: pnfsd-exofs: Change layoutget return codes

[Boaz Harrosh]

On 02/10/2010 10:24 AM, Boaz Harrosh wrote:
> 
> Dependent on patch from Andy:
> 	[PATCH 1/6] pnfsd: fix file system API layout_get error codes
> 
> Change codes to nfs4.1 codes
> 
> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>

Rrrr I spoke to soon.

Andy, Benny
This will not work, currently. All the nfserr_xxx constants are defined
if fs/nfsd/nfsd.h. 
(Why do they exist at all, why can't we use the client's definitions for these?)

At the minimum they need to move to include/linux/nfsd/export.h. But I say kill them
and use these from include/linux/nfs4.h. Added bonus these are enums, so prototype
of .layout_get() can change to return enum nfsstat4 and the compiler fixes all our
bugs.

I'm posting a second patch that uses "enum nfsstat4" constants in exofs which will
work just fine, but is really ugly on the documentation aspect of Andy's patch.

Bruce may I submit a patch that globally gets rid of all nfserr_* defines and uses
NFS4ERR_* in their place?

Cheers
Boaz

> ---
>  fs/exofs/export.c |   10 ++++++----
>  1 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/exofs/export.c b/fs/exofs/export.c
> index be3d54e..7a59614 100644
> --- a/fs/exofs/export.c
> +++ b/fs/exofs/export.c
> @@ -95,13 +95,13 @@ static int exofs_layout_get(
>  	/* skip opaque size, will be filled-in later */
>  	start = exp_xdr_reserve_qwords(xdr, 1);
>  	if (!start) {
> -		err = -E2BIG;
> +		err = nfserr_toosmall;
>  		goto err;
>  	}
>  
>  	creds = kcalloc(el->s_numdevs, sizeof(*creds), GFP_KERNEL);
>  	if (!creds) {
> -		err = -ENOMEM;
> +		err = nfserr_layouttrylater;
>  		goto err;
>  	}
>  
> @@ -134,8 +134,10 @@ static int exofs_layout_get(
>  	layout.olo_comps = creds;
>  
>  	err = pnfs_osd_xdr_encode_layout(xdr, &layout);
> -	if (err)
> +	if (err) {
> +		err = nfserr_toosmall; /*FIXME: Change osd_xdr error codes */
>  		goto err;
> +	}
>  
>  	exp_xdr_encode_opaque_len(start, xdr->p);
>  
> @@ -146,7 +148,7 @@ static int exofs_layout_get(
>  	spin_unlock(&oi->i_layout_lock);
>  
>  	if (in_recall)
> -		err = -EAGAIN;
> +		err = nfserr_layouttrylater;
>  
>  err:
>  	kfree(creds);

RE: [pnfs] [PATCH] SQUASHME: pnfsd-exofs: Change layoutget return codes

[Halevy, Benny]

The nfserr_xxx constants are already encoded in network order.
This is the internal convention de-facto in nfsd.
We can either leave them in the internal header and translate
the canonical constants to their net-order representation
or define them in a public header and have the filesystem
use them.  The former seems cleaner with regards to layering.

Benny

-----Original Message-----
From: pnfs-bounces@linux-nfs.org on behalf of Boaz Harrosh
Sent: Wed 2010-02-10 12:45
To: andros@netapp.com
Cc: linux-nfs@vger.kernel.org; pnfs@linux-nfs.org; J. Bruce Fields
Subject: Re: [pnfs] [PATCH] SQUASHME: pnfsd-exofs: Change layoutget return codes
 
On 02/10/2010 10:24 AM, Boaz Harrosh wrote:
> 
> Dependent on patch from Andy:
> 	[PATCH 1/6] pnfsd: fix file system API layout_get error codes
> 
> Change codes to nfs4.1 codes
> 
> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>

Rrrr I spoke to soon.

Andy, Benny
This will not work, currently. All the nfserr_xxx constants are defined
if fs/nfsd/nfsd.h. 
(Why do they exist at all, why can't we use the client's definitions for these?)

At the minimum they need to move to include/linux/nfsd/export.h. But I say kill them
and use these from include/linux/nfs4.h. Added bonus these are enums, so prototype
of .layout_get() can change to return enum nfsstat4 and the compiler fixes all our
bugs.

I'm posting a second patch that uses "enum nfsstat4" constants in exofs which will
work just fine, but is really ugly on the documentation aspect of Andy's patch.

Bruce may I submit a patch that globally gets rid of all nfserr_* defines and uses
NFS4ERR_* in their place?

Cheers
Boaz

> ---
>  fs/exofs/export.c |   10 ++++++----
>  1 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/exofs/export.c b/fs/exofs/export.c
> index be3d54e..7a59614 100644
> --- a/fs/exofs/export.c
> +++ b/fs/exofs/export.c
> @@ -95,13 +95,13 @@ static int exofs_layout_get(
>  	/* skip opaque size, will be filled-in later */
>  	start = exp_xdr_reserve_qwords(xdr, 1);
>  	if (!start) {
> -		err = -E2BIG;
> +		err = nfserr_toosmall;
>  		goto err;
>  	}
>  
>  	creds = kcalloc(el->s_numdevs, sizeof(*creds), GFP_KERNEL);
>  	if (!creds) {
> -		err = -ENOMEM;
> +		err = nfserr_layouttrylater;
>  		goto err;
>  	}
>  
> @@ -134,8 +134,10 @@ static int exofs_layout_get(
>  	layout.olo_comps = creds;
>  
>  	err = pnfs_osd_xdr_encode_layout(xdr, &layout);
> -	if (err)
> +	if (err) {
> +		err = nfserr_toosmall; /*FIXME: Change osd_xdr error codes */
>  		goto err;
> +	}
>  
>  	exp_xdr_encode_opaque_len(start, xdr->p);
>  
> @@ -146,7 +148,7 @@ static int exofs_layout_get(
>  	spin_unlock(&oi->i_layout_lock);
>  
>  	if (in_recall)
> -		err = -EAGAIN;
> +		err = nfserr_layouttrylater;
>  
>  err:
>  	kfree(creds);

_______________________________________________
pNFS mailing list
pNFS@linux-nfs.org
http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs

Re: [pnfs] [PATCH] SQUASHME: pnfsd-exofs: Change layoutget return codes

[Boaz Harrosh]

On 02/10/2010 03:06 PM, Halevy, Benny wrote:
> The nfserr_xxx constants are already encoded in network order.
> This is the internal convention de-facto in nfsd.
> We can either leave them in the internal header and translate
> the canonical constants to their net-order representation
> or define them in a public header and have the filesystem
> use them.  The former seems cleaner with regards to layering.
> 

OK So the "version 2" patch is a bug as well.

I agree with benny, external code can just use the include/linux/nfs4.h
constants as in my "version 2" patch. (Just my $0.017)

In any way or another, once decided, I'll fix accordingly.

Thanks
Boaz

> Benny
> 
> -----Original Message-----
> From: pnfs-bounces@linux-nfs.org on behalf of Boaz Harrosh
> Sent: Wed 2010-02-10 12:45
> To: andros@netapp.com
> Cc: linux-nfs@vger.kernel.org; pnfs@linux-nfs.org; J. Bruce Fields
> Subject: Re: [pnfs] [PATCH] SQUASHME: pnfsd-exofs: Change layoutget
> return codes
> 
> On 02/10/2010 10:24 AM, Boaz Harrosh wrote:
>>
>> Dependent on patch from Andy:
>>       [PATCH 1/6] pnfsd: fix file system API layout_get error codes
>>
>> Change codes to nfs4.1 codes
>>
>> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
> 
> Rrrr I spoke to soon.
> 
> Andy, Benny
> This will not work, currently. All the nfserr_xxx constants are defined
> if fs/nfsd/nfsd.h.
> (Why do they exist at all, why can't we use the client's definitions for
> these?)
> 
> At the minimum they need to move to include/linux/nfsd/export.h. But I
> say kill them
> and use these from include/linux/nfs4.h. Added bonus these are enums, so
> prototype
> of .layout_get() can change to return enum nfsstat4 and the compiler
> fixes all our
> bugs.
> 
> I'm posting a second patch that uses "enum nfsstat4" constants in exofs
> which will
> work just fine, but is really ugly on the documentation aspect of Andy's
> patch.
> 
> Bruce may I submit a patch that globally gets rid of all nfserr_*
> defines and uses
> NFS4ERR_* in their place?
> 
> Cheers
> Boaz
> 
>> ---
>>  fs/exofs/export.c |   10 ++++++----
>>  1 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/exofs/export.c b/fs/exofs/export.c
>> index be3d54e..7a59614 100644
>> --- a/fs/exofs/export.c
>> +++ b/fs/exofs/export.c
>> @@ -95,13 +95,13 @@ static int exofs_layout_get(
>>       /* skip opaque size, will be filled-in later */
>>       start = exp_xdr_reserve_qwords(xdr, 1);
>>       if (!start) {
>> -             err = -E2BIG;
>> +             err = nfserr_toosmall;
>>               goto err;
>>       }
>> 
>>       creds = kcalloc(el->s_numdevs, sizeof(*creds), GFP_KERNEL);
>>       if (!creds) {
>> -             err = -ENOMEM;
>> +             err = nfserr_layouttrylater;
>>               goto err;
>>       }
>> 
>> @@ -134,8 +134,10 @@ static int exofs_layout_get(
>>       layout.olo_comps = creds;
>> 
>>       err = pnfs_osd_xdr_encode_layout(xdr, &layout);
>> -     if (err)
>> +     if (err) {
>> +             err = nfserr_toosmall; /*FIXME: Change osd_xdr error
> codes */
>>               goto err;
>> +     }
>> 
>>       exp_xdr_encode_opaque_len(start, xdr->p);
>> 
>> @@ -146,7 +148,7 @@ static int exofs_layout_get(
>>       spin_unlock(&oi->i_layout_lock);
>> 
>>       if (in_recall)
>> -             err = -EAGAIN;
>> +             err = nfserr_layouttrylater;
>> 
>>  err:
>>       kfree(creds);
> 
> _______________________________________________
> pNFS mailing list
> pNFS@linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
> 

Re: [pnfs] [PATCH] SQUASHME: pnfsd-exofs: Change layoutget return codes

[J. Bruce Fields]

On Wed, Feb 10, 2010 at 12:45:12PM +0200, Boaz Harrosh wrote:
> On 02/10/2010 10:24 AM, Boaz Harrosh wrote:
> > 
> > Dependent on patch from Andy:
> > 	[PATCH 1/6] pnfsd: fix file system API layout_get error codes
> > 
> > Change codes to nfs4.1 codes
> > 
> > Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
> 
> Rrrr I spoke to soon.
> 
> Andy, Benny
> This will not work, currently. All the nfserr_xxx constants are defined
> if fs/nfsd/nfsd.h. 
> (Why do they exist at all, why can't we use the client's definitions for these?)
> 
> At the minimum they need to move to include/linux/nfsd/export.h. But I say kill them
> and use these from include/linux/nfs4.h. Added bonus these are enums, so prototype
> of .layout_get() can change to return enum nfsstat4 and the compiler fixes all our
> bugs.
> 
> I'm posting a second patch that uses "enum nfsstat4" constants in exofs which will
> work just fine, but is really ugly on the documentation aspect of Andy's patch.
> 
> Bruce may I submit a patch that globally gets rid of all nfserr_* defines and uses
> NFS4ERR_* in their place?

And ditto for v2/v3 errors?

Note you'll also need to track down all the function return types and
the variables that hold errors, and change them from __be32 to something
else.  (Hopefully not int, as the distinction between nfs errors and
-ERRNO errors is useful.)

I agree that it would be better not to have two entirely separate sets
of errors defines.

It's a large change that will conflict with everything, but as it's
currently Benny that will probably have the most fixing-up to do I guess
I'd defer to him to weigh the pain caused by that.

--b.

Re: [pnfs] [PATCH] SQUASHME: pnfsd-exofs: Change layoutget return codes

[Benny Halevy]

On Feb. 11, 2010, 18:38 +0200, "J. Bruce Fields" <bfields@citi.umich.edu> wrote:
> On Wed, Feb 10, 2010 at 12:45:12PM +0200, Boaz Harrosh wrote:
>> On 02/10/2010 10:24 AM, Boaz Harrosh wrote:
>>> Dependent on patch from Andy:
>>> 	[PATCH 1/6] pnfsd: fix file system API layout_get error codes
>>>
>>> Change codes to nfs4.1 codes
>>>
>>> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
>> Rrrr I spoke to soon.
>>
>> Andy, Benny
>> This will not work, currently. All the nfserr_xxx constants are defined
>> if fs/nfsd/nfsd.h. 
>> (Why do they exist at all, why can't we use the client's definitions for these?)
>>
>> At the minimum they need to move to include/linux/nfsd/export.h. But I say kill them
>> and use these from include/linux/nfs4.h. Added bonus these are enums, so prototype
>> of .layout_get() can change to return enum nfsstat4 and the compiler fixes all our
>> bugs.
>>
>> I'm posting a second patch that uses "enum nfsstat4" constants in exofs which will
>> work just fine, but is really ugly on the documentation aspect of Andy's patch.
>>
>> Bruce may I submit a patch that globally gets rid of all nfserr_* defines and uses
>> NFS4ERR_* in their place?
> 
> And ditto for v2/v3 errors?
> 
> Note you'll also need to track down all the function return types and
> the variables that hold errors, and change them from __be32 to something
> else.  (Hopefully not int, as the distinction between nfs errors and
> -ERRNO errors is useful.)
> 
> I agree that it would be better not to have two entirely separate sets
> of errors defines.
> 
> It's a large change that will conflict with everything, but as it's
> currently Benny that will probably have the most fixing-up to do I guess
> I'd defer to him to weigh the pain caused by that.

I think that such a clean up internally to the nfsd layer is really
low priority.  However, new pnfs code, beyond the superblock api,
should be cleaner and avoid using the internal, encoded constants.

We just saw how confusing it is, when the DLM layout could've returned
a mix of be32 error codes, negative host-order nfs4.1 errors codes, and
negative system errors. Latter two, erronuously from filelayout_encode_layout.

Note that filelayout_encode_layout can be called both from the dlm
implementation that is technically a part of nfsd, but from spnfs
and other file systems supporting the file layout so it better
agree on one simple convention.

Benny

> 
> --b.
> _______________________________________________
> pNFS mailing list
> pNFS@linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs

[PATCH version2] SQUASHME: pnfsd-exofs: Change layoutget return codes

[Boaz Harrosh]

Dependent on patch from Andy:
	[PATCH 1/6] pnfsd: fix file system API layout_get error codes

Change codes to nfs4.1 codes

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 fs/exofs/export.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/exofs/export.c b/fs/exofs/export.c
index be3d54e..4a4e3a0 100644
--- a/fs/exofs/export.c
+++ b/fs/exofs/export.c
@@ -95,13 +95,13 @@ static int exofs_layout_get(
 	/* skip opaque size, will be filled-in later */
 	start = exp_xdr_reserve_qwords(xdr, 1);
 	if (!start) {
-		err = -E2BIG;
+		err = NFS4ERR_TOOSMALL;
 		goto err;
 	}
 
 	creds = kcalloc(el->s_numdevs, sizeof(*creds), GFP_KERNEL);
 	if (!creds) {
-		err = -ENOMEM;
+		err = NFS4ERR_LAYOUTTRYLATER;
 		goto err;
 	}
 
@@ -134,8 +134,10 @@ static int exofs_layout_get(
 	layout.olo_comps = creds;
 
 	err = pnfs_osd_xdr_encode_layout(xdr, &layout);
-	if (err)
+	if (err) {
+		err = NFS4ERR_TOOSMALL; /*FIXME: Change osd_xdr error codes */
 		goto err;
+	}
 
 	exp_xdr_encode_opaque_len(start, xdr->p);
 
@@ -146,7 +148,7 @@ static int exofs_layout_get(
 	spin_unlock(&oi->i_layout_lock);
 
 	if (in_recall)
-		err = -EAGAIN;
+		err = NFS4ERR_LAYOUTTRYLATER;
 
 err:
 	kfree(creds);
-- 
1.6.6

Re: [PATCH version2] SQUASHME: pnfsd-exofs: Change layoutget return codes

[Benny Halevy]

On Feb. 10, 2010, 12:47 +0200, Boaz Harrosh <bharrosh@panasas.com> wrote:
> Dependent on patch from Andy:
> 	[PATCH 1/6] pnfsd: fix file system API layout_get error codes
> 
> Change codes to nfs4.1 codes
> 
> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
> ---
>  fs/exofs/export.c |   10 ++++++----
>  1 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/exofs/export.c b/fs/exofs/export.c
> index be3d54e..4a4e3a0 100644
> --- a/fs/exofs/export.c
> +++ b/fs/exofs/export.c
> @@ -95,13 +95,13 @@ static int exofs_layout_get(
>  	/* skip opaque size, will be filled-in later */
>  	start = exp_xdr_reserve_qwords(xdr, 1);
>  	if (!start) {
> -		err = -E2BIG;
> +		err = NFS4ERR_TOOSMALL;
>  		goto err;
>  	}
>  
>  	creds = kcalloc(el->s_numdevs, sizeof(*creds), GFP_KERNEL);
>  	if (!creds) {
> -		err = -ENOMEM;
> +		err = NFS4ERR_LAYOUTTRYLATER;
>  		goto err;
>  	}
>  
> @@ -134,8 +134,10 @@ static int exofs_layout_get(
>  	layout.olo_comps = creds;
>  
>  	err = pnfs_osd_xdr_encode_layout(xdr, &layout);
> -	if (err)
> +	if (err) {
> +		err = NFS4ERR_TOOSMALL; /*FIXME: Change osd_xdr error codes */
>  		goto err;
> +	}
>  
>  	exp_xdr_encode_opaque_len(start, xdr->p);
>  
> @@ -146,7 +148,7 @@ static int exofs_layout_get(
>  	spin_unlock(&oi->i_layout_lock);
>  
>  	if (in_recall)
> -		err = -EAGAIN;
> +		err = NFS4ERR_LAYOUTTRYLATER;

That should be NFS4ERR_RECALLCONFLICT...
I'll fix that in my tree.

Benny

>  
>  err:
>  	kfree(creds);