On 31 Aug 2023, at 11:15, Benjamin Coddington wrote:
> Expose a knob in sysfs to set the READDIR requested attributes for a
> non-plus READDIR request. This allows installations another option for
> tuning READDIR on v4. Further work is needed to detect whether enough
> attributes are being returned to also prime the dcache.
>
> Signed-off-by: Benjamin Coddington <bcodding@xxxxxxxxxx>
> ---
> fs/nfs/client.c | 2 ++
> fs/nfs/nfs4client.c | 3 ++
> fs/nfs/nfs4proc.c | 1 +
> fs/nfs/nfs4xdr.c | 7 ++---
> fs/nfs/sysfs.c | 58 +++++++++++++++++++++++++++++++++++++++
> include/linux/nfs_fs_sb.h | 1 +
> include/linux/nfs_xdr.h | 1 +
> 7 files changed, 69 insertions(+), 4 deletions(-)
>
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index e4c5f193ed5e..cf23a7c54bf1 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -920,6 +920,8 @@ void nfs_server_copy_userdata(struct nfs_server *target, struct nfs_server *sour
> target->options = source->options;
> target->auth_info = source->auth_info;
> target->port = source->port;
> + memcpy(target->readdir_attrs, source->readdir_attrs,
> + sizeof(target->readdir_attrs));
> }
> EXPORT_SYMBOL_GPL(nfs_server_copy_userdata);
>
> diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
> index d9114a754db7..ba1dffdd25eb 100644
> --- a/fs/nfs/nfs4client.c
> +++ b/fs/nfs/nfs4client.c
> @@ -1108,6 +1108,9 @@ static int nfs4_server_common_setup(struct nfs_server *server,
>
> nfs4_server_set_init_caps(server);
>
> + server->readdir_attrs[0] = FATTR4_WORD0_RDATTR_ERROR;
> + server->readdir_attrs[1] = FATTR4_WORD1_MOUNTED_ON_FILEID;
> +
> /* Probe the root fh to retrieve its FSID and filehandle */
> error = nfs4_get_rootfh(server, mntfh, auth_probe);
> if (error < 0)
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 832fa226b8f2..12cc9e972f36 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -5109,6 +5109,7 @@ static int _nfs4_proc_readdir(struct nfs_readdir_arg *nr_arg,
> .pgbase = 0,
> .count = nr_arg->page_len,
> .plus = nr_arg->plus,
> + .server = server,
> };
> struct nfs4_readdir_res res;
> struct rpc_message msg = {
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index deec76cf5afe..1825e3eeb34b 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -1601,16 +1601,15 @@ static void encode_read(struct xdr_stream *xdr, const struct nfs_pgio_args *args
>
> static void encode_readdir(struct xdr_stream *xdr, const struct nfs4_readdir_arg *readdir, struct rpc_rqst *req, struct compound_hdr *hdr)
> {
> - uint32_t attrs[3] = {
> - FATTR4_WORD0_RDATTR_ERROR,
> - FATTR4_WORD1_MOUNTED_ON_FILEID,
> - };
> + uint32_t attrs[3];
> uint32_t dircount = readdir->count;
> uint32_t maxcount = readdir->count;
> __be32 *p, verf[2];
> uint32_t attrlen = 0;
> unsigned int i;
>
> + memcpy(attrs, readdir->server->readdir_attrs, sizeof(attrs));
> +
> if (readdir->plus) {
> attrs[0] |= FATTR4_WORD0_TYPE|FATTR4_WORD0_CHANGE|FATTR4_WORD0_SIZE|
> FATTR4_WORD0_FSID|FATTR4_WORD0_FILEHANDLE|FATTR4_WORD0_FILEID;
> diff --git a/fs/nfs/sysfs.c b/fs/nfs/sysfs.c
> index bf378ecd5d9f..6bded395df18 100644
> --- a/fs/nfs/sysfs.c
> +++ b/fs/nfs/sysfs.c
> @@ -270,7 +270,59 @@ shutdown_store(struct kobject *kobj, struct kobj_attribute *attr,
> return count;
> }
>
> +static ssize_t
> +v4_readdir_attrs_show(struct kobject *kobj, struct kobj_attribute *attr,
> + char *buf)
> +{
> + struct nfs_server *server;
> + server = container_of(kobj, struct nfs_server, kobj);
> +
> + return sysfs_emit(buf, "0x%x 0x%x 0x%x\n",
> + server->readdir_attrs[0],
> + server->readdir_attrs[1],
> + server->readdir_attrs[2]);
> +}
> +
> +static ssize_t
> +v4_readdir_attrs_store(struct kobject *kobj, struct kobj_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct nfs_server *server;
> + u32 attrs[3];
> + char p[24], *v;
> + size_t token = 0;
> + int i;
> +
> + if (count > 24)
> + return -EINVAL;
> +
> + v = strncpy(p, buf, count);
> +
> + for (i = 0; i < 3; i++) {
> + token += strcspn(v, " ") + 1;
> + if (token > 22)
> + return -EINVAL;
> +
> + p[token - 1] = '\0';
> + if (kstrtoint(v, 0, &attrs[i]))
> + return -EINVAL;
> + v = &p[token];
> + }
> +
> + server = container_of(kobj, struct nfs_server, kobj);
> +
> + if (attrs[0])
> + server->readdir_attrs[0] = attrs[0];
> + if (attrs[1])
> + server->readdir_attrs[1] = attrs[1];
> + if (attrs[2])
> + server->readdir_attrs[2] = attrs[2];
Eh, this ^^ is obviously wrong - we want to allow setting to 0. I will fix
this if there's interest.
Ben
