All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] pnfs  support servers with multiple layout types
@ 2016-03-02 12:08 Tigran Mkrtchyan
  2016-03-02 12:47 ` kbuild test robot
  2016-03-02 13:57 ` Tigran Mkrtchyan
  0 siblings, 2 replies; 9+ messages in thread
From: Tigran Mkrtchyan @ 2016-03-02 12:08 UTC (permalink / raw)
  To: trond.myklebust; +Cc: linux-nfs, Tigran Mkrtchyan

current NFSv4.1/pNFS client assumes that MDS supports
only one layout type. While it's true for most existing servers,
nevertheless, this can be change in the near future.

This patch is an attempt to multi layouttype MDS support. To make
it possible for such servers to function with existing clients,
server must always send default layout type first in the list. The
client starts processing layout types starting from the second element
and will fall back to the wfirst one, if none of presented types
is supported.

Testing done:

  - started a server with nfs4_file and flex_file layout
  - new kernel picked flexr_-file layout
  - old complained about multiple layout types and proceeded  nfs4_file layout

Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
---
 fs/nfs/client.c         |  2 +-
 fs/nfs/nfs4xdr.c        | 23 ++++++++++-------------
 fs/nfs/pnfs.c           | 45 ++++++++++++++++++++++++++++++++-------------
 fs/nfs/pnfs.h           |  2 +-
 include/linux/nfs_xdr.h |  8 +++++++-
 5 files changed, 51 insertions(+), 29 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index d6d5d2a..ec8e7db 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -787,7 +787,7 @@ int nfs_probe_fsinfo(struct nfs_server *server, struct nfs_fh *mntfh, struct nfs
 	}
 
 	fsinfo.fattr = fattr;
-	fsinfo.layouttype = 0;
+	memset(fsinfo.layouttype, 0, sizeof(fsinfo.layouttype));
 	error = clp->rpc_ops->fsinfo(server, mntfh, &fsinfo);
 	if (error < 0)
 		goto out_error;
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 4e44412..ac8747b 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -4702,14 +4702,13 @@ static int decode_getfattr(struct xdr_stream *xdr, struct nfs_fattr *fattr,
 }
 
 /*
- * Decode potentially multiple layout types. Currently we only support
- * one layout driver per file system.
+ * Decode potentially multiple layout types.
  */
-static int decode_first_pnfs_layout_type(struct xdr_stream *xdr,
+static int decode_pnfs_layout_types(struct xdr_stream *xdr,
 					 uint32_t *layouttype)
 {
 	__be32 *p;
-	int num;
+	uint32_t num, i;
 
 	p = xdr_inline_decode(xdr, 4);
 	if (unlikely(!p))
@@ -4718,18 +4717,17 @@ static int decode_first_pnfs_layout_type(struct xdr_stream *xdr,
 
 	/* pNFS is not supported by the underlying file system */
 	if (num == 0) {
-		*layouttype = 0;
 		return 0;
 	}
-	if (num > 1)
-		printk(KERN_INFO "NFS: %s: Warning: Multiple pNFS layout "
-			"drivers per filesystem not supported\n", __func__);
+	if (num > NFS_MAX_LAYOUT_TYPES)
+		printk(KERN_INFO "NFS: %s: Warning: Too many (%d) pNFS layout types\n", __func__, num);
 
 	/* Decode and set first layout type, move xdr->p past unused types */
 	p = xdr_inline_decode(xdr, num * 4);
 	if (unlikely(!p))
 		goto out_overflow;
-	*layouttype = be32_to_cpup(p);
+	for(i = 0; i < num && i < NFS_MAX_LAYOUT_TYPES; i++)
+		layouttype[i] = be32_to_cpup(p++);
 	return 0;
 out_overflow:
 	print_overflow_msg(__func__, xdr);
@@ -4749,10 +4747,9 @@ static int decode_attr_pnfstype(struct xdr_stream *xdr, uint32_t *bitmap,
 	if (unlikely(bitmap[1] & (FATTR4_WORD1_FS_LAYOUT_TYPES - 1U)))
 		return -EIO;
 	if (bitmap[1] & FATTR4_WORD1_FS_LAYOUT_TYPES) {
-		status = decode_first_pnfs_layout_type(xdr, layouttype);
+		status = decode_pnfs_layout_types(xdr, layouttype);
 		bitmap[1] &= ~FATTR4_WORD1_FS_LAYOUT_TYPES;
-	} else
-		*layouttype = 0;
+	}
 	return status;
 }
 
@@ -4833,7 +4830,7 @@ static int decode_fsinfo(struct xdr_stream *xdr, struct nfs_fsinfo *fsinfo)
 	status = decode_attr_time_delta(xdr, bitmap, &fsinfo->time_delta);
 	if (status != 0)
 		goto xdr_error;
-	status = decode_attr_pnfstype(xdr, bitmap, &fsinfo->layouttype);
+	status = decode_attr_pnfstype(xdr, bitmap, fsinfo->layouttype);
 	if (status != 0)
 		goto xdr_error;
 
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 2fa483e..6b3b67a 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -102,32 +102,51 @@ unset_pnfs_layoutdriver(struct nfs_server *nfss)
  * Try to set the server's pnfs module to the pnfs layout type specified by id.
  * Currently only one pNFS layout driver per filesystem is supported.
  *
- * @id layout type. Zero (illegal layout type) indicates pNFS not in use.
+ * @ids array of layout types supported by MDS.
  */
 void
 set_pnfs_layoutdriver(struct nfs_server *server, const struct nfs_fh *mntfh,
-		      u32 id)
+		      u32 *ids)
 {
 	struct pnfs_layoutdriver_type *ld_type = NULL;
+	u32 id;
+	int i;
 
-	if (id == 0)
-		goto out_no_driver;
 	if (!(server->nfs_client->cl_exchange_flags &
 		 (EXCHGID4_FLAG_USE_NON_PNFS | EXCHGID4_FLAG_USE_PNFS_MDS))) {
-		printk(KERN_ERR "NFS: %s: id %u cl_exchange_flags 0x%x\n",
-			__func__, id, server->nfs_client->cl_exchange_flags);
+		printk(KERN_ERR "NFS: %s: cl_exchange_flags 0x%x\n",
+			__func__, server->nfs_client->cl_exchange_flags);
 		goto out_no_driver;
 	}
-	ld_type = find_pnfs_driver(id);
-	if (!ld_type) {
+	/*
+	 * If server supports more than one layout types.
+	 * By assuming, that server will put 'common default' as the first
+	 * entry, try all following entries ibefore and fall back to the default
+	 * if we did not found a matching one.
+	 */
+	for(i = 1; i < NFS_MAX_LAYOUT_TYPES && ids[i] != 0; i++) {
+		id = ids[i];
 		request_module("%s-%u", LAYOUT_NFSV4_1_MODULE_PREFIX, id);
 		ld_type = find_pnfs_driver(id);
-		if (!ld_type) {
-			dprintk("%s: No pNFS module found for %u.\n",
-				__func__, id);
-			goto out_no_driver;
-		}
+		if(ld_type)
+			goto found_module;
+
+		dprintk("%s: No pNFS module found for %u.\n", __func__, id);
+	}
+
+	/*
+	 * no other layout types found. Try default one.
+	 */
+	id = ids[0];
+	request_module("%s-%u", LAYOUT_NFSV4_1_MODULE_PREFIX, id);
+	ld_type = find_pnfs_driver(id);
+
+	if (!ld_type) {
+		dprintk("%s: No pNFS module found for %u.\n", __func__, id);
+		goto out_no_driver;
 	}
+
+found_module:
 	server->pnfs_curr_ld = ld_type;
 	if (ld_type->set_layoutdriver
 	    && ld_type->set_layoutdriver(server, mntfh)) {
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index 1ac1db5..5d7d9e0 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -234,7 +234,7 @@ void pnfs_get_layout_hdr(struct pnfs_layout_hdr *lo);
 void pnfs_put_lseg(struct pnfs_layout_segment *lseg);
 void pnfs_put_lseg_locked(struct pnfs_layout_segment *lseg);
 
-void set_pnfs_layoutdriver(struct nfs_server *, const struct nfs_fh *, u32);
+void set_pnfs_layoutdriver(struct nfs_server *, const struct nfs_fh *, u32 *);
 void unset_pnfs_layoutdriver(struct nfs_server *);
 void pnfs_generic_pg_init_read(struct nfs_pageio_descriptor *, struct nfs_page *);
 int pnfs_generic_pg_readpages(struct nfs_pageio_descriptor *desc);
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index d320906..e45bcb9 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -125,6 +125,12 @@ struct nfs_fattr {
 		| NFS_ATTR_FATTR_V4_SECURITY_LABEL)
 
 /*
+ * Maximal number of supported layout drivers.
+ */
+#define NFS_MAX_LAYOUT_TYPES 8
+
+
+/*
  * Info on the file system
  */
 struct nfs_fsinfo {
@@ -139,7 +145,7 @@ struct nfs_fsinfo {
 	__u64			maxfilesize;
 	struct timespec		time_delta; /* server time granularity */
 	__u32			lease_time; /* in seconds */
-	__u32			layouttype; /* supported pnfs layout driver */
+	__u32			layouttype[NFS_MAX_LAYOUT_TYPES]; /* supported pnfs layout driver */
 	__u32			blksize; /* preferred pnfs io block size */
 	__u32			clone_blksize; /* granularity of a CLONE operation */
 };
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] pnfs support servers with multiple layout types
  2016-03-02 12:08 [PATCH] pnfs support servers with multiple layout types Tigran Mkrtchyan
@ 2016-03-02 12:47 ` kbuild test robot
  2016-03-02 13:57 ` Tigran Mkrtchyan
  1 sibling, 0 replies; 9+ messages in thread
From: kbuild test robot @ 2016-03-02 12:47 UTC (permalink / raw)
  To: Tigran Mkrtchyan; +Cc: kbuild-all, trond.myklebust, linux-nfs, Tigran Mkrtchyan

[-- Attachment #1: Type: text/plain, Size: 3312 bytes --]

Hi Tigran,

[auto build test WARNING on v4.5-rc6]
[also build test WARNING on next-20160302]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Tigran-Mkrtchyan/pnfs-support-servers-with-multiple-layout-types/20160302-201105
config: m68k-sun3_defconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=m68k 

All warnings (new ones prefixed by >>):

   fs/nfs/nfs4proc.c: In function 'nfs4_proc_fsinfo':
>> fs/nfs/nfs4proc.c:4232:3: warning: passing argument 3 of 'set_pnfs_layoutdriver' makes integer from pointer without a cast
      set_pnfs_layoutdriver(server, fhandle, fsinfo->layouttype);
      ^
   In file included from fs/nfs/nfs4proc.c:64:0:
   fs/nfs/pnfs.h:657:20: note: expected 'u32' but argument is of type '__u32 *'
    static inline void set_pnfs_layoutdriver(struct nfs_server *s,
                       ^

vim +/set_pnfs_layoutdriver +4232 fs/nfs/nfs4proc.c

83ca7f5a Chuck Lever     2013-03-16  4216  			break;
83ca7f5a Chuck Lever     2013-03-16  4217  		}
83ca7f5a Chuck Lever     2013-03-16  4218  		err = nfs4_handle_exception(server, err, &exception);
^1da177e Linus Torvalds  2005-04-16  4219  	} while (exception.retry);
^1da177e Linus Torvalds  2005-04-16  4220  	return err;
^1da177e Linus Torvalds  2005-04-16  4221  }
^1da177e Linus Torvalds  2005-04-16  4222  
^1da177e Linus Torvalds  2005-04-16  4223  static int nfs4_proc_fsinfo(struct nfs_server *server, struct nfs_fh *fhandle, struct nfs_fsinfo *fsinfo)
^1da177e Linus Torvalds  2005-04-16  4224  {
e38eb650 Bryan Schumaker 2012-06-20  4225  	int error;
e38eb650 Bryan Schumaker 2012-06-20  4226  
0e574af1 Trond Myklebust 2005-10-27  4227  	nfs_fattr_init(fsinfo->fattr);
e38eb650 Bryan Schumaker 2012-06-20  4228  	error = nfs4_do_fsinfo(server, fhandle, fsinfo);
dc182549 Peng Tao        2012-08-24  4229  	if (error == 0) {
dc182549 Peng Tao        2012-08-24  4230  		/* block layout checks this! */
dc182549 Peng Tao        2012-08-24  4231  		server->pnfs_blksize = fsinfo->blksize;
e38eb650 Bryan Schumaker 2012-06-20 @4232  		set_pnfs_layoutdriver(server, fhandle, fsinfo->layouttype);
dc182549 Peng Tao        2012-08-24  4233  	}
e38eb650 Bryan Schumaker 2012-06-20  4234  
e38eb650 Bryan Schumaker 2012-06-20  4235  	return error;
^1da177e Linus Torvalds  2005-04-16  4236  }
^1da177e Linus Torvalds  2005-04-16  4237  
^1da177e Linus Torvalds  2005-04-16  4238  static int _nfs4_proc_pathconf(struct nfs_server *server, struct nfs_fh *fhandle,
^1da177e Linus Torvalds  2005-04-16  4239  		struct nfs_pathconf *pathconf)
^1da177e Linus Torvalds  2005-04-16  4240  {

:::::: The code at line 4232 was first introduced by commit
:::::: e38eb6506ff426a2bb93433fecfcc863a95fcd03 NFS: set_pnfs_layoutdriver() from nfs4_proc_fsinfo()

:::::: TO: Bryan Schumaker <bjschuma@netapp.com>
:::::: CC: Trond Myklebust <Trond.Myklebust@netapp.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 11191 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] pnfs  support servers with multiple layout types
  2016-03-02 12:08 [PATCH] pnfs support servers with multiple layout types Tigran Mkrtchyan
  2016-03-02 12:47 ` kbuild test robot
@ 2016-03-02 13:57 ` Tigran Mkrtchyan
  2016-03-02 16:37   ` Christoph Hellwig
  1 sibling, 1 reply; 9+ messages in thread
From: Tigran Mkrtchyan @ 2016-03-02 13:57 UTC (permalink / raw)
  To: trond.myklebust; +Cc: linux-nfs, Tigran Mkrtchyan

current NFSv4.1/pNFS client assumes that MDS supports
only one layout type. While it's true for most existing servers,
nevertheless, this can be change in the near future.

This patch is an attempt to multi layouttype MDS support. To make
it possible for such servers to function with existing clients,
server must always send default layout type first in the list. The
client starts processing layout types starting from the second element
and will fall back to the wfirst one, if none of presented types
is supported.

Testing done:

  - started a server with nfs4_file and flex_file layout
  - new kernel picked flexr_-file layout
  - old complained about multiple layout types and proceeded  nfs4_file layout

Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
---
 fs/nfs/client.c         |  2 +-
 fs/nfs/nfs4xdr.c        | 23 ++++++++++-------------
 fs/nfs/pnfs.c           | 45 ++++++++++++++++++++++++++++++++-------------
 fs/nfs/pnfs.h           |  4 ++--
 include/linux/nfs_xdr.h |  8 +++++++-
 5 files changed, 52 insertions(+), 30 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index d6d5d2a..ec8e7db 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -787,7 +787,7 @@ int nfs_probe_fsinfo(struct nfs_server *server, struct nfs_fh *mntfh, struct nfs
 	}
 
 	fsinfo.fattr = fattr;
-	fsinfo.layouttype = 0;
+	memset(fsinfo.layouttype, 0, sizeof(fsinfo.layouttype));
 	error = clp->rpc_ops->fsinfo(server, mntfh, &fsinfo);
 	if (error < 0)
 		goto out_error;
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 4e44412..ac8747b 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -4702,14 +4702,13 @@ static int decode_getfattr(struct xdr_stream *xdr, struct nfs_fattr *fattr,
 }
 
 /*
- * Decode potentially multiple layout types. Currently we only support
- * one layout driver per file system.
+ * Decode potentially multiple layout types.
  */
-static int decode_first_pnfs_layout_type(struct xdr_stream *xdr,
+static int decode_pnfs_layout_types(struct xdr_stream *xdr,
 					 uint32_t *layouttype)
 {
 	__be32 *p;
-	int num;
+	uint32_t num, i;
 
 	p = xdr_inline_decode(xdr, 4);
 	if (unlikely(!p))
@@ -4718,18 +4717,17 @@ static int decode_first_pnfs_layout_type(struct xdr_stream *xdr,
 
 	/* pNFS is not supported by the underlying file system */
 	if (num == 0) {
-		*layouttype = 0;
 		return 0;
 	}
-	if (num > 1)
-		printk(KERN_INFO "NFS: %s: Warning: Multiple pNFS layout "
-			"drivers per filesystem not supported\n", __func__);
+	if (num > NFS_MAX_LAYOUT_TYPES)
+		printk(KERN_INFO "NFS: %s: Warning: Too many (%d) pNFS layout types\n", __func__, num);
 
 	/* Decode and set first layout type, move xdr->p past unused types */
 	p = xdr_inline_decode(xdr, num * 4);
 	if (unlikely(!p))
 		goto out_overflow;
-	*layouttype = be32_to_cpup(p);
+	for(i = 0; i < num && i < NFS_MAX_LAYOUT_TYPES; i++)
+		layouttype[i] = be32_to_cpup(p++);
 	return 0;
 out_overflow:
 	print_overflow_msg(__func__, xdr);
@@ -4749,10 +4747,9 @@ static int decode_attr_pnfstype(struct xdr_stream *xdr, uint32_t *bitmap,
 	if (unlikely(bitmap[1] & (FATTR4_WORD1_FS_LAYOUT_TYPES - 1U)))
 		return -EIO;
 	if (bitmap[1] & FATTR4_WORD1_FS_LAYOUT_TYPES) {
-		status = decode_first_pnfs_layout_type(xdr, layouttype);
+		status = decode_pnfs_layout_types(xdr, layouttype);
 		bitmap[1] &= ~FATTR4_WORD1_FS_LAYOUT_TYPES;
-	} else
-		*layouttype = 0;
+	}
 	return status;
 }
 
@@ -4833,7 +4830,7 @@ static int decode_fsinfo(struct xdr_stream *xdr, struct nfs_fsinfo *fsinfo)
 	status = decode_attr_time_delta(xdr, bitmap, &fsinfo->time_delta);
 	if (status != 0)
 		goto xdr_error;
-	status = decode_attr_pnfstype(xdr, bitmap, &fsinfo->layouttype);
+	status = decode_attr_pnfstype(xdr, bitmap, fsinfo->layouttype);
 	if (status != 0)
 		goto xdr_error;
 
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 2fa483e..6b3b67a 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -102,32 +102,51 @@ unset_pnfs_layoutdriver(struct nfs_server *nfss)
  * Try to set the server's pnfs module to the pnfs layout type specified by id.
  * Currently only one pNFS layout driver per filesystem is supported.
  *
- * @id layout type. Zero (illegal layout type) indicates pNFS not in use.
+ * @ids array of layout types supported by MDS.
  */
 void
 set_pnfs_layoutdriver(struct nfs_server *server, const struct nfs_fh *mntfh,
-		      u32 id)
+		      u32 *ids)
 {
 	struct pnfs_layoutdriver_type *ld_type = NULL;
+	u32 id;
+	int i;
 
-	if (id == 0)
-		goto out_no_driver;
 	if (!(server->nfs_client->cl_exchange_flags &
 		 (EXCHGID4_FLAG_USE_NON_PNFS | EXCHGID4_FLAG_USE_PNFS_MDS))) {
-		printk(KERN_ERR "NFS: %s: id %u cl_exchange_flags 0x%x\n",
-			__func__, id, server->nfs_client->cl_exchange_flags);
+		printk(KERN_ERR "NFS: %s: cl_exchange_flags 0x%x\n",
+			__func__, server->nfs_client->cl_exchange_flags);
 		goto out_no_driver;
 	}
-	ld_type = find_pnfs_driver(id);
-	if (!ld_type) {
+	/*
+	 * If server supports more than one layout types.
+	 * By assuming, that server will put 'common default' as the first
+	 * entry, try all following entries ibefore and fall back to the default
+	 * if we did not found a matching one.
+	 */
+	for(i = 1; i < NFS_MAX_LAYOUT_TYPES && ids[i] != 0; i++) {
+		id = ids[i];
 		request_module("%s-%u", LAYOUT_NFSV4_1_MODULE_PREFIX, id);
 		ld_type = find_pnfs_driver(id);
-		if (!ld_type) {
-			dprintk("%s: No pNFS module found for %u.\n",
-				__func__, id);
-			goto out_no_driver;
-		}
+		if(ld_type)
+			goto found_module;
+
+		dprintk("%s: No pNFS module found for %u.\n", __func__, id);
+	}
+
+	/*
+	 * no other layout types found. Try default one.
+	 */
+	id = ids[0];
+	request_module("%s-%u", LAYOUT_NFSV4_1_MODULE_PREFIX, id);
+	ld_type = find_pnfs_driver(id);
+
+	if (!ld_type) {
+		dprintk("%s: No pNFS module found for %u.\n", __func__, id);
+		goto out_no_driver;
 	}
+
+found_module:
 	server->pnfs_curr_ld = ld_type;
 	if (ld_type->set_layoutdriver
 	    && ld_type->set_layoutdriver(server, mntfh)) {
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index 1ac1db5..bfb5f3f 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -234,7 +234,7 @@ void pnfs_get_layout_hdr(struct pnfs_layout_hdr *lo);
 void pnfs_put_lseg(struct pnfs_layout_segment *lseg);
 void pnfs_put_lseg_locked(struct pnfs_layout_segment *lseg);
 
-void set_pnfs_layoutdriver(struct nfs_server *, const struct nfs_fh *, u32);
+void set_pnfs_layoutdriver(struct nfs_server *, const struct nfs_fh *, u32 *);
 void unset_pnfs_layoutdriver(struct nfs_server *);
 void pnfs_generic_pg_init_read(struct nfs_pageio_descriptor *, struct nfs_page *);
 int pnfs_generic_pg_readpages(struct nfs_pageio_descriptor *desc);
@@ -655,7 +655,7 @@ pnfs_wait_on_layoutreturn(struct inode *ino, struct rpc_task *task)
 }
 
 static inline void set_pnfs_layoutdriver(struct nfs_server *s,
-					 const struct nfs_fh *mntfh, u32 id)
+					 const struct nfs_fh *mntfh, u32 *ids)
 {
 }
 
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index d320906..e45bcb9 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -125,6 +125,12 @@ struct nfs_fattr {
 		| NFS_ATTR_FATTR_V4_SECURITY_LABEL)
 
 /*
+ * Maximal number of supported layout drivers.
+ */
+#define NFS_MAX_LAYOUT_TYPES 8
+
+
+/*
  * Info on the file system
  */
 struct nfs_fsinfo {
@@ -139,7 +145,7 @@ struct nfs_fsinfo {
 	__u64			maxfilesize;
 	struct timespec		time_delta; /* server time granularity */
 	__u32			lease_time; /* in seconds */
-	__u32			layouttype; /* supported pnfs layout driver */
+	__u32			layouttype[NFS_MAX_LAYOUT_TYPES]; /* supported pnfs layout driver */
 	__u32			blksize; /* preferred pnfs io block size */
 	__u32			clone_blksize; /* granularity of a CLONE operation */
 };
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] pnfs  support servers with multiple layout types
  2016-03-02 13:57 ` Tigran Mkrtchyan
@ 2016-03-02 16:37   ` Christoph Hellwig
  2016-03-02 19:22     ` Mkrtchyan, Tigran
  2016-03-03 13:50     ` Mkrtchyan, Tigran
  0 siblings, 2 replies; 9+ messages in thread
From: Christoph Hellwig @ 2016-03-02 16:37 UTC (permalink / raw)
  To: Tigran Mkrtchyan; +Cc: trond.myklebust, linux-nfs

> +	for(i = 0; i < num && i < NFS_MAX_LAYOUT_TYPES; i++)
> +		layouttype[i] = be32_to_cpup(p++);

Can we use a bitmap and do something like

	layouttype |= (1 << be32_to_cpup(p++));

that's what I did for my unsubmitted patches to submit multiple
layouttypes in the Linux server..

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] pnfs  support servers with multiple layout types
  2016-03-02 16:37   ` Christoph Hellwig
@ 2016-03-02 19:22     ` Mkrtchyan, Tigran
  2016-03-03 13:50     ` Mkrtchyan, Tigran
  1 sibling, 0 replies; 9+ messages in thread
From: Mkrtchyan, Tigran @ 2016-03-02 19:22 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Trond Myklebust, linux-nfs



----- Original Message -----
> From: "Christoph Hellwig" <hch@infradead.org>
> To: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de>
> Cc: "Trond Myklebust" <trond.myklebust@primarydata.com>, linux-nfs@vger.kernel.org
> Sent: Wednesday, March 2, 2016 5:37:46 PM
> Subject: Re: [PATCH] pnfs  support servers with multiple layout types

>> +	for(i = 0; i < num && i < NFS_MAX_LAYOUT_TYPES; i++)
>> +		layouttype[i] = be32_to_cpup(p++);
> 
> Can we use a bitmap and do something like
> 
>	layouttype |= (1 << be32_to_cpup(p++));
> 

Sure, if we describe in spec that layout type must be power of two
and max value is 32.

Tigran.

> that's what I did for my unsubmitted patches to submit multiple
> layouttypes in the Linux server..
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] pnfs  support servers with multiple layout types
  2016-03-02 16:37   ` Christoph Hellwig
  2016-03-02 19:22     ` Mkrtchyan, Tigran
@ 2016-03-03 13:50     ` Mkrtchyan, Tigran
  1 sibling, 0 replies; 9+ messages in thread
From: Mkrtchyan, Tigran @ 2016-03-03 13:50 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Trond Myklebust, linux-nfs



----- Original Message -----
> From: "Christoph Hellwig" <hch@infradead.org>
> To: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de>
> Cc: "Trond Myklebust" <trond.myklebust@primarydata.com>, linux-nfs@vger.kernel.org
> Sent: Wednesday, March 2, 2016 5:37:46 PM
> Subject: Re: [PATCH] pnfs  support servers with multiple layout types

>> +	for(i = 0; i < num && i < NFS_MAX_LAYOUT_TYPES; i++)
>> +		layouttype[i] = be32_to_cpup(p++);
> 
> Can we use a bitmap and do something like
> 
>	layouttype |= (1 << be32_to_cpup(p++));
> 

Sorry, took me a bit longer to understand what you actually was saying.
But still won't work, as we will loose the order of supported layouts.

To make it working, we can add a mount option for preferred layout:

mount -o preferred_layout=nfs4_file,vers=4.1 ....

Then client will try preferred first, and if it's not provided, 
then will go through the bitmask and pick first working.

Shall I do that? Trond, any comments?

Tigran.

> that's what I did for my unsubmitted patches to submit multiple
> layouttypes in the Linux server..
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] pnfs  support servers with multiple layout types
  2015-09-22 22:31 ` Trond Myklebust
@ 2015-09-23  6:38   ` Mkrtchyan, Tigran
  0 siblings, 0 replies; 9+ messages in thread
From: Mkrtchyan, Tigran @ 2015-09-23  6:38 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: linux-nfs



----- Original Message -----
> From: "Trond Myklebust" <trond.myklebust@primarydata.com>
> To: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de>, linux-nfs@vger.kernel.org
> Sent: Wednesday, September 23, 2015 12:31:34 AM
> Subject: Re: [PATCH] pnfs  support servers with multiple layout types

> On Tue, 2015-09-22 at 15:45 +0200, Tigran Mkrtchyan wrote:
>> current NFSv4.1/pNFS client assumes that MDS supports
>> only one layout type. While it's true for most existing servers,
>> nevertheless, this can be change in the near future.
>> 
>> This patch is an attempt to multi layouttype MDS support.  To make
>> it possible for such servers still function with existing clients,
>> the layout
>> type list will be processed in reverse order. This gives old clients
>> still
>> pick the first entry, but newer client will pick last entry.  Proof:
>> 
>> 4.3.0-rc1-dirty
>> [  180.868764] nfs4flexfilelayout_init: NFSv4 Flexfile Layout Driver
>> Registering...
>> 
>> 2.6.32-573.3.1.el6.x86_64
>> nfs4filelayout_init: NFSv4 File Layout Driver Registering...
>> 
>> Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
>> ---
>>  fs/nfs/client.c         |  2 +-
>>  fs/nfs/nfs4xdr.c        | 22 +++++++++---------
>>  fs/nfs/pnfs.c           | 62 ++++++++++++++++++++++++++++++---------
>> ----------
>>  fs/nfs/pnfs.h           |  2 +-
>>  include/linux/nfs_xdr.h |  4 +++-
>>  5 files changed, 54 insertions(+), 38 deletions(-)
>> 
>> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
>> index 57c5a02..1145bc7 100644
>> --- a/fs/nfs/client.c
>> +++ b/fs/nfs/client.c
>> @@ -786,7 +786,7 @@ int nfs_probe_fsinfo(struct nfs_server *server,
>> struct nfs_fh *mntfh, struct nfs
>>  	}
>>  
>>  	fsinfo.fattr = fattr;
>> -	fsinfo.layouttype = 0;
>> +	memset(fsinfo.layouttype, 0, sizeof(fsinfo.layouttype));
>>  	error = clp->rpc_ops->fsinfo(server, mntfh, &fsinfo);
>>  	if (error < 0)
>>  		goto out_error;
>> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
>> index 788adf3..7fbd411 100644
>> --- a/fs/nfs/nfs4xdr.c
>> +++ b/fs/nfs/nfs4xdr.c
>> @@ -4688,14 +4688,14 @@ static int decode_getfattr(struct xdr_stream
>> *xdr, struct nfs_fattr *fattr,
>>  }
>>  
>>  /*
>> - * Decode potentially multiple layout types. Currently we only
>> support
>> - * one layout driver per file system.
>> + * Decode potentially multiple layout types.
>>   */
>> -static int decode_first_pnfs_layout_type(struct xdr_stream *xdr,
>> +static int decode_pnfs_layout_types(struct xdr_stream *xdr,
>>  					 uint32_t *layouttype)
>>  {
>>  	__be32 *p;
>>  	int num;
>> +	int i;
> 
> Make these both unsigned int?
> 
>>  
>>  	p = xdr_inline_decode(xdr, 4);
>>  	if (unlikely(!p))
>> @@ -4704,18 +4704,18 @@ static int
>> decode_first_pnfs_layout_type(struct xdr_stream *xdr,
>>  
>>  	/* pNFS is not supported by the underlying file system */
>>  	if (num == 0) {
>> -		*layouttype = 0;
>>  		return 0;
>>  	}
>> -	if (num > 1)
>> -		printk(KERN_INFO "NFS: %s: Warning: Multiple pNFS
>> layout "
>> -			"drivers per filesystem not supported\n",
>> __func__);
>> +	if (num > NFS_MAX_LAYOUT_TYPES)
>> +		printk(KERN_INFO "NFS: %s: Warning: Too many (%d)
>> pNFS layout types\n", __func__, num);
>>  
>>  	/* Decode and set first layout type, move xdr->p past unused
>> types */
>>  	p = xdr_inline_decode(xdr, num * 4);
>>  	if (unlikely(!p))
>>  		goto out_overflow;
>> -	*layouttype = be32_to_cpup(p);
>> +	for(i = 0; i < num && i < NFS_MAX_LAYOUT_TYPES; i++) {
>> +		layouttype[i] = be32_to_cpup(p++);
>> +	}
> 
> You don't need the { } block enclosure here.
> 
>>  	return 0;
>>  out_overflow:
>>  	print_overflow_msg(__func__, xdr);
>> @@ -4735,10 +4735,10 @@ static int decode_attr_pnfstype(struct
>> xdr_stream *xdr, uint32_t *bitmap,
>>  	if (unlikely(bitmap[1] & (FATTR4_WORD1_FS_LAYOUT_TYPES -
>> 1U)))
>>  		return -EIO;
>>  	if (bitmap[1] & FATTR4_WORD1_FS_LAYOUT_TYPES) {
>> -		status = decode_first_pnfs_layout_type(xdr,
>> layouttype);
>> +		status = decode_pnfs_layout_types(xdr, layouttype);
>>  		bitmap[1] &= ~FATTR4_WORD1_FS_LAYOUT_TYPES;
>>  	} else
>> -		*layouttype = 0;
>> +		memset(layouttype, 0, sizeof(*layouttype));
> 
> Isn't this already done by the caller in fs/nfs/client.c?
> 
>> 	return status;
>>  }
>>  
>> @@ -4792,7 +4792,7 @@ static int decode_fsinfo(struct xdr_stream
>> *xdr, struct nfs_fsinfo *fsinfo)
>>  	status = decode_attr_time_delta(xdr, bitmap, &fsinfo
>> ->time_delta);
>>  	if (status != 0)
>>  		goto xdr_error;
>> -	status = decode_attr_pnfstype(xdr, bitmap, &fsinfo
>> ->layouttype);
>> +	status = decode_attr_pnfstype(xdr, bitmap, fsinfo
>> ->layouttype);
>>  	if (status != 0)
>>  		goto xdr_error;
>>  	status = decode_attr_layout_blksize(xdr, bitmap, &fsinfo
>> ->blksize);
>> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
>> index ba12464..883bfa2 100644
>> --- a/fs/nfs/pnfs.c
>> +++ b/fs/nfs/pnfs.c
>> @@ -104,45 +104,59 @@ unset_pnfs_layoutdriver(struct nfs_server
>> *nfss)
>>   * Try to set the server's pnfs module to the pnfs layout type
>> specified by id.
>>   * Currently only one pNFS layout driver per filesystem is
>> supported.
>>   *
>> - * @id layout type. Zero (illegal layout type) indicates pNFS not in
>> use.
>> + * @ids array of layout types supported by MDS.
>>   */
>>  void
>>  set_pnfs_layoutdriver(struct nfs_server *server, const struct nfs_fh
>> *mntfh,
>> -		      u32 id)
>> +		      u32 *ids)
>>  {
>>  	struct pnfs_layoutdriver_type *ld_type = NULL;
>> +	u32 id;
>> +	int i;
>>  
>> -	if (id == 0)
>> -		goto out_no_driver;
>>  	if (!(server->nfs_client->cl_exchange_flags &
>>  		 (EXCHGID4_FLAG_USE_NON_PNFS |
>> EXCHGID4_FLAG_USE_PNFS_MDS))) {
>> -		printk(KERN_ERR "NFS: %s: id %u cl_exchange_flags
>> 0x%x\n",
>> -			__func__, id, server->nfs_client
>> ->cl_exchange_flags);
>> +		printk(KERN_ERR "NFS: %s: cl_exchange_flags 0x%x\n",
>> +			__func__, server->nfs_client
>> ->cl_exchange_flags);
>>  		goto out_no_driver;
>>  	}
>> -	ld_type = find_pnfs_driver(id);
>> -	if (!ld_type) {
>> -		request_module("%s-%u",
>> LAYOUT_NFSV4_1_MODULE_PREFIX, id);
>> +
>> +	/*
>> + 	 * walk in reverse ordet to use last in the list first.
>> +   	 * By assyming, that server will send most common type
>> first,
>> +   	 * most preferred one last, old client will be happy with
>> +   	 * a first entry, where new clients (4.4+) will pick the
>> +   	 * most advance one.
> 
> This implies that old clients will always pick the least preferred
yes, as with 100% probability they support only that one. RHEL6/7 clients
will always pick the first entry. Having them accessing the multi-layout
MDS enforce nfs_4_1_file be the first in the list even if you server
prefers flexfiles.


> layout type from the server's perspective. Ditto if newer clients get
> the value of NFS_MAX_LAYOUT_TYPES wrong.
> 
> Why not instead assume the following? 1st entry == default. Others are
> sorted in order of decreasing server preference.

we try all others first and if none of them works for us the first one is
taken? Works for me (as it effectively the same semantic - first entry last).


> 
>> + 	 */
>> +	for(i = NFS_MAX_LAYOUT_TYPES; i >=0; i--) {
>> +		id = ids[i];
>> +		if(!id)
>> +			continue;
>> +
>>  		ld_type = find_pnfs_driver(id);
>>  		if (!ld_type) {
>> -			dprintk("%s: No pNFS module found for
>> %u.\n",
>> -				__func__, id);
>> +			request_module("%s-%u",
>> LAYOUT_NFSV4_1_MODULE_PREFIX, id);
>> +			ld_type = find_pnfs_driver(id);
>> +			if (!ld_type) {
>> +				dprintk("%s: No pNFS module found
>> for %u.\n",
>> +					__func__, id);
>> +				continue;
>> +			}
>> +		}
>> +		server->pnfs_curr_ld = ld_type;
>> +		if (ld_type->set_layoutdriver
>> +		    && ld_type->set_layoutdriver(server, mntfh)) {
>> +			printk(KERN_ERR "NFS: %s: Error initializing
>> pNFS layout "
>> +				"driver %u.\n", __func__, id);
>> +			module_put(ld_type->owner);
>>  			goto out_no_driver;
>>  		}
>> -	}
>> -	server->pnfs_curr_ld = ld_type;
>> -	if (ld_type->set_layoutdriver
>> -	    && ld_type->set_layoutdriver(server, mntfh)) {
>> -		printk(KERN_ERR "NFS: %s: Error initializing pNFS
>> layout "
>> -			"driver %u.\n", __func__, id);
>> -		module_put(ld_type->owner);
>> -		goto out_no_driver;
>> -	}
>> -	/* Bump the MDS count */
>> -	atomic_inc(&server->nfs_client->cl_mds_count);
>> +		/* Bump the MDS count */
>> +		atomic_inc(&server->nfs_client->cl_mds_count);
>>  
>> -	dprintk("%s: pNFS module for %u set\n", __func__, id);
>> -	return;
>> +		dprintk("%s: pNFS module for %u set\n", __func__,
>> id);
>> +		return;
>> +	}
>>  
>>  out_no_driver:
>>  	dprintk("%s: Using NFSv4 I/O\n", __func__);
>> diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
>> index 78c9351..e7609c3 100644
>> --- a/fs/nfs/pnfs.h
>> +++ b/fs/nfs/pnfs.h
>> @@ -235,7 +235,7 @@ void pnfs_get_layout_hdr(struct pnfs_layout_hdr
>> *lo);
>>  void pnfs_put_lseg(struct pnfs_layout_segment *lseg);
>>  void pnfs_put_lseg_locked(struct pnfs_layout_segment *lseg);
>>  
>> -void set_pnfs_layoutdriver(struct nfs_server *, const struct nfs_fh
>> *, u32);
>> +void set_pnfs_layoutdriver(struct nfs_server *, const struct nfs_fh
>> *, u32 *);
>>  void unset_pnfs_layoutdriver(struct nfs_server *);
>>  void pnfs_generic_pg_init_read(struct nfs_pageio_descriptor *,
>> struct nfs_page *);
>>  int pnfs_generic_pg_readpages(struct nfs_pageio_descriptor *desc);
>> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
>> index 52faf7e..6c88745 100644
>> --- a/include/linux/nfs_xdr.h
>> +++ b/include/linux/nfs_xdr.h
>> @@ -124,6 +124,8 @@ struct nfs_fattr {
>>  		| NFS_ATTR_FATTR_SPACE_USED \
>>  		| NFS_ATTR_FATTR_V4_SECURITY_LABEL)
>>  
>> +#define NFS_MAX_LAYOUT_TYPES 32
> 
> There are only RFCs for 4 layout types today. Where does the number
> '32' come from?

the idea was that if in a future some server sends the following array of supported layouts types:
{
  fancy_layout1,
  fancy_layout2,
  fancy_layout3,
  fancy_layout4,
  fancy_layout5,
  fancy_layout6,
  nfs4_1_file
}

then you still want client to pick nfs4_1_file even if it's 7-th in the list. NFS_MAX_LAYOUT_TYPES is the
max number of layout types which we will consider to evaluate. 32 is a overkill of course. 

As an alternative, I can filter out all unknown layout types and result will fit into array of 4 ints.

I will update the patch and resend.

Thanks,
   Tigran.

> 
>> +
>>  /*
>>   * Info on the file system
>>   */
>> @@ -139,7 +141,7 @@ struct nfs_fsinfo {
>>  	__u64			maxfilesize;
>>  	struct timespec		time_delta; /* server time
>> granularity */
>>  	__u32			lease_time; /* in seconds */
>> -	__u32			layouttype; /* supported pnfs
>> layout driver */
>> +	__u32			layouttype[NFS_MAX_LAYOUT_TYPES
>> ]; /* supported pnfs layout drivers */
>>  	__u32			blksize; /* preferred pnfs io
>> block size */
>>  };
>>  
> --
> Trond Myklebust
> Linux NFS client maintainer, PrimaryData
> trond.myklebust@primarydata.com
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] pnfs  support servers with multiple layout types
  2015-09-22 13:45 Tigran Mkrtchyan
@ 2015-09-22 22:31 ` Trond Myklebust
  2015-09-23  6:38   ` Mkrtchyan, Tigran
  0 siblings, 1 reply; 9+ messages in thread
From: Trond Myklebust @ 2015-09-22 22:31 UTC (permalink / raw)
  To: Tigran Mkrtchyan, linux-nfs

On Tue, 2015-09-22 at 15:45 +0200, Tigran Mkrtchyan wrote:
> current NFSv4.1/pNFS client assumes that MDS supports
> only one layout type. While it's true for most existing servers,
> nevertheless, this can be change in the near future.
> 
> This patch is an attempt to multi layouttype MDS support.  To make
> it possible for such servers still function with existing clients,
> the layout
> type list will be processed in reverse order. This gives old clients
> still
> pick the first entry, but newer client will pick last entry.  Proof:
> 
> 4.3.0-rc1-dirty
> [  180.868764] nfs4flexfilelayout_init: NFSv4 Flexfile Layout Driver
> Registering...
> 
> 2.6.32-573.3.1.el6.x86_64
> nfs4filelayout_init: NFSv4 File Layout Driver Registering...
> 
> Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
> ---
>  fs/nfs/client.c         |  2 +-
>  fs/nfs/nfs4xdr.c        | 22 +++++++++---------
>  fs/nfs/pnfs.c           | 62 ++++++++++++++++++++++++++++++---------
> ----------
>  fs/nfs/pnfs.h           |  2 +-
>  include/linux/nfs_xdr.h |  4 +++-
>  5 files changed, 54 insertions(+), 38 deletions(-)
> 
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 57c5a02..1145bc7 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -786,7 +786,7 @@ int nfs_probe_fsinfo(struct nfs_server *server,
> struct nfs_fh *mntfh, struct nfs
>  	}
>  
>  	fsinfo.fattr = fattr;
> -	fsinfo.layouttype = 0;
> +	memset(fsinfo.layouttype, 0, sizeof(fsinfo.layouttype));
>  	error = clp->rpc_ops->fsinfo(server, mntfh, &fsinfo);
>  	if (error < 0)
>  		goto out_error;
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index 788adf3..7fbd411 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -4688,14 +4688,14 @@ static int decode_getfattr(struct xdr_stream
> *xdr, struct nfs_fattr *fattr,
>  }
>  
>  /*
> - * Decode potentially multiple layout types. Currently we only
> support
> - * one layout driver per file system.
> + * Decode potentially multiple layout types.
>   */
> -static int decode_first_pnfs_layout_type(struct xdr_stream *xdr,
> +static int decode_pnfs_layout_types(struct xdr_stream *xdr,
>  					 uint32_t *layouttype)
>  {
>  	__be32 *p;
>  	int num;
> +	int i;

Make these both unsigned int?

>  
>  	p = xdr_inline_decode(xdr, 4);
>  	if (unlikely(!p))
> @@ -4704,18 +4704,18 @@ static int
> decode_first_pnfs_layout_type(struct xdr_stream *xdr,
>  
>  	/* pNFS is not supported by the underlying file system */
>  	if (num == 0) {
> -		*layouttype = 0;
>  		return 0;
>  	}
> -	if (num > 1)
> -		printk(KERN_INFO "NFS: %s: Warning: Multiple pNFS
> layout "
> -			"drivers per filesystem not supported\n",
> __func__);
> +	if (num > NFS_MAX_LAYOUT_TYPES)
> +		printk(KERN_INFO "NFS: %s: Warning: Too many (%d)
> pNFS layout types\n", __func__, num);
>  
>  	/* Decode and set first layout type, move xdr->p past unused
> types */
>  	p = xdr_inline_decode(xdr, num * 4);
>  	if (unlikely(!p))
>  		goto out_overflow;
> -	*layouttype = be32_to_cpup(p);
> +	for(i = 0; i < num && i < NFS_MAX_LAYOUT_TYPES; i++) {
> +		layouttype[i] = be32_to_cpup(p++);
> +	}

You don't need the { } block enclosure here.

>  	return 0;
>  out_overflow:
>  	print_overflow_msg(__func__, xdr);
> @@ -4735,10 +4735,10 @@ static int decode_attr_pnfstype(struct
> xdr_stream *xdr, uint32_t *bitmap,
>  	if (unlikely(bitmap[1] & (FATTR4_WORD1_FS_LAYOUT_TYPES -
> 1U)))
>  		return -EIO;
>  	if (bitmap[1] & FATTR4_WORD1_FS_LAYOUT_TYPES) {
> -		status = decode_first_pnfs_layout_type(xdr,
> layouttype);
> +		status = decode_pnfs_layout_types(xdr, layouttype);
>  		bitmap[1] &= ~FATTR4_WORD1_FS_LAYOUT_TYPES;
>  	} else
> -		*layouttype = 0;
> +		memset(layouttype, 0, sizeof(*layouttype));

Isn't this already done by the caller in fs/nfs/client.c?

> 	return status;
>  }
>  
> @@ -4792,7 +4792,7 @@ static int decode_fsinfo(struct xdr_stream
> *xdr, struct nfs_fsinfo *fsinfo)
>  	status = decode_attr_time_delta(xdr, bitmap, &fsinfo
> ->time_delta);
>  	if (status != 0)
>  		goto xdr_error;
> -	status = decode_attr_pnfstype(xdr, bitmap, &fsinfo
> ->layouttype);
> +	status = decode_attr_pnfstype(xdr, bitmap, fsinfo
> ->layouttype);
>  	if (status != 0)
>  		goto xdr_error;
>  	status = decode_attr_layout_blksize(xdr, bitmap, &fsinfo
> ->blksize);
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index ba12464..883bfa2 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -104,45 +104,59 @@ unset_pnfs_layoutdriver(struct nfs_server
> *nfss)
>   * Try to set the server's pnfs module to the pnfs layout type
> specified by id.
>   * Currently only one pNFS layout driver per filesystem is
> supported.
>   *
> - * @id layout type. Zero (illegal layout type) indicates pNFS not in
> use.
> + * @ids array of layout types supported by MDS.
>   */
>  void
>  set_pnfs_layoutdriver(struct nfs_server *server, const struct nfs_fh
> *mntfh,
> -		      u32 id)
> +		      u32 *ids)
>  {
>  	struct pnfs_layoutdriver_type *ld_type = NULL;
> +	u32 id;
> +	int i;
>  
> -	if (id == 0)
> -		goto out_no_driver;
>  	if (!(server->nfs_client->cl_exchange_flags &
>  		 (EXCHGID4_FLAG_USE_NON_PNFS |
> EXCHGID4_FLAG_USE_PNFS_MDS))) {
> -		printk(KERN_ERR "NFS: %s: id %u cl_exchange_flags
> 0x%x\n",
> -			__func__, id, server->nfs_client
> ->cl_exchange_flags);
> +		printk(KERN_ERR "NFS: %s: cl_exchange_flags 0x%x\n",
> +			__func__, server->nfs_client
> ->cl_exchange_flags);
>  		goto out_no_driver;
>  	}
> -	ld_type = find_pnfs_driver(id);
> -	if (!ld_type) {
> -		request_module("%s-%u",
> LAYOUT_NFSV4_1_MODULE_PREFIX, id);
> +
> +	/*
> + 	 * walk in reverse ordet to use last in the list first.
> +   	 * By assyming, that server will send most common type
> first,
> +   	 * most preferred one last, old client will be happy with
> +   	 * a first entry, where new clients (4.4+) will pick the
> +   	 * most advance one.

This implies that old clients will always pick the least preferred
layout type from the server's perspective. Ditto if newer clients get
the value of NFS_MAX_LAYOUT_TYPES wrong.

Why not instead assume the following? 1st entry == default. Others are
sorted in order of decreasing server preference.

> + 	 */
> +	for(i = NFS_MAX_LAYOUT_TYPES; i >=0; i--) {
> +		id = ids[i];
> +		if(!id)
> +			continue;
> +
>  		ld_type = find_pnfs_driver(id);
>  		if (!ld_type) {
> -			dprintk("%s: No pNFS module found for
> %u.\n",
> -				__func__, id);
> +			request_module("%s-%u",
> LAYOUT_NFSV4_1_MODULE_PREFIX, id);
> +			ld_type = find_pnfs_driver(id);
> +			if (!ld_type) {
> +				dprintk("%s: No pNFS module found
> for %u.\n",
> +					__func__, id);
> +				continue;
> +			}
> +		}
> +		server->pnfs_curr_ld = ld_type;
> +		if (ld_type->set_layoutdriver
> +		    && ld_type->set_layoutdriver(server, mntfh)) {
> +			printk(KERN_ERR "NFS: %s: Error initializing
> pNFS layout "
> +				"driver %u.\n", __func__, id);
> +			module_put(ld_type->owner);
>  			goto out_no_driver;
>  		}
> -	}
> -	server->pnfs_curr_ld = ld_type;
> -	if (ld_type->set_layoutdriver
> -	    && ld_type->set_layoutdriver(server, mntfh)) {
> -		printk(KERN_ERR "NFS: %s: Error initializing pNFS
> layout "
> -			"driver %u.\n", __func__, id);
> -		module_put(ld_type->owner);
> -		goto out_no_driver;
> -	}
> -	/* Bump the MDS count */
> -	atomic_inc(&server->nfs_client->cl_mds_count);
> +		/* Bump the MDS count */
> +		atomic_inc(&server->nfs_client->cl_mds_count);
>  
> -	dprintk("%s: pNFS module for %u set\n", __func__, id);
> -	return;
> +		dprintk("%s: pNFS module for %u set\n", __func__,
> id);
> +		return;
> +	}
>  
>  out_no_driver:
>  	dprintk("%s: Using NFSv4 I/O\n", __func__);
> diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
> index 78c9351..e7609c3 100644
> --- a/fs/nfs/pnfs.h
> +++ b/fs/nfs/pnfs.h
> @@ -235,7 +235,7 @@ void pnfs_get_layout_hdr(struct pnfs_layout_hdr
> *lo);
>  void pnfs_put_lseg(struct pnfs_layout_segment *lseg);
>  void pnfs_put_lseg_locked(struct pnfs_layout_segment *lseg);
>  
> -void set_pnfs_layoutdriver(struct nfs_server *, const struct nfs_fh
> *, u32);
> +void set_pnfs_layoutdriver(struct nfs_server *, const struct nfs_fh
> *, u32 *);
>  void unset_pnfs_layoutdriver(struct nfs_server *);
>  void pnfs_generic_pg_init_read(struct nfs_pageio_descriptor *,
> struct nfs_page *);
>  int pnfs_generic_pg_readpages(struct nfs_pageio_descriptor *desc);
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 52faf7e..6c88745 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -124,6 +124,8 @@ struct nfs_fattr {
>  		| NFS_ATTR_FATTR_SPACE_USED \
>  		| NFS_ATTR_FATTR_V4_SECURITY_LABEL)
>  
> +#define NFS_MAX_LAYOUT_TYPES 32

There are only RFCs for 4 layout types today. Where does the number
'32' come from?

> +
>  /*
>   * Info on the file system
>   */
> @@ -139,7 +141,7 @@ struct nfs_fsinfo {
>  	__u64			maxfilesize;
>  	struct timespec		time_delta; /* server time
> granularity */
>  	__u32			lease_time; /* in seconds */
> -	__u32			layouttype; /* supported pnfs
> layout driver */
> +	__u32			layouttype[NFS_MAX_LAYOUT_TYPES
> ]; /* supported pnfs layout drivers */
>  	__u32			blksize; /* preferred pnfs io
> block size */
>  };
>  
-- 
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@primarydata.com




^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] pnfs  support servers with multiple layout types
@ 2015-09-22 13:45 Tigran Mkrtchyan
  2015-09-22 22:31 ` Trond Myklebust
  0 siblings, 1 reply; 9+ messages in thread
From: Tigran Mkrtchyan @ 2015-09-22 13:45 UTC (permalink / raw)
  To: linux-nfs; +Cc: Tigran Mkrtchyan

current NFSv4.1/pNFS client assumes that MDS supports
only one layout type. While it's true for most existing servers,
nevertheless, this can be change in the near future.

This patch is an attempt to multi layouttype MDS support.  To make
it possible for such servers still function with existing clients, the layout
type list will be processed in reverse order. This gives old clients still
pick the first entry, but newer client will pick last entry.  Proof:

4.3.0-rc1-dirty
[  180.868764] nfs4flexfilelayout_init: NFSv4 Flexfile Layout Driver Registering...

2.6.32-573.3.1.el6.x86_64
nfs4filelayout_init: NFSv4 File Layout Driver Registering...

Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
---
 fs/nfs/client.c         |  2 +-
 fs/nfs/nfs4xdr.c        | 22 +++++++++---------
 fs/nfs/pnfs.c           | 62 ++++++++++++++++++++++++++++++-------------------
 fs/nfs/pnfs.h           |  2 +-
 include/linux/nfs_xdr.h |  4 +++-
 5 files changed, 54 insertions(+), 38 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 57c5a02..1145bc7 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -786,7 +786,7 @@ int nfs_probe_fsinfo(struct nfs_server *server, struct nfs_fh *mntfh, struct nfs
 	}
 
 	fsinfo.fattr = fattr;
-	fsinfo.layouttype = 0;
+	memset(fsinfo.layouttype, 0, sizeof(fsinfo.layouttype));
 	error = clp->rpc_ops->fsinfo(server, mntfh, &fsinfo);
 	if (error < 0)
 		goto out_error;
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 788adf3..7fbd411 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -4688,14 +4688,14 @@ static int decode_getfattr(struct xdr_stream *xdr, struct nfs_fattr *fattr,
 }
 
 /*
- * Decode potentially multiple layout types. Currently we only support
- * one layout driver per file system.
+ * Decode potentially multiple layout types.
  */
-static int decode_first_pnfs_layout_type(struct xdr_stream *xdr,
+static int decode_pnfs_layout_types(struct xdr_stream *xdr,
 					 uint32_t *layouttype)
 {
 	__be32 *p;
 	int num;
+	int i;
 
 	p = xdr_inline_decode(xdr, 4);
 	if (unlikely(!p))
@@ -4704,18 +4704,18 @@ static int decode_first_pnfs_layout_type(struct xdr_stream *xdr,
 
 	/* pNFS is not supported by the underlying file system */
 	if (num == 0) {
-		*layouttype = 0;
 		return 0;
 	}
-	if (num > 1)
-		printk(KERN_INFO "NFS: %s: Warning: Multiple pNFS layout "
-			"drivers per filesystem not supported\n", __func__);
+	if (num > NFS_MAX_LAYOUT_TYPES)
+		printk(KERN_INFO "NFS: %s: Warning: Too many (%d) pNFS layout types\n", __func__, num);
 
 	/* Decode and set first layout type, move xdr->p past unused types */
 	p = xdr_inline_decode(xdr, num * 4);
 	if (unlikely(!p))
 		goto out_overflow;
-	*layouttype = be32_to_cpup(p);
+	for(i = 0; i < num && i < NFS_MAX_LAYOUT_TYPES; i++) {
+		layouttype[i] = be32_to_cpup(p++);
+	}
 	return 0;
 out_overflow:
 	print_overflow_msg(__func__, xdr);
@@ -4735,10 +4735,10 @@ static int decode_attr_pnfstype(struct xdr_stream *xdr, uint32_t *bitmap,
 	if (unlikely(bitmap[1] & (FATTR4_WORD1_FS_LAYOUT_TYPES - 1U)))
 		return -EIO;
 	if (bitmap[1] & FATTR4_WORD1_FS_LAYOUT_TYPES) {
-		status = decode_first_pnfs_layout_type(xdr, layouttype);
+		status = decode_pnfs_layout_types(xdr, layouttype);
 		bitmap[1] &= ~FATTR4_WORD1_FS_LAYOUT_TYPES;
 	} else
-		*layouttype = 0;
+		memset(layouttype, 0, sizeof(*layouttype));
 	return status;
 }
 
@@ -4792,7 +4792,7 @@ static int decode_fsinfo(struct xdr_stream *xdr, struct nfs_fsinfo *fsinfo)
 	status = decode_attr_time_delta(xdr, bitmap, &fsinfo->time_delta);
 	if (status != 0)
 		goto xdr_error;
-	status = decode_attr_pnfstype(xdr, bitmap, &fsinfo->layouttype);
+	status = decode_attr_pnfstype(xdr, bitmap, fsinfo->layouttype);
 	if (status != 0)
 		goto xdr_error;
 	status = decode_attr_layout_blksize(xdr, bitmap, &fsinfo->blksize);
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index ba12464..883bfa2 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -104,45 +104,59 @@ unset_pnfs_layoutdriver(struct nfs_server *nfss)
  * Try to set the server's pnfs module to the pnfs layout type specified by id.
  * Currently only one pNFS layout driver per filesystem is supported.
  *
- * @id layout type. Zero (illegal layout type) indicates pNFS not in use.
+ * @ids array of layout types supported by MDS.
  */
 void
 set_pnfs_layoutdriver(struct nfs_server *server, const struct nfs_fh *mntfh,
-		      u32 id)
+		      u32 *ids)
 {
 	struct pnfs_layoutdriver_type *ld_type = NULL;
+	u32 id;
+	int i;
 
-	if (id == 0)
-		goto out_no_driver;
 	if (!(server->nfs_client->cl_exchange_flags &
 		 (EXCHGID4_FLAG_USE_NON_PNFS | EXCHGID4_FLAG_USE_PNFS_MDS))) {
-		printk(KERN_ERR "NFS: %s: id %u cl_exchange_flags 0x%x\n",
-			__func__, id, server->nfs_client->cl_exchange_flags);
+		printk(KERN_ERR "NFS: %s: cl_exchange_flags 0x%x\n",
+			__func__, server->nfs_client->cl_exchange_flags);
 		goto out_no_driver;
 	}
-	ld_type = find_pnfs_driver(id);
-	if (!ld_type) {
-		request_module("%s-%u", LAYOUT_NFSV4_1_MODULE_PREFIX, id);
+
+	/*
+ 	 * walk in reverse ordet to use last in the list first.
+   	 * By assyming, that server will send most common type first,
+   	 * most preferred one last, old client will be happy with
+   	 * a first entry, where new clients (4.4+) will pick the
+   	 * most advance one.
+ 	 */
+	for(i = NFS_MAX_LAYOUT_TYPES; i >=0; i--) {
+		id = ids[i];
+		if(!id)
+			continue;
+
 		ld_type = find_pnfs_driver(id);
 		if (!ld_type) {
-			dprintk("%s: No pNFS module found for %u.\n",
-				__func__, id);
+			request_module("%s-%u", LAYOUT_NFSV4_1_MODULE_PREFIX, id);
+			ld_type = find_pnfs_driver(id);
+			if (!ld_type) {
+				dprintk("%s: No pNFS module found for %u.\n",
+					__func__, id);
+				continue;
+			}
+		}
+		server->pnfs_curr_ld = ld_type;
+		if (ld_type->set_layoutdriver
+		    && ld_type->set_layoutdriver(server, mntfh)) {
+			printk(KERN_ERR "NFS: %s: Error initializing pNFS layout "
+				"driver %u.\n", __func__, id);
+			module_put(ld_type->owner);
 			goto out_no_driver;
 		}
-	}
-	server->pnfs_curr_ld = ld_type;
-	if (ld_type->set_layoutdriver
-	    && ld_type->set_layoutdriver(server, mntfh)) {
-		printk(KERN_ERR "NFS: %s: Error initializing pNFS layout "
-			"driver %u.\n", __func__, id);
-		module_put(ld_type->owner);
-		goto out_no_driver;
-	}
-	/* Bump the MDS count */
-	atomic_inc(&server->nfs_client->cl_mds_count);
+		/* Bump the MDS count */
+		atomic_inc(&server->nfs_client->cl_mds_count);
 
-	dprintk("%s: pNFS module for %u set\n", __func__, id);
-	return;
+		dprintk("%s: pNFS module for %u set\n", __func__, id);
+		return;
+	}
 
 out_no_driver:
 	dprintk("%s: Using NFSv4 I/O\n", __func__);
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index 78c9351..e7609c3 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -235,7 +235,7 @@ void pnfs_get_layout_hdr(struct pnfs_layout_hdr *lo);
 void pnfs_put_lseg(struct pnfs_layout_segment *lseg);
 void pnfs_put_lseg_locked(struct pnfs_layout_segment *lseg);
 
-void set_pnfs_layoutdriver(struct nfs_server *, const struct nfs_fh *, u32);
+void set_pnfs_layoutdriver(struct nfs_server *, const struct nfs_fh *, u32 *);
 void unset_pnfs_layoutdriver(struct nfs_server *);
 void pnfs_generic_pg_init_read(struct nfs_pageio_descriptor *, struct nfs_page *);
 int pnfs_generic_pg_readpages(struct nfs_pageio_descriptor *desc);
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 52faf7e..6c88745 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -124,6 +124,8 @@ struct nfs_fattr {
 		| NFS_ATTR_FATTR_SPACE_USED \
 		| NFS_ATTR_FATTR_V4_SECURITY_LABEL)
 
+#define NFS_MAX_LAYOUT_TYPES 32
+
 /*
  * Info on the file system
  */
@@ -139,7 +141,7 @@ struct nfs_fsinfo {
 	__u64			maxfilesize;
 	struct timespec		time_delta; /* server time granularity */
 	__u32			lease_time; /* in seconds */
-	__u32			layouttype; /* supported pnfs layout driver */
+	__u32			layouttype[NFS_MAX_LAYOUT_TYPES]; /* supported pnfs layout drivers */
 	__u32			blksize; /* preferred pnfs io block size */
 };
 
-- 
2.4.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-03-03 13:50 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-02 12:08 [PATCH] pnfs support servers with multiple layout types Tigran Mkrtchyan
2016-03-02 12:47 ` kbuild test robot
2016-03-02 13:57 ` Tigran Mkrtchyan
2016-03-02 16:37   ` Christoph Hellwig
2016-03-02 19:22     ` Mkrtchyan, Tigran
2016-03-03 13:50     ` Mkrtchyan, Tigran
  -- strict thread matches above, loose matches on Subject: below --
2015-09-22 13:45 Tigran Mkrtchyan
2015-09-22 22:31 ` Trond Myklebust
2015-09-23  6:38   ` Mkrtchyan, Tigran

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.