RE: [PATCH] sysfs: Remove page boundary align limitation on sysfs_emit and sysfs_emit_at

[Yu, Lang]

[AMD Official Use Only]



>-----Original Message-----
>From: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>Sent: Thursday, September 9, 2021 1:35 PM
>To: Yu, Lang <Lang.Yu@xxxxxxx>
>Cc: Joe Perches <joe@xxxxxxxxxxx>; Rafael J . Wysocki <rafael@xxxxxxxxxx>;
>linux-kernel@xxxxxxxxxxxxxxx
>Subject: Re: [PATCH] sysfs: Remove page boundary align limitation on sysfs_emit
>and sysfs_emit_at
>
>On Thu, Sep 09, 2021 at 05:27:49AM +0000, Yu, Lang wrote:
>>
>>
>>
>> >-----Original Message-----
>> >From: Joe Perches <joe@xxxxxxxxxxx>
>> >Sent: Thursday, September 9, 2021 1:06 PM
>> >To: Yu, Lang <Lang.Yu@xxxxxxx>; Greg Kroah-Hartman
>> ><gregkh@xxxxxxxxxxxxxxxxxxx>; Rafael J . Wysocki <rafael@xxxxxxxxxx>;
>> >linux- kernel@xxxxxxxxxxxxxxx
>> >Subject: Re: [PATCH] sysfs: Remove page boundary align limitation on
>> >sysfs_emit and sysfs_emit_at
>> >
>> >On Wed, 2021-09-08 at 20:07 +0800, Lang Yu wrote:
>> >> The key purpose of sysfs_emit and sysfs_emit_at is to ensure that
>> >> no overrun is done. Make them more equivalent with scnprintf.
>> >
>> >I can't think of a single reason to do this.
>> >sysfs_emit and sysfs_emit_at are specific to sysfs.
>> >
>> >Use of these functions outside of sysfs is not desired or supported.
>> >
>> >
>> Thanks for your reply. But I'm still curious why you put such a limitation.
>> As "Documentation/filesystems/sysfs.rst" described, we can just  use
>> scnprintf(buf, PAGE_SIZE, "%s\n", dev->name) in show functions without
>> such a limitation.
>>
>> But you said that " - show() should only use sysfs_emit() or
>> sysfs_emit_at() when formatting the value to be returned to user space. " in
>Documentation/filesystems/sysfs.rst.
>>
>> Some guys just try to replace scnprintf with sysfs_emit() or sysfs_emit_at() per
>above documents.
>
>That is just fine in sysfs show() functions.
>
>> But sprintf and sysfs_emit/sysfs_emit_at are not totally equivalent(e.g., page
>boundary align).
>
>Which is good, it checks for more error conditions.
>
>I fail to understand what problem you have had with this.  What sysfs file was
>converted and had issues?
>
>> In my opinion, we add a new api and try to replace an old api. Does we
>> need to make it more compatible with old api? Thanks.
>
>There is no "old api" here.  People used a wide variety of different things in sysfs
>file show() functions, now you can just use a single one.

Yes, replace these things in sysfs show functions to avoid sprintf defects makes life better. 
But assume users will put a page boundary align buffer address in its' first argument makes life so hard.

Regards,
Lang

>thanks,
>
>greg k-h