| Message ID | 4a471ab1bdea1052f45d894c967d0a6b6e38d4a6.1741806879.git.bcodding@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Mount option - force READDIRPLUS | expand |
On Wed, 2025-03-12 at 15:46 -0400, Benjamin Coddington wrote: > There are certain users that wish to force the NFS client to choose > READDIRPLUS over READDIR for a particular mount. Add a new kernel > mount > option "force_rdirplus" to carry that intent. Could we perhaps convert rdirplus to be a string with an optional payload? Does the "fs_param_can_be_empty" flag allow you to convert rdirplus into something that can behave as currently if you just specify '-ordirplus', but that would allow you to specify '- ordirplus=force' if you wanted to always use readdirplus?
On 12 Mar 2025, at 18:48, Trond Myklebust wrote: > On Wed, 2025-03-12 at 15:46 -0400, Benjamin Coddington wrote: >> There are certain users that wish to force the NFS client to choose >> READDIRPLUS over READDIR for a particular mount. Add a new kernel >> mount >> option "force_rdirplus" to carry that intent. > > Could we perhaps convert rdirplus to be a string with an optional > payload? Does the "fs_param_can_be_empty" flag allow you to convert > rdirplus into something that can behave as currently if you just > specify '-ordirplus', but that would allow you to specify '- > ordirplus=force' if you wanted to always use readdirplus? Yes, I think that's possible. I originally started down that route, but abandoned it after it appeared to be a bigger code diff because you have to re-define the nordirplus option which we get for free with fsparam_flag_no. I can send a v2 that way if it's preferred. Ben
On Wed, 2025-03-12 at 18:57 -0400, Benjamin Coddington wrote: > On 12 Mar 2025, at 18:48, Trond Myklebust wrote: > > > On Wed, 2025-03-12 at 15:46 -0400, Benjamin Coddington wrote: > > > There are certain users that wish to force the NFS client to > > > choose > > > READDIRPLUS over READDIR for a particular mount. Add a new > > > kernel > > > mount > > > option "force_rdirplus" to carry that intent. > > > > Could we perhaps convert rdirplus to be a string with an optional > > payload? Does the "fs_param_can_be_empty" flag allow you to convert > > rdirplus into something that can behave as currently if you just > > specify '-ordirplus', but that would allow you to specify '- > > ordirplus=force' if you wanted to always use readdirplus? > > Yes, I think that's possible. I originally started down that route, > but > abandoned it after it appeared to be a bigger code diff because you > have to > re-define the nordirplus option which we get for free with > fsparam_flag_no. > > I can send a v2 that way if it's preferred. To me that seems tidier, and easier to remember. It also allows you to alias nordirplus as a 'rdirplus=none' option, should that be desirable.
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index 2b04038b0e40..c9de0e474cf5 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -666,6 +666,8 @@ static bool nfs_use_readdirplus(struct inode *dir, struct dir_context *ctx, { if (!nfs_server_capable(dir, NFS_CAP_READDIRPLUS)) return false; + if (NFS_SERVER(dir)->flags && NFS_MOUNT_FORCE_RDIRPLUS) + return true; if (ctx->pos == 0 || cache_hits + cache_misses > NFS_READDIR_CACHE_USAGE_THRESHOLD) return true; diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c index b069385eea17..3ba44b444031 100644 --- a/fs/nfs/fs_context.c +++ b/fs/nfs/fs_context.c @@ -72,6 +72,7 @@ enum nfs_param { Opt_posix, Opt_proto, Opt_rdirplus, + Opt_force_rdirplus, Opt_rdma, Opt_resvport, Opt_retrans, @@ -175,6 +176,7 @@ static const struct fs_parameter_spec nfs_fs_parameters[] = { fsparam_flag_no("posix", Opt_posix), fsparam_string("proto", Opt_proto), fsparam_flag_no("rdirplus", Opt_rdirplus), + fsparam_flag ("force_rdirplus", Opt_force_rdirplus), fsparam_flag ("rdma", Opt_rdma), fsparam_flag_no("resvport", Opt_resvport), fsparam_u32 ("retrans", Opt_retrans), @@ -636,10 +638,18 @@ static int nfs_fs_context_parse_param(struct fs_context *fc, ctx->flags &= ~NFS_MOUNT_NOACL; break; case Opt_rdirplus: - if (result.negated) + if (result.negated) { + if (ctx->flags && NFS_MOUNT_FORCE_RDIRPLUS) + return nfs_invalf(fc, "NFS: Cannot both force and disable READDIR PLUS"); ctx->flags |= NFS_MOUNT_NORDIRPLUS; - else + } else { ctx->flags &= ~NFS_MOUNT_NORDIRPLUS; + } + break; + case Opt_force_rdirplus: + if (ctx->flags && NFS_MOUNT_NORDIRPLUS) + return nfs_invalf(fc, "NFS: Cannot both force and disable READDIR PLUS"); + ctx->flags |= NFS_MOUNT_FORCE_RDIRPLUS; break; case Opt_sharecache: if (result.negated) diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h index f00bfcee7120..3774b2235a1e 100644 --- a/include/linux/nfs_fs_sb.h +++ b/include/linux/nfs_fs_sb.h @@ -167,6 +167,7 @@ struct nfs_server { #define NFS_MOUNT_TRUNK_DISCOVERY 0x04000000 #define NFS_MOUNT_SHUTDOWN 0x08000000 #define NFS_MOUNT_NO_ALIGNWRITE 0x10000000 +#define NFS_MOUNT_FORCE_RDIRPLUS 0x20000000 unsigned int fattr_valid; /* Valid attributes */ unsigned int caps; /* server capabilities */
There are certain users that wish to force the NFS client to choose READDIRPLUS over READDIR for a particular mount. Add a new kernel mount option "force_rdirplus" to carry that intent. Signed-off-by: Benjamin Coddington <bcodding@redhat.com> --- fs/nfs/dir.c | 2 ++ fs/nfs/fs_context.c | 14 ++++++++++++-- include/linux/nfs_fs_sb.h | 1 + 3 files changed, 15 insertions(+), 2 deletions(-)