| Message ID | 1465295891-4952-4-git-send-email-jlayton@poochiereds.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 -- 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
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
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; }
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(-)