[PATCH 0/3] pnfs/nfsd: have client and server support multiple layout types

[PATCH 0/3] pnfs/nfsd: have client and server support multiple layout types

[Jeff Layton]

This is a follow-up to the RFC set that I sent a week ago.

The basic idea is to allow the client to handle lists of layout types,
and for the server to provide them when there are multiple layout types
available for a particular filesystem.

The main change since the RFC set is to change how the client-side
layout driver selection code works. I dropped the patch that I had
written for the client and picked up Tigran's instead, and added a
patch on top to change the selection order.

Only lightly tested by mounting a server that sends both flexfiles and
block layouts. The client successfully selected the block layout in most
cases, but if I blacklist blocklayoutdriver then it selects flexfiles
instead.

I'm sending these together, but I'd expect Bruce to pick up the nfsd
patches and Trond or Anna to pick up the client-side ones.

The nfsd patch is based on top of Tom's nfsd flexfile layout patches.
Probably we should squash patches 2 and 3 before merging, but I left
them apart for now so you can see the change on top of what Tigran
originally proposed.

Jeff Layton (2):
  nfsd: allow nfsd to advertise multiple layout types
  pnfs: add a new mechanism to select a layout driver according to an
    ordered list

Tigran Mkrtchyan (1):
  pnfs support servers with multiple layout types

 fs/nfs/client.c         |  2 +-
 fs/nfs/nfs4xdr.c        | 23 +++++++--------
 fs/nfs/pnfs.c           | 76 ++++++++++++++++++++++++++++++++++++++++---------
 fs/nfs/pnfs.h           |  2 +-
 fs/nfsd/export.c        |  4 +--
 fs/nfsd/export.h        |  2 +-
 fs/nfsd/nfs4layouts.c   |  6 ++--
 fs/nfsd/nfs4proc.c      |  4 +--
 fs/nfsd/nfs4xdr.c       | 30 +++++++++----------
 include/linux/nfs_xdr.h |  8 +++++-
 10 files changed, 103 insertions(+), 54 deletions(-)

-- 
2.5.5

[PATCH 1/3] nfsd: allow nfsd to advertise multiple layout types

[Jeff Layton]

If the underlying filesystem supports multiple layout types, then there
is little reason not to advertise that fact to clients and let them
choose what type to use.

Turn the ex_layout_type field into a bitfield. For each supported
layout type, we set a bit in that field. When the client requests a
layout, ensure that the bit for that layout type is set. When the
client requests attributes, send back a list of supported types.

Signed-off-by: Jeff Layton <jeff.layton@primarydata.com>
Reviewed-by: Weston Andros Adamson <dros@primarydata.com>
---
 fs/nfsd/export.c      |  4 ++--
 fs/nfsd/export.h      |  2 +-
 fs/nfsd/nfs4layouts.c |  6 +++---
 fs/nfsd/nfs4proc.c    |  4 ++--
 fs/nfsd/nfs4xdr.c     | 30 ++++++++++++++----------------
 5 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index b4d84b579f20..f97ba49d5e66 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -706,7 +706,7 @@ static void svc_export_init(struct cache_head *cnew, struct cache_head *citem)
 	new->ex_fslocs.locations = NULL;
 	new->ex_fslocs.locations_count = 0;
 	new->ex_fslocs.migrated = 0;
-	new->ex_layout_type = 0;
+	new->ex_layout_types = 0;
 	new->ex_uuid = NULL;
 	new->cd = item->cd;
 }
@@ -731,7 +731,7 @@ static void export_update(struct cache_head *cnew, struct cache_head *citem)
 	item->ex_fslocs.locations_count = 0;
 	new->ex_fslocs.migrated = item->ex_fslocs.migrated;
 	item->ex_fslocs.migrated = 0;
-	new->ex_layout_type = item->ex_layout_type;
+	new->ex_layout_types = item->ex_layout_types;
 	new->ex_nflavors = item->ex_nflavors;
 	for (i = 0; i < MAX_SECINFO_LIST; i++) {
 		new->ex_flavors[i] = item->ex_flavors[i];
diff --git a/fs/nfsd/export.h b/fs/nfsd/export.h
index 2e315072bf3f..730f15eeb7ed 100644
--- a/fs/nfsd/export.h
+++ b/fs/nfsd/export.h
@@ -57,7 +57,7 @@ struct svc_export {
 	struct nfsd4_fs_locations ex_fslocs;
 	uint32_t		ex_nflavors;
 	struct exp_flavor_info	ex_flavors[MAX_SECINFO_LIST];
-	enum pnfs_layouttype	ex_layout_type;
+	u32			ex_layout_types;
 	struct nfsd4_deviceid_map *ex_devid_map;
 	struct cache_detail	*cd;
 };
diff --git a/fs/nfsd/nfs4layouts.c b/fs/nfsd/nfs4layouts.c
index 6d98d16b3354..2be9602b0221 100644
--- a/fs/nfsd/nfs4layouts.c
+++ b/fs/nfsd/nfs4layouts.c
@@ -139,21 +139,21 @@ void nfsd4_setup_layout_type(struct svc_export *exp)
 	 * otherwise advertise the block layout.
 	 */
 #ifdef CONFIG_NFSD_FLEXFILELAYOUT
-	exp->ex_layout_type = LAYOUT_FLEX_FILES;
+	exp->ex_layout_types |= 1 << LAYOUT_FLEX_FILES;
 #endif
 #ifdef CONFIG_NFSD_BLOCKLAYOUT
 	/* overwrite flex file layout selection if needed */
 	if (sb->s_export_op->get_uuid &&
 	    sb->s_export_op->map_blocks &&
 	    sb->s_export_op->commit_blocks)
-		exp->ex_layout_type = LAYOUT_BLOCK_VOLUME;
+		exp->ex_layout_types |= 1 << LAYOUT_BLOCK_VOLUME;
 #endif
 #ifdef CONFIG_NFSD_SCSILAYOUT
 	/* overwrite block layout selection if needed */
 	if (sb->s_export_op->map_blocks &&
 	    sb->s_export_op->commit_blocks &&
 	    sb->s_bdev && sb->s_bdev->bd_disk->fops->pr_ops)
-		exp->ex_layout_type = LAYOUT_SCSI;
+		exp->ex_layout_types |= 1 << LAYOUT_SCSI;
 #endif
 }
 
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 2ee2dc170327..719c620753e2 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1219,12 +1219,12 @@ nfsd4_verify(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 static const struct nfsd4_layout_ops *
 nfsd4_layout_verify(struct svc_export *exp, unsigned int layout_type)
 {
-	if (!exp->ex_layout_type) {
+	if (!exp->ex_layout_types) {
 		dprintk("%s: export does not support pNFS\n", __func__);
 		return NULL;
 	}
 
-	if (exp->ex_layout_type != layout_type) {
+	if (!(exp->ex_layout_types & (1 << layout_type))) {
 		dprintk("%s: layout type %d not supported\n",
 			__func__, layout_type);
 		return NULL;
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 9df898ba648f..4d3ae75ad4c8 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2164,22 +2164,20 @@ nfsd4_encode_aclname(struct xdr_stream *xdr, struct svc_rqst *rqstp,
 }
 
 static inline __be32
-nfsd4_encode_layout_type(struct xdr_stream *xdr, enum pnfs_layouttype layout_type)
+nfsd4_encode_layout_types(struct xdr_stream *xdr, u32 layout_types)
 {
-	__be32 *p;
+	__be32		*p;
+	unsigned long	i = hweight_long(layout_types);
 
-	if (layout_type) {
-		p = xdr_reserve_space(xdr, 8);
-		if (!p)
-			return nfserr_resource;
-		*p++ = cpu_to_be32(1);
-		*p++ = cpu_to_be32(layout_type);
-	} else {
-		p = xdr_reserve_space(xdr, 4);
-		if (!p)
-			return nfserr_resource;
-		*p++ = cpu_to_be32(0);
-	}
+	p = xdr_reserve_space(xdr, 4 + 4 * i);
+	if (!p)
+		return nfserr_resource;
+
+	*p++ = cpu_to_be32(i);
+
+	for (i = LAYOUT_NFSV4_1_FILES; i < LAYOUT_TYPE_MAX; ++i)
+		if (layout_types & (1 << i))
+			*p++ = cpu_to_be32(i);
 
 	return 0;
 }
@@ -2754,13 +2752,13 @@ out_acl:
 	}
 #ifdef CONFIG_NFSD_PNFS
 	if (bmval1 & FATTR4_WORD1_FS_LAYOUT_TYPES) {
-		status = nfsd4_encode_layout_type(xdr, exp->ex_layout_type);
+		status = nfsd4_encode_layout_types(xdr, exp->ex_layout_types);
 		if (status)
 			goto out;
 	}
 
 	if (bmval2 & FATTR4_WORD2_LAYOUT_TYPES) {
-		status = nfsd4_encode_layout_type(xdr, exp->ex_layout_type);
+		status = nfsd4_encode_layout_types(xdr, exp->ex_layout_types);
 		if (status)
 			goto out;
 	}
-- 
2.5.5

[PATCH 2/3] pnfs support servers with multiple layout types

[Jeff Layton]

From: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>

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 0c96528db94a..067f489aab3f 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 661e753fe1c9..b2c698499ad9 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -4720,14 +4720,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))
@@ -4736,18 +4735,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);
@@ -4767,10 +4765,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;
 }
 
@@ -4851,7 +4848,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 0c7e0d45a4de..b02cad9c04bf 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 b21bd0bee784..7d74e3da1d69 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -236,7 +236,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 c304a11b5b1a..15f7979494b2 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.5

Re: [PATCH 2/3] pnfs support servers with multiple layout types

[J. Bruce Fields]

On Tue, Jun 07, 2016 at 06:38:10AM -0400, Jeff Layton wrote:
> From: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
> 
> 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.

I don't follow this explanation.

Also, it gets overridden by the following patch.

How about making this patch purely a preperatory patch that puts the
necessary code in place without changing behavior, and leaving any
discussion of ordering to the following patch.

So, change set_pnfs_layoutdriver to take an array, but have it just do
the trivial thing (take the first element) till the following patch.

--b.

> 
> 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 0c96528db94a..067f489aab3f 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 661e753fe1c9..b2c698499ad9 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -4720,14 +4720,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))
> @@ -4736,18 +4735,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);
> @@ -4767,10 +4765,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;
>  }
>  
> @@ -4851,7 +4848,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 0c7e0d45a4de..b02cad9c04bf 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 b21bd0bee784..7d74e3da1d69 100644
> --- a/fs/nfs/pnfs.h
> +++ b/fs/nfs/pnfs.h
> @@ -236,7 +236,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 c304a11b5b1a..15f7979494b2 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.5

Re: [PATCH 2/3] pnfs support servers with multiple layout types

[Jeff Layton]

On Tue, 2016-06-07 at 17:33 -0400, J. Bruce Fields wrote:
> On Tue, Jun 07, 2016 at 06:38:10AM -0400, Jeff Layton wrote:
> > From: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
> > 
> > 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.
> 
> I don't follow this explanation.
> 
> Also, it gets overridden by the following patch.
> 
> How about making this patch purely a preperatory patch that puts the
> necessary code in place without changing behavior, and leaving any
> discussion of ordering to the following patch.
> 
> So, change set_pnfs_layoutdriver to take an array, but have it just do
> the trivial thing (take the first element) till the following patch.
> 
> --b.
> 

Sounds good. I'll do that for the next respin.

> > 
> > 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 0c96528db94a..067f489aab3f 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 661e753fe1c9..b2c698499ad9 100644
> > --- a/fs/nfs/nfs4xdr.c
> > +++ b/fs/nfs/nfs4xdr.c
> > @@ -4720,14 +4720,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))
> > @@ -4736,18 +4735,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);
> > @@ -4767,10 +4765,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;
> >  }
> >  
> > @@ -4851,7 +4848,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 0c7e0d45a4de..b02cad9c04bf 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 b21bd0bee784..7d74e3da1d69 100644
> > --- a/fs/nfs/pnfs.h
> > +++ b/fs/nfs/pnfs.h
> > @@ -236,7 +236,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 c304a11b5b1a..15f7979494b2 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.5
-- 
Jeff Layton <jlayton@poochiereds.net>

[PATCH 3/3] pnfs: add a new mechanism to select a layout driver according to an ordered list

[Jeff Layton]

Currently, the layout driver selection code attempts to divine meaning
from the order of the layout driver list sent by the server.
Unfortunately, the spec doesn't place any significance on the order
and the server is free to send them in any order it likes.

Instead, set a list of preferred driver types in the kernel and have
the selection code try them in order until it finds one that can be
loaded.

If we go through the whole list of preferred drivers and don't find one,
then try any that weren't recognized in the first scan. This should
allow the use of 3rd party and experimental drivers without needing to
muck with the order of preference.

For now, the order of preference is hardcoded, but it should be possible
to make this configurable (via module param perhaps?).

Signed-off-by: Jeff Layton <jeff.layton@primarydata.com>
---
 fs/nfs/pnfs.c | 71 +++++++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 50 insertions(+), 21 deletions(-)

diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index b02cad9c04bf..3ec5f2b392b6 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -99,6 +99,21 @@ unset_pnfs_layoutdriver(struct nfs_server *nfss)
 }
 
 /*
+ * When the server sends a list of layout types, we choose one in the order
+ * given in the list below.
+ *
+ * FIXME: should this list be configurable via module_param or something?
+ */
+static const u32 ld_prefs[] = {
+	LAYOUT_SCSI,
+	LAYOUT_BLOCK_VOLUME,
+	LAYOUT_OSD2_OBJECTS,
+	LAYOUT_FLEX_FILES,
+	LAYOUT_NFSV4_1_FILES,
+	0
+};
+
+/*
  * 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.
  *
@@ -110,7 +125,7 @@ set_pnfs_layoutdriver(struct nfs_server *server, const struct nfs_fh *mntfh,
 {
 	struct pnfs_layoutdriver_type *ld_type = NULL;
 	u32 id;
-	int i;
+	int i, j;
 
 	if (!(server->nfs_client->cl_exchange_flags &
 		 (EXCHGID4_FLAG_USE_NON_PNFS | EXCHGID4_FLAG_USE_PNFS_MDS))) {
@@ -118,31 +133,45 @@ set_pnfs_layoutdriver(struct nfs_server *server, const struct nfs_fh *mntfh,
 			__func__, server->nfs_client->cl_exchange_flags);
 		goto out_no_driver;
 	}
-	/*
-	 * 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)
-			goto found_module;
 
-		dprintk("%s: No pNFS module found for %u.\n", __func__, id);
+	/* scan the list for each layout type in order of preference */
+	for (j = 0; ld_prefs[j] != 0; j++) {
+		for (i = 0; i < NFS_MAX_LAYOUT_TYPES && ids[i] != 0; i++) {
+			id = ids[i];
+
+			if (ld_prefs[j] == id) {
+				request_module("%s-%u", LAYOUT_NFSV4_1_MODULE_PREFIX, id);
+				ld_type = find_pnfs_driver(id);
+				if (ld_type)
+					goto found_module;
+			}
+		}
 	}
 
-	/*
-	 * 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);
+	/* didn't find one -- make sure we try any that weren't in ld_prefs */
+	for (i = 0; i < NFS_MAX_LAYOUT_TYPES && ids[i] != 0; i++) {
+		bool	match = false;
+
+		id = ids[i];
+
+		/* Was it in the prefs list? */
+		for (j = 0; ld_prefs[j] != 0; j++) {
+			if (ld_prefs[j] != id) {
+				match = true;
+				break;
+			}
+		}
+
+		if (!match) {
+			request_module("%s-%u", LAYOUT_NFSV4_1_MODULE_PREFIX, id);
+			ld_type = find_pnfs_driver(id);
+			if (ld_type)
+				goto found_module;
+		}
+	}
 
 	if (!ld_type) {
-		dprintk("%s: No pNFS module found for %u.\n", __func__, id);
+		dprintk("%s: No pNFS module found!\n", __func__);
 		goto out_no_driver;
 	}
 
-- 
2.5.5

Re: [PATCH 3/3] pnfs: add a new mechanism to select a layout driver according to an ordered list

[J. Bruce Fields]

On Tue, Jun 07, 2016 at 06:38:11AM -0400, Jeff Layton wrote:
> Currently, the layout driver selection code attempts to divine meaning
> from the order of the layout driver list sent by the server.
> Unfortunately, the spec doesn't place any significance on the order
> and the server is free to send them in any order it likes.
> 
> Instead, set a list of preferred driver types in the kernel and have
> the selection code try them in order until it finds one that can be
> loaded.
> 
> If we go through the whole list of preferred drivers and don't find one,
> then try any that weren't recognized in the first scan. This should
> allow the use of 3rd party and experimental drivers without needing to
> muck with the order of preference.
> 
> For now, the order of preference is hardcoded, but it should be possible
> to make this configurable (via module param perhaps?).
> 
> Signed-off-by: Jeff Layton <jeff.layton@primarydata.com>
> ---
>  fs/nfs/pnfs.c | 71 +++++++++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 50 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index b02cad9c04bf..3ec5f2b392b6 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -99,6 +99,21 @@ unset_pnfs_layoutdriver(struct nfs_server *nfss)
>  }
>  
>  /*
> + * When the server sends a list of layout types, we choose one in the order
> + * given in the list below.
> + *
> + * FIXME: should this list be configurable via module_param or something?
> + */
> +static const u32 ld_prefs[] = {
> +	LAYOUT_SCSI,
> +	LAYOUT_BLOCK_VOLUME,
> +	LAYOUT_OSD2_OBJECTS,
> +	LAYOUT_FLEX_FILES,
> +	LAYOUT_NFSV4_1_FILES,
> +	0
> +};
> +
> +/*
>   * 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.
>   *
> @@ -110,7 +125,7 @@ set_pnfs_layoutdriver(struct nfs_server *server, const struct nfs_fh *mntfh,
>  {
>  	struct pnfs_layoutdriver_type *ld_type = NULL;
>  	u32 id;
> -	int i;
> +	int i, j;
>  
>  	if (!(server->nfs_client->cl_exchange_flags &
>  		 (EXCHGID4_FLAG_USE_NON_PNFS | EXCHGID4_FLAG_USE_PNFS_MDS))) {
> @@ -118,31 +133,45 @@ set_pnfs_layoutdriver(struct nfs_server *server, const struct nfs_fh *mntfh,
>  			__func__, server->nfs_client->cl_exchange_flags);
>  		goto out_no_driver;
>  	}
> -	/*
> -	 * 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)
> -			goto found_module;
>  
> -		dprintk("%s: No pNFS module found for %u.\n", __func__, id);
> +	/* scan the list for each layout type in order of preference */
> +	for (j = 0; ld_prefs[j] != 0; j++) {
> +		for (i = 0; i < NFS_MAX_LAYOUT_TYPES && ids[i] != 0; i++) {
> +			id = ids[i];
> +
> +			if (ld_prefs[j] == id) {
> +				request_module("%s-%u", LAYOUT_NFSV4_1_MODULE_PREFIX, id);
> +				ld_type = find_pnfs_driver(id);
> +				if (ld_type)
> +					goto found_module;
> +			}
> +		}
>  	}
>  
> -	/*
> -	 * 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);
> +	/* didn't find one -- make sure we try any that weren't in ld_prefs */
> +	for (i = 0; i < NFS_MAX_LAYOUT_TYPES && ids[i] != 0; i++) {
> +		bool	match = false;
> +
> +		id = ids[i];
> +
> +		/* Was it in the prefs list? */
> +		for (j = 0; ld_prefs[j] != 0; j++) {
> +			if (ld_prefs[j] != id) {
> +				match = true;
> +				break;
> +			}
> +		}

This logic feels more complicated than necessary.

We're going out of our way to support (at this point purely theoretical)
3rd-party layout modules.  When new layouts are develop, the chance
they'll be implementable with *no* changes to kernel code outside that
module seem small, and once you have to touch other code you may as well
update ld_prefs.

But anyway, if we want this, it might be easier to follow with logic
like:

	1. sort the ids[] array so the known layouts are at the top, in
	   ld_prefs order.
	2. try request_module() and find_pnfs_driver() in order on
	   the sorted ids[] array.

--b.

> +
> +		if (!match) {
> +			request_module("%s-%u", LAYOUT_NFSV4_1_MODULE_PREFIX, id);
> +			ld_type = find_pnfs_driver(id);
> +			if (ld_type)
> +				goto found_module;
> +		}
> +	}
>  
>  	if (!ld_type) {
> -		dprintk("%s: No pNFS module found for %u.\n", __func__, id);
> +		dprintk("%s: No pNFS module found!\n", __func__);
>  		goto out_no_driver;
>  	}
>  
> -- 
> 2.5.5

Re: [PATCH 3/3] pnfs: add a new mechanism to select a layout driver according to an ordered list

[Jeff Layton]

On Tue, 2016-06-07 at 17:46 -0400, J. Bruce Fields wrote:
> On Tue, Jun 07, 2016 at 06:38:11AM -0400, Jeff Layton wrote:
> > Currently, the layout driver selection code attempts to divine meaning
> > from the order of the layout driver list sent by the server.
> > Unfortunately, the spec doesn't place any significance on the order
> > and the server is free to send them in any order it likes.
> > 
> > Instead, set a list of preferred driver types in the kernel and have
> > the selection code try them in order until it finds one that can be
> > loaded.
> > 
> > If we go through the whole list of preferred drivers and don't find one,
> > then try any that weren't recognized in the first scan. This should
> > allow the use of 3rd party and experimental drivers without needing to
> > muck with the order of preference.
> > 
> > For now, the order of preference is hardcoded, but it should be possible
> > to make this configurable (via module param perhaps?).
> > 
> > Signed-off-by: Jeff Layton <jeff.layton@primarydata.com>
> > ---
> >  fs/nfs/pnfs.c | 71 +++++++++++++++++++++++++++++++++++++++++------------------
> >  1 file changed, 50 insertions(+), 21 deletions(-)
> > 
> > diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> > index b02cad9c04bf..3ec5f2b392b6 100644
> > --- a/fs/nfs/pnfs.c
> > +++ b/fs/nfs/pnfs.c
> > @@ -99,6 +99,21 @@ unset_pnfs_layoutdriver(struct nfs_server *nfss)
> >  }
> >  
> >  /*
> > + * When the server sends a list of layout types, we choose one in the order
> > + * given in the list below.
> > + *
> > + * FIXME: should this list be configurable via module_param or something?
> > + */
> > +static const u32 ld_prefs[] = {
> > +	LAYOUT_SCSI,
> > +	LAYOUT_BLOCK_VOLUME,
> > +	LAYOUT_OSD2_OBJECTS,
> > +	LAYOUT_FLEX_FILES,
> > +	LAYOUT_NFSV4_1_FILES,
> > +	0
> > +};
> > +
> > +/*
> >   * 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.
> >   *
> > @@ -110,7 +125,7 @@ set_pnfs_layoutdriver(struct nfs_server *server, const struct nfs_fh *mntfh,
> >  {
> >  	struct pnfs_layoutdriver_type *ld_type = NULL;
> >  	u32 id;
> > -	int i;
> > +	int i, j;
> >  
> >  	if (!(server->nfs_client->cl_exchange_flags &
> >  		 (EXCHGID4_FLAG_USE_NON_PNFS | EXCHGID4_FLAG_USE_PNFS_MDS))) {
> > @@ -118,31 +133,45 @@ set_pnfs_layoutdriver(struct nfs_server *server, const struct nfs_fh *mntfh,
> >  			__func__, server->nfs_client->cl_exchange_flags);
> >  		goto out_no_driver;
> >  	}
> > -	/*
> > -	 * 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)
> > -			goto found_module;
> >  
> > -		dprintk("%s: No pNFS module found for %u.\n", __func__, id);
> > +	/* scan the list for each layout type in order of preference */
> > +	for (j = 0; ld_prefs[j] != 0; j++) {
> > +		for (i = 0; i < NFS_MAX_LAYOUT_TYPES && ids[i] != 0; i++) {
> > +			id = ids[i];
> > +
> > +			if (ld_prefs[j] == id) {
> > +				request_module("%s-%u", LAYOUT_NFSV4_1_MODULE_PREFIX, id);
> > +				ld_type = find_pnfs_driver(id);
> > +				if (ld_type)
> > +					goto found_module;
> > +			}
> > +		}
> >  	}
> >  
> > -	/*
> > -	 * 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);
> > +	/* didn't find one -- make sure we try any that weren't in ld_prefs */
> > +	for (i = 0; i < NFS_MAX_LAYOUT_TYPES && ids[i] != 0; i++) {
> > +		bool	match = false;
> > +
> > +		id = ids[i];
> > +
> > +		/* Was it in the prefs list? */
> > +		for (j = 0; ld_prefs[j] != 0; j++) {
> > +			if (ld_prefs[j] != id) {
> > +				match = true;
> > +				break;
> > +			}
> > +		}
> 
> This logic feels more complicated than necessary.
> 
> We're going out of our way to support (at this point purely theoretical)
> 3rd-party layout modules.  When new layouts are develop, the chance
> they'll be implementable with *no* changes to kernel code outside that
> module seem small, and once you have to touch other code you may as well
> update ld_prefs.
> 
> But anyway, if we want this, it might be easier to follow with logic
> like:
> 
> 	1. sort the ids[] array so the known layouts are at the top, in
> 	   ld_prefs order.
> 	2. try request_module() and find_pnfs_driver() in order on
> 	   the sorted ids[] array.
> 
> --b.
> 

That's possible, but I'm not sure that will really make the code any
less complicated. I'll see what I can come up with.

> > +
> > +		if (!match) {
> > +			request_module("%s-%u", LAYOUT_NFSV4_1_MODULE_PREFIX, id);
> > +			ld_type = find_pnfs_driver(id);
> > +			if (ld_type)
> > +				goto found_module;
> > +		}
> > +	}
> >  
> >  	if (!ld_type) {
> > -		dprintk("%s: No pNFS module found for %u.\n", __func__, id);
> > +		dprintk("%s: No pNFS module found!\n", __func__);
> >  		goto out_no_driver;
> >  	}
> >  
> > -- 
> > 2.5.5
-- 
Jeff Layton <jlayton@poochiereds.net>