Re: [PATCH] blk-mq: remove blk_mq_hw_sysfs_cpus

[Greg KH <gregkh@linuxfoundation.org>]

On Fri, Aug 16, 2019 at 09:12:33AM -0600, Jens Axboe wrote:
> On 8/16/19 8:54 AM, Greg KH wrote:
> > On Fri, Aug 16, 2019 at 08:20:42AM -0600, Jens Axboe wrote:
> >> On 8/16/19 1:48 AM, Ming Lei wrote:
> >>> It is reported that sysfs buffer overflow can be triggered in case
> >>> of too many CPU cores(>841 on 4K PAGE_SIZE) when showing CPUs in
> >>> blk_mq_hw_sysfs_cpus_show().
> >>>
> >>> This info isn't useful, given users may retrieve the CPU list
> >>> from sw queue entries under same kobject dir, so far not see
> >>> any active users.
> >>>
> >>> So remove the entry as suggested by Greg.
> >>
> >> I think that's a bit frivolous, there could very well be scripts or
> >> apps that use it. Let's just fix the overflow.
> > 
> > As no one really knows what the format is (and the patch to fix the
> > overflow changes the format of the file), I would say that it needs to
> > just be dropped as it is not an example of what you should be doing in
> > sysfs.
> 
> It's a list of CPUs, I think the format is quite self explanatory?

I'm not disagreeing, but the patch to fix the length changes the
formatting to be different.  So if you were needing to parse that file,
it now just broke the parser.

Which is why sysfs is to be one-value-per-file :)

> But in any case, I'm not 100% opposed to removing it, it's just not
> one of those things that should be done on a whim.

If the format of the file is going to change, I would argue that the
filename should change as well, so that it's obvious what is happening
here.  This is how we got in trouble with /proc files so many times...

thanks,

greg k-h