Hi Jeff,
On 07/10/2016 03:56 PM, Jeff Layton wrote:
> From: Jeff Layton <jlayton@xxxxxxxxxxxxxxx>
>
> Currently, the layout driver selection code always chooses the first one
> from the list. That's not really ideal however, as the server can send
> the list of layout types in any order that it likes. It's up to the
> client to select the best one for its needs.
>
> This patch adds an ordered list of preferred driver types and has the
> selection code sort the list of avaiable layout drivers according to it.
> Any unrecognized layout type is sorted to the end of the list.
>
> For now, the order of preference is hardcoded, but it should be possible
> to make this configurable in the future.
>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxxxxxxx>
> ---
> fs/nfs/client.c | 1 +
> fs/nfs/nfs4proc.c | 3 ++-
> fs/nfs/nfs4xdr.c | 29 +++++++++++++++-----------
> fs/nfs/pnfs.c | 54 +++++++++++++++++++++++++++++++++++++++++--------
> fs/nfs/pnfs.h | 5 +++--
> include/linux/nfs_xdr.h | 1 +
> 6 files changed, 70 insertions(+), 23 deletions(-)
>
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 067f489aab3f..2525d8f770d2 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -787,6 +787,7 @@ int nfs_probe_fsinfo(struct nfs_server *server, struct nfs_fh *mntfh, struct nfs
> }
>
> fsinfo.fattr = fattr;
> + fsinfo.nlayouttypes = 0;
> memset(fsinfo.layouttype, 0, sizeof(fsinfo.layouttype));
> error = clp->rpc_ops->fsinfo(server, mntfh, &fsinfo);
> if (error < 0)
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index de97567795a5..a1d6d4bd0d50 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -4252,7 +4252,8 @@ 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->nlayouttypes,
> + fsinfo->layouttype);
I think it might be a little cleaner to pass the fsinfo structure here instead of adding more variables to the function.
> }
>
> return error;
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index b2c698499ad9..4eee3a32e070 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -4723,28 +4723,32 @@ static int decode_getfattr(struct xdr_stream *xdr, struct nfs_fattr *fattr,
> * Decode potentially multiple layout types.
> */
> static int decode_pnfs_layout_types(struct xdr_stream *xdr,
> - uint32_t *layouttype)
> + uint32_t *nlayouttypes, uint32_t *layouttype)
Same thing here with decoding.
> {
> __be32 *p;
> - uint32_t num, i;
> + uint32_t i;
>
> p = xdr_inline_decode(xdr, 4);
> if (unlikely(!p))
> goto out_overflow;
> - num = be32_to_cpup(p);
> + *nlayouttypes = be32_to_cpup(p);
>
> /* pNFS is not supported by the underlying file system */
> - if (num == 0) {
> + if (*nlayouttypes == 0)
> return 0;
> - }
> - 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);
> + p = xdr_inline_decode(xdr, *nlayouttypes * 4);
> if (unlikely(!p))
> goto out_overflow;
> - for(i = 0; i < num && i < NFS_MAX_LAYOUT_TYPES; i++)
> +
> + /* If we get too many, then just cap it at the max */
> + if (*nlayouttypes > NFS_MAX_LAYOUT_TYPES) {
> + printk(KERN_INFO "NFS: %s: Warning: Too many (%u) pNFS layout types\n", __func__, *nlayouttypes);
> + *nlayouttypes = NFS_MAX_LAYOUT_TYPES;
> + }
> +
> + for(i = 0; i < *nlayouttypes; ++i)
> layouttype[i] = be32_to_cpup(p++);
> return 0;
> out_overflow:
> @@ -4757,7 +4761,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)
> + uint32_t *nlayouttypes, uint32_t *layouttype)
> {
> int status = 0;
>
> @@ -4765,7 +4769,7 @@ 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_pnfs_layout_types(xdr, layouttype);
> + status = decode_pnfs_layout_types(xdr, nlayouttypes, layouttype);
> bitmap[1] &= ~FATTR4_WORD1_FS_LAYOUT_TYPES;
> }
> return status;
> @@ -4848,7 +4852,8 @@ 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->nlayouttypes,
> + fsinfo->layouttype);
> if (status != 0)
> goto xdr_error;
>
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index 2fc1b9354345..11e56225e6d2 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -30,6 +30,7 @@
> #include <linux/nfs_fs.h>
> #include <linux/nfs_page.h>
> #include <linux/module.h>
> +#include <linux/sort.h>
> #include "internal.h"
> #include "pnfs.h"
> #include "iostat.h"
> @@ -99,6 +100,38 @@ 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 mount option?
> + */
> +static const u32 ld_prefs[] = {
> + LAYOUT_SCSI,
> + LAYOUT_BLOCK_VOLUME,
> + LAYOUT_OSD2_OBJECTS,
> + LAYOUT_FLEX_FILES,
> + LAYOUT_NFSV4_1_FILES,
> + 0
> +};
> +
> +static int
> +ld_cmp(const void *e1, const void *e2)
> +{
> + u32 ld1 = *((u32 *)e1);
> + u32 ld2 = *((u32 *)e2);
Looks like this conversion is used pretty frequently. I wish there was a "POINTER_TO_UINT" macro to make it clearer what's going on.
Thanks,
Anna
> + int i;
> +
> + for (i = 0; ld_prefs[i] != 0; i++) {
> + if (ld1 == ld_prefs[i])
> + return -1;
> +
> + if (ld2 == ld_prefs[i])
> + return 1;
> + }
> + return 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.
> *
> @@ -106,10 +139,11 @@ unset_pnfs_layoutdriver(struct nfs_server *nfss)
> */
> void
> set_pnfs_layoutdriver(struct nfs_server *server, const struct nfs_fh *mntfh,
> - u32 *ids)
> + u32 nids, u32 *ids)
> {
> struct pnfs_layoutdriver_type *ld_type = NULL;
> u32 id;
> + int i;
>
> if (!(server->nfs_client->cl_exchange_flags &
> (EXCHGID4_FLAG_USE_NON_PNFS | EXCHGID4_FLAG_USE_PNFS_MDS))) {
> @@ -118,18 +152,22 @@ set_pnfs_layoutdriver(struct nfs_server *server, const struct nfs_fh *mntfh,
> goto out_no_driver;
> }
>
> - id = ids[0];
> - if (!id)
> - goto out_no_driver;
> + sort(ids, nids, sizeof(*ids), ld_cmp, NULL);
>
> - ld_type = find_pnfs_driver(id);
> - if (!ld_type) {
> - request_module("%s-%u", LAYOUT_NFSV4_1_MODULE_PREFIX, id);
> + for (i = 0; i < nids; i++) {
> + id = ids[i];
> 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)
> + break;
> }
>
> 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;
> }
>
> diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
> index 500473c87e82..a879e747f708 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, 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);
> @@ -656,7 +656,8 @@ 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 *ids)
> + const struct nfs_fh *mntfh,
> + u32 nids, u32 *ids)
> {
> }
>
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 4eef590200c3..a7e6739ac936 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -144,6 +144,7 @@ struct nfs_fsinfo {
> __u64 maxfilesize;
> struct timespec time_delta; /* server time granularity */
> __u32 lease_time; /* in seconds */
> + __u32 nlayouttypes; /* number of layouttypes */
> __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 */
>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
