Re: [PATCH] leds: remove PAGE_SIZE limit of /sys/class/leds/<led>/trigger

From: Greg KH
Date: Mon Sep 02 2019 - 14:12:12 EST


On Sun, Sep 01, 2019 at 06:53:34PM +0200, Jacek Anaszewski wrote:
> Hi Akinobu,
>
> Thank you for the patch.
>
> I have one nit below but in general it looks good to me.
> I've tested it with 2000 mtd triggers (~14kB file size)
> and it worked flawlessly.
>
> Still, I would like to have ack from Greg for it.
>
> Adding Greg on Cc.
>
> On 8/29/19 4:49 PM, Akinobu Mita wrote:
> > Reading /sys/class/leds/<led>/trigger returns all available LED triggers.
> > However, the size of this file is limited to PAGE_SIZE because of the
> > limitation for sysfs attribute.
> >
> > Enabling LED CPU trigger on systems with thousands of CPUs easily hits
> > PAGE_SIZE limit, and makes it impossible to see all available LED triggers
> > and which trigger is currently activated.
> >
> > This converts /sys/class/leds/<led>/trigger to bin attribute and removes
> > the PAGE_SIZE limitation.

But this is NOT a binary file. A sysfs binary file is used for when the
kernel passes data to or from hardware without any parsing of the data
by the kernel.

You are not doing that here, you are abusing the "one value per file"
rule of sysfs so much that you are forced to work around the limitation
it put in place on purpose to keep you from doing stuff like this.

Please fix this "correctly" by creating a new api that works properly
and just live with the fact that this file will never work correctly and
move everyone to use the new api instead.

Don't keep on abusing the interface by workarounds like this, it is not
ok.

thanks,

greg k-h