On 5/31/16, 17:09, "Jeff Layton" <jlayton@xxxxxxxxxxxxxxx> wrote:
>On Tue, 2016-05-31 at 16:03 +0000, Trond Myklebust wrote:
>>
>> On 5/30/16, 12:35, "Jeff Layton" <jlayton@xxxxxxxxxxxxxxx> wrote:
>>
>> > Allow the client to deal with servers that hand out multiple layout
>> > types for the same filesystem. When this happens, we pick the "best" one,
>> > based on a hardcoded assumed order in the client code.
>> >
>> > Signed-off-by: Jeff Layton <jeff.layton@xxxxxxxxxxxxxxx>
>> > ---
>> > fs/nfs/client.c | 2 +-
>> > fs/nfs/nfs4proc.c | 2 +-
>> > fs/nfs/nfs4xdr.c | 41 +++++++++++++-------------
>> > fs/nfs/pnfs.c | 76 ++++++++++++++++++++++++++++++++++++++-----------
>> > include/linux/nfs_xdr.h | 2 +-
>> > 5 files changed, 85 insertions(+), 38 deletions(-)
>> >
>> > diff --git a/fs/nfs/client.c b/fs/nfs/client.c
>> > index 0c96528db94a..53b41f4bd45a 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;
>> > + fsinfo.layouttypes = 0;
>> > error = clp->rpc_ops->fsinfo(server, mntfh, &fsinfo);
>> > if (error < 0)
>> > goto out_error;
>> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> > index de97567795a5..9446aef89b48 100644
>> > --- a/fs/nfs/nfs4proc.c
>> > +++ b/fs/nfs/nfs4proc.c
>> > @@ -4252,7 +4252,7 @@ static int nfs4_proc_fsinfo(struct nfs_server *server, struct nfs_fh *fhandle, s
>> > if (error == 0) {
>> > /* block layout checks this! */
>> > server->pnfs_blksize = fsinfo->blksize;
>> > - set_pnfs_layoutdriver(server, fhandle, fsinfo->layouttype);
>> > + set_pnfs_layoutdriver(server, fhandle, fsinfo->layouttypes);
>> > }
>> >
>> > return error;
>> > diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
>> > index 661e753fe1c9..876a80802c1d 100644
>> > --- a/fs/nfs/nfs4xdr.c
>> > +++ b/fs/nfs/nfs4xdr.c
>> > @@ -4723,33 +4723,36 @@ 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.
>> > */
>> > -static int decode_first_pnfs_layout_type(struct xdr_stream *xdr,
>> > - uint32_t *layouttype)
>> > +static int decode_pnfs_layout_types(struct xdr_stream *xdr, u32 *layouttypes)
>> > {
>> > __be32 *p;
>> > int num;
>> > + u32 type;
>> >
>> > p = xdr_inline_decode(xdr, 4);
>> > if (unlikely(!p))
>> > goto out_overflow;
>> > num = be32_to_cpup(p);
>> >
>> > - /* 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__);
>> > + *layouttypes = 0;
>> >
>> > - /* 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 (; num; --num) {
>> > + p = xdr_inline_decode(xdr, 4);
>> > +
>> > + if (unlikely(!p))
>> > + goto out_overflow;
>> > +
>> > + type = be32_to_cpup(p);
>> > +
>> > + /* Ignore any that we don't understand */
>> > + if (unlikely(type >= LAYOUT_TYPE_MAX))
>>
>> This will in effect hard code the layouts that the client supports.
>> LAYOUT_TYPE_MAX is something that applies to knfsd only for now.
>> Let’s not leak it into the client. I suggest just making this
>> 8*sizeof(*layouttypes).
>>
>
>Fair enough. I'll make that change.
>
>That said...LAYOUT_TYPE_MAX is a value in the pnfs_layouttype enum, and
>that enum is used in both the client and the server code, AFAICT. If we
>add a new LAYOUT_* value to that enum for the client, then we'll need
>to increase that value anyway. So, I'm not sure I understand how this
>limits the client in any way...
No, the client doesn’t use enum pnfs_layouttype anywhere. If you look at set_pnfs_layoutdriver(), you’ll note that we currently support all values for the layout type.
>
>
>> > + continue;
>> > +
>> > + *layouttypes |= 1 << type;
>> > + }
>> > return 0;
>> > out_overflow:
>> > + *layouttypes = 0;
>> > print_overflow_msg(__func__, xdr);
>> > return -EIO;
>> > }
>> > @@ -4759,7 +4762,7 @@ out_overflow:
>> > * Note we must ensure that layouttype is set in any non-error case.
>> > */
>> > static int decode_attr_pnfstype(struct xdr_stream *xdr, uint32_t *bitmap,
>> > - uint32_t *layouttype)
>> > + __u32 *layouttypes)
>> > {
>> > int status = 0;
>> >
>> > @@ -4767,10 +4770,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, layouttypes);
>> > bitmap[1] &= ~FATTR4_WORD1_FS_LAYOUT_TYPES;
>> > } else
>> > - *layouttype = 0;
>> > + *layouttypes = 0;
>> > return status;
>> > }
>> >
>> > @@ -4851,7 +4854,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->layouttypes);
>> > if (status != 0)
>> > goto xdr_error;
>> >
>> > diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
>> > index 0c7e0d45a4de..20b7b1f4e041 100644
>> > --- a/fs/nfs/pnfs.c
>> > +++ b/fs/nfs/pnfs.c
>> > @@ -70,7 +70,7 @@ out:
>> > }
>> >
>> > static struct pnfs_layoutdriver_type *
>> > -find_pnfs_driver(u32 id)
>> > +__find_pnfs_driver(u32 id)
>> > {
>> > struct pnfs_layoutdriver_type *local;
>> >
>> > @@ -84,6 +84,22 @@ find_pnfs_driver(u32 id)
>> > return local;
>> > }
>> >
>> > +static struct pnfs_layoutdriver_type *
>> > +find_pnfs_driver(u32 id)
>> > +{
>> > + struct pnfs_layoutdriver_type *ld_type;
>> > +
>> > + ld_type = __find_pnfs_driver(id);
>> > + if (!ld_type) {
>> > + 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);
>> > + }
>> > + return ld_type;
>> > +}
>> > +
>> > void
>> > unset_pnfs_layoutdriver(struct nfs_server *nfss)
>> > {
>> > @@ -102,44 +118,72 @@ 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.
>> > + * @layouttypes: bitfield showing what layout types server supports
>> > */
>> > void
>> > set_pnfs_layoutdriver(struct nfs_server *server, const struct nfs_fh *mntfh,
>> > - u32 id)
>> > + u32 layouttypes)
>> > {
>> > struct pnfs_layoutdriver_type *ld_type = NULL;
>> >
>> > - if (id == 0)
>> > + if (layouttypes == 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: layouttypes 0x%x cl_exchange_flags 0x%x\n",
>> > + __func__, layouttypes, 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);
>> > - ld_type = find_pnfs_driver(id);
>> > - if (!ld_type) {
>> > - dprintk("%s: No pNFS module found for %u.\n",
>> > - __func__, id);
>> > +
>> > + /*
>> > + * See if one of the layout types that we got handed is usable. We
>> > + * attempt in a hardcoded order of preference, in order of (assumed)
>> > + * decreasing speeds and functionality.
>> > + *
>> > + * FIXME: should this order be configurable in some fashion?
>> > + */
>> > + if (layouttypes & (1 << LAYOUT_SCSI)) {
>> > + ld_type = find_pnfs_driver(LAYOUT_SCSI);
>> > + if (ld_type)
>> > + goto set_driver;
>> > + }
>> > +
>> > + if (layouttypes & (1 << LAYOUT_BLOCK_VOLUME)) {
>> > + ld_type = find_pnfs_driver(LAYOUT_BLOCK_VOLUME);
>> > + if (ld_type)
>> > + goto set_driver;
>> > + }
>> > +
>> > + if (layouttypes & (1 << LAYOUT_OSD2_OBJECTS)) {
>> > + ld_type = find_pnfs_driver(LAYOUT_OSD2_OBJECTS);
>> > + if (ld_type)
>> > + goto set_driver;
>> > + }
>> > +
>> > + if (layouttypes & (1 << LAYOUT_FLEX_FILES)) {
>> > + ld_type = find_pnfs_driver(LAYOUT_FLEX_FILES);
>> > + if (ld_type)
>> > + goto set_driver;
>> > + }
>> > +
>> > + if (layouttypes & (1 << LAYOUT_NFSV4_1_FILES)) {
>> > + ld_type = find_pnfs_driver(LAYOUT_NFSV4_1_FILES);
>> > + if (!ld_type)
>> > goto out_no_driver;
>> > - }
>> > }
>> > +set_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);
>> > + "driver %u.\n", __func__, ld_type->id);
>> > module_put(ld_type->owner);
>> > goto out_no_driver;
>> > }
>> > /* Bump the MDS count */
>> > atomic_inc(&server->nfs_client->cl_mds_count);
>> >
>> > - dprintk("%s: pNFS module for %u set\n", __func__, id);
>> > + dprintk("%s: pNFS module for %u set\n", __func__, ld_type->id);
>> > return;
>> >
>> > out_no_driver:
>> > diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
>> > index c304a11b5b1a..1f6bb59f05f2 100644
>> > --- a/include/linux/nfs_xdr.h
>> > +++ b/include/linux/nfs_xdr.h
>> > @@ -139,7 +139,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 layouttypes; /* supported pnfs layout drivers */
>> > __u32 blksize; /* preferred pnfs io block size */
>> > __u32 clone_blksize; /* granularity of a CLONE operation */
>> > };
>> > --
>> > 2.5.5
>> >
>>
>> NrybXǧv^){.n+{"^nrz?h&?Gh?(階ݢj"??mzޖfh~m
>--
>Jeff Layton <jlayton@xxxxxxxxxxxxxxx>
>
��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
