Messages in this thread | | | From | "Yu, Lang" <> | Subject | RE: [PATCH] sysfs: Remove page boundary align limitation on sysfs_emit and sysfs_emit_at | Date | Wed, 8 Sep 2021 15:33:51 +0000 |
| |
[AMD Official Use Only]
>-----Original Message----- >From: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >Sent: Wednesday, September 8, 2021 9:49 PM >To: Yu, Lang <Lang.Yu@amd.com> >Cc: Joe Perches <joe@perches.com>; Rafael J . Wysocki <rafael@kernel.org>; >linux-kernel@vger.kernel.org >Subject: Re: [PATCH] sysfs: Remove page boundary align limitation on sysfs_emit >and sysfs_emit_at > >On Wed, Sep 08, 2021 at 01:21:16PM +0000, Yu, Lang wrote: >> [AMD Official Use Only] >> >> >> >> >-----Original Message----- >> >From: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> >Sent: Wednesday, September 8, 2021 9:04 PM >> >To: Yu, Lang <Lang.Yu@amd.com> >> >Cc: Joe Perches <joe@perches.com>; Rafael J . Wysocki >> ><rafael@kernel.org>; linux-kernel@vger.kernel.org >> >Subject: Re: [PATCH] sysfs: Remove page boundary align limitation on >> >sysfs_emit and sysfs_emit_at >> > >> >A: >> >https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fen.w >> >ikipe%2F&data=04%7C01%7CLang.Yu%40amd.com%7C43d1354fdeda45 >713a340 >> >8d972cf6fcd%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63766 >7057520 >> >373013%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2l >uMzIiLC >> >JBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=tfUI5HXg6YbMRtFXs7 >X0o7Z >> >rRgKdwJfk%2FwIykAEkNCY%3D&reserved=0 >> >dia.org%2Fwiki%2FTop_post&data=04%7C01%7CLang.Yu%40amd.com% >7C >> >fed047de547541548fcc08d972c92627%7C3dd8961fe4884e608e11a82d994e1 >83d >> >%7C0%7C0%7C637667030534349355%7CUnknown%7CTWFpbGZsb3d8eyJWIj >oi >> >MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C100 >0& >> >amp;sdata=LHujj041jxZjvoYxVYUKtNr7us%2FX4pl%2FdOkFSOP1W8U%3D&am >p;r >> >eserved=0 >> >Q: Were do I find info about this thing called top-posting? >> >A: Because it messes up the order in which people normally read text. >> >Q: Why is top-posting such a bad thing? >> >A: Top-posting. >> >Q: What is the most annoying thing in e-mail? >> > >> >A: No. >> >Q: Should I include quotations after my reply? >> > >> >https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdari >> >ngfire >> >ball.net%2F2007%2F07%2Fon_top&data=04%7C01%7CLang.Yu%40amd. >co >> >m%7Cfed047de547541548fcc08d972c92627%7C3dd8961fe4884e608e11a82d >99 >> >4e183d%7C0%7C0%7C637667030534349355%7CUnknown%7CTWFpbGZsb3d >8ey >> >JWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D% >7C >> >1000&sdata=AOLGBdj01XiEjhmsBSGTNuqejgU%2B6jg416Paz5XdM1A%3D >&a >> >mp;reserved=0 >> > >> > >> >On Wed, Sep 08, 2021 at 12:52:43PM +0000, Yu, Lang wrote: >> >> [AMD Official Use Only] >> >> >> >> Thanks for your reply. >> >> Just curious if we don't put such a limitation, what are the consequences? >> >> If we remove the limitation, sys_emit/sys_emit_at api will be more flexible. >> >> Since the comments of sysfs_emit/ sys_emit_at api are " sysfs_emit >> >> - scnprintf equivalent, aware of PAGE_SIZE buffer. ", Why not make >> >> them more equivalent with scnprintf? >> > >> >Because this is not a general replacement for scnprintf(), it is only >> >to be used with sysfs files. >> > >> >Where else are you wanting to use these functions that this patch >> >woulud be required that does not haver to deal with sysfs? >> > >> >thanks, >> > >> >greg k-h >> >> But some guys think it is a general replacement for scnprintf(), > >Who thinks that? Where? The name should give them a clue that this is not true. > >> and recommend that use sysfs_emit() instead of scnprintf(), > >Please no. > >> and send many patches that replace scnprintf() with sysfs_emit(), and >> finally cause some invalid sysfs_emit_at: buf:00000000f19bdfde warnings. > >Where were those patches sent? I will be glad to review those. > >> I think we better not put " scnprintf equivalent, aware of PAGE_SIZE buffer " >words in comments. >> It is obviously not. Some guys are misled by that. Thanks! > >Please feel free to add better documentation for the functions if you feel people >are getting confused, do not change the existing behavior of the code as it rightly >caught it being misused.
You can find many patches named "convert sysfs scnprintf/snprintf to syfs_emit/sysfs_emit_at". or "use sysfs_emit/sysfs_emit_at in show functions". They may think it's better to use syfs_emit/sysfs_emit_at given its overrun avoidance. But there are still some corner cases(e.g., a non page boundary aligned buf address : ). Thanks for your explanations and have a nice day!
Regards, Lang
> >thanks, > >greg k-h
| |