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

[Jeff Layton <jlayton@poochiereds.net>]

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>