Re: [PATCH v2 4/7] sysfs: Add SYSFS_HUGE_BIN_FILE flag for binary attributes larger than PAGE_SIZE

K Prateek Nayak
2026-04-28T07:20:38Z

(+ Danilo for the sysfs bits)

Hello folks,

Please treat this patch as an RFC.

On 4/27/2026 9:21 PM, Muralidhara M K wrote:
> Historically, sysfs read buffers were allocated with get_zeroed_page(),
> limiting reads to PAGE_SIZE. Commit 13c589d5b0ac ("sysfs: use seq_file
> when reading regular files") transitioned regular (text) attribute reads
> to seq_file, which can dynamically grow buffers beyond PAGE_SIZE.
> However, the PAGE_SIZE limit was intentionally preserved for
> compatibility. When binary attribute handling was later unified into
> the same codebase, the non-seq_file read path (kernfs_file_read_iter)
> retained this PAGE_SIZE cap for binary files as well.
>
> Drivers that expose binary attributes larger than PAGE_SIZE — such as
> the AMD HSMP metric table (~13 KB) — cannot deliver the full content
> in a single read() call through the existing path.

I have to add that neither of us are familiar with filesystems so our
assumptions might be a way off since we are out of our depths - we only
brought up what we could dig from the commit logs and by looking at the
implementation.

It looks like HSMP users want a sysfs way to read the *entire* metric
table in one go which has proved to be slightly harder to deliver with
the PAGE_SIZE limits in presence on concurrent readers.

Please forgive if this is not correct and if we have failed to uncover
a better approach for the same. We are all ears. (This should have been
ideally marked as an RFC; sorry for the oversight!)

>
> Introduce a new opt-in flag SYSFS_HUGE_BIN_FILE (040000) that drivers
> can OR into their bin_attribute mode. When set, sysfs selects a new
> kernfs_ops (sysfs_bin_kfops_huge_file_ro) whose .seq_show callback
> pipes the bin_attribute ->read() result through seq_file, allowing
> reads of arbitrary size in one shot. Existing binary attributes
> without the flag continue using the legacy capped path.
>
> Co-developed-by: Nayak K Prateek <kprateek.nayak@amd.com>
> Signed-off-by: Nayak K Prateek <kprateek.nayak@amd.com>
> Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
> ---
> Changes v1->v2: New patch
>
> fs/sysfs/file.c | 45 +++++++++++++++++++++++++++++++++++++++++++
> fs/sysfs/group.c | 8 ++++----
> include/linux/sysfs.h | 1 +
> 3 files changed, 50 insertions(+), 4 deletions(-)
>
> diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
> index 5709cede1d75..be42c3c1e056 100644
> --- a/fs/sysfs/file.c
> +++ b/fs/sysfs/file.c
> @@ -38,6 +38,45 @@ static const struct sysfs_ops *sysfs_file_ops(struct kernfs_node *kn)
> return kobj->ktype ? kobj->ktype->sysfs_ops : NULL;
> }
>
> +/*
> + * Reads on huge sysfs bin files are handled through seq_file, which
> + * takes care of hairy details like buffering and seeking. The
> + * following function pipes the bin_attribute ->read() result through
> + * seq_file so that reads larger than PAGE_SIZE work in one shot.
> + */
> +static int sysfs_kf_huge_file_seq_show(struct seq_file *sf, void *v)
> +{
> + struct kernfs_open_file *of = sf->private;
> + const struct bin_attribute *battr = of->kn->priv;
> + struct kobject *kobj = sysfs_file_kobj(of->kn);
> + loff_t size = file_inode(of->file)->i_size;
> + ssize_t count;
> + char *buf;
> +
> + if (!battr->read)
> + return -EIO;
> +
> + if (!size)
> + return -EIO;
> +
> + /* acquire buffer and ensure that it's >= size */
> + count = seq_get_buf(sf, &buf);
> + if (count < size) {
> + seq_commit(sf, -1);
> + return 0;
> + }
> +
> + memset(buf, 0, size);
> +
> + count = battr->read(of->file, kobj, battr, buf, 0, size);
> + if (count < 0)
> + return count;
> +
> + WARN_ON(count > size);
> + seq_commit(sf, min_t(ssize_t, count, size));
> + return 0;
> +}
> +
> /*
> * Reads on sysfs are handled through seq_file, which takes care of hairy
> * details like buffering and seeking. The following function pipes
> @@ -249,6 +288,10 @@ static const struct kernfs_ops sysfs_prealloc_kfops_rw = {
> .prealloc = true,
> };
>
> +static const struct kernfs_ops sysfs_bin_kfops_huge_file_ro = {
> + .seq_show = sysfs_kf_huge_file_seq_show,
> +};
> +
> static const struct kernfs_ops sysfs_bin_kfops_ro = {
> .read = sysfs_kf_bin_read,
> };
> @@ -333,6 +376,8 @@ int sysfs_add_bin_file_mode_ns(struct kernfs_node *parent,
> ops = &sysfs_bin_kfops_mmap;
> else if (battr->read && battr->write)
> ops = &sysfs_bin_kfops_rw;
> + else if (battr->read && (mode & SYSFS_HUGE_BIN_FILE))
> + ops = &sysfs_bin_kfops_huge_file_ro;
> else if (battr->read)
> ops = &sysfs_bin_kfops_ro;
> else if (battr->write)
> diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
> index b3edae0578c0..2d0b01c00a97 100644
> --- a/fs/sysfs/group.c
> +++ b/fs/sysfs/group.c
> @@ -74,11 +74,11 @@ static int create_files(struct kernfs_node *parent, struct kobject *kobj,
> continue;
> }
>
> - WARN(mode & ~(SYSFS_PREALLOC | 0664),
> + WARN(mode & ~(SYSFS_PREALLOC | SYSFS_HUGE_BIN_FILE | 0664),
> "Attribute %s: Invalid permissions 0%o\n",
> (*attr)->name, mode);
>
> - mode &= SYSFS_PREALLOC | 0664;
> + mode &= SYSFS_PREALLOC | SYSFS_HUGE_BIN_FILE | 0664;

nit.

Since we would like the flags to only apply to bin files, we probably
don't need this hunk that allows for adding SYSFS_PREALLOC to plain
files.

> error = sysfs_add_file_mode_ns(parent, *attr, mode, uid,
> gid, NULL);
> if (unlikely(error))
> @@ -107,11 +107,11 @@ static int create_files(struct kernfs_node *parent, struct kobject *kobj,
> if (grp->bin_size)
> size = grp->bin_size(kobj, *bin_attr, i);
>
> - WARN(mode & ~(SYSFS_PREALLOC | 0664),
> + WARN(mode & ~(SYSFS_PREALLOC | SYSFS_HUGE_BIN_FILE | 0664),
> "Attribute %s: Invalid permissions 0%o\n",
> (*bin_attr)->attr.name, mode);
>
> - mode &= SYSFS_PREALLOC | 0664;
> + mode &= SYSFS_PREALLOC | SYSFS_HUGE_BIN_FILE | 0664;
> error = sysfs_add_bin_file_mode_ns(parent, *bin_attr,
> mode, size, uid, gid,
> NULL);
> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index b1a3a1e6ad09..78f6c6252cf9 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -124,6 +124,7 @@ struct attribute_group {
>
> #define SYSFS_PREALLOC 010000
> #define SYSFS_GROUP_INVISIBLE 020000
> +#define SYSFS_HUGE_BIN_FILE 040000
>
> /*
> * DEFINE_SYSFS_GROUP_VISIBLE(name):

--
Thanks and Regards,
Prateek