Re: [PATCH v2 4/7] sysfs: Add SYSFS_HUGE_BIN_FILE flag for binary attributes larger than PAGE_SIZE
From: K Prateek Nayak
Date: Wed May 13 2026 - 02:37:42 EST
Hello Greg,
On 5/13/2026 11:54 AM, Greg KH wrote:
>>> That's fine, userspace must be able to handle a "short" read, and will
>>> just continue on and read everything afterward, right? You can't rely
>>> on userspace always asking for more data.
>>
>> I think this is complicated by the HSMP driver bits that requires the
>> read to issue a HSMP command to the hardware first to updates the
>> table before copying from the MMIO region.
>
> Then you have bigger problems here :(
>
>> If a concurrent reader arrives, they'll refresh the table for their
>> PAGE_SIZE chunk read and the prior user will see a torn value. For
>> most part it shouldn't be a problem but folks try to co-relate the
>> Temperature and Power data from the first chunk with the Throttle
>> Indicators in the second chunk and sometimes, they don't match the
>> expectations.
>
> Again, this is a problem, perhaps do not use sysfs for this? You can't
> control userspace, and to expect it to always work properly is not going
> to end well. This change isn't going to fix your problems listed above
> at all.
At some point users had asked for a sysfs interface to read the metric
table but I think we've outgrown it now based on this use-case.
>
>> The table should never have grown this big but some folks decided it
>> was a good idea and we can't fix it for a while and have hit the
>> PAGE_SIZE limit now.
>
> Just delete it and use a different interface to the kernel instead
> please. If you need atomic read/writes, use an ioctl. Don't try to fix
> sysfs into something that it was not designed for at all.
>
>> If there is a better alternate, we are all ears, and more than happy
>> to try out an alternative suggestion for the described problem.
>
> A misc device sounds like the properly solution.
Thank you for that suggestion! That should work for HSMP. We'll check
with the HSMP users and (try) convince them to move away from the
sysfs based interface from the next platform.
>
>>>> 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.
>>>
>>> If this is such a big issue, why not just do it always for binary files?
>>> What is the benefit of keeping two different code paths just for this
>>> "new" flag?
>>
>> We can do that! For bin attributes that specify .size or a size
>> function, we can use a flexible buffer and for the ones that don't, we
>> can enforce a PAGE_SIZE cap like today.
>>
>> Would that be okay?
>
> Overall, yes, but again, I don't think this is going to fix your
> problem.
Ack! Thank you again for the suggestion. Hopefully everyone if happy
with the misc device based interface and we don't need these changes
in sysfs.
Let me know if you would like the bin attr bits moved to seq_file
based reads similar to sysfs_file_kfops* and I can find some cycles
to do it irrespective of the HSMP bits :-)
--
Thanks and Regards,
Prateek