* [PATCH] include: sysfs: Add macro to assign show for RO attributes @ 2021-01-27 4:19 Orson Zhai 2021-01-27 7:50 ` Greg Kroah-Hartman 0 siblings, 1 reply; 4+ messages in thread From: Orson Zhai @ 2021-01-27 4:19 UTC (permalink / raw) To: Jonathan Corbet, Mauro Carvalho Chehab, Joe Perches, Denis Efremov, Greg Kroah-Hartman, David S. Miller, Christian Brauner, Dan Williams, Sourabh Jain Cc: Orson Zhai, linux-doc, linux-kernel, Orson Zhai In some circumstances, multiple __ATTR_RO attributes need to be assigned with a single show function. Add this macro to make life easier with simple code. Signed-off-by: Orson Zhai <orsonzhai@gmail.com> --- Documentation/filesystems/sysfs.rst | 2 ++ include/linux/sysfs.h | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/Documentation/filesystems/sysfs.rst b/Documentation/filesystems/sysfs.rst index 004d490..0e2274a 100644 --- a/Documentation/filesystems/sysfs.rst +++ b/Documentation/filesystems/sysfs.rst @@ -141,6 +141,8 @@ __ATTR_RO_MODE(name, mode): fore more restrictive RO access currently only use case is the EFI System Resource Table (see drivers/firmware/efi/esrt.c) +__ATTR_RO_SHOW(name, show): + assumes default mode 0444 with specified show. __ATTR_RW(name): assumes default name_show, name_store and setting mode to 0644. diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h index 2caa34c..c851592 100644 --- a/include/linux/sysfs.h +++ b/include/linux/sysfs.h @@ -117,6 +117,11 @@ struct attribute_group { .show = _name##_show, \ } +#define __ATTR_RO_SHOW(_name, _show) { \ + .attr = { .name = __stringify(_name), .mode = 0444 }, \ + .show = _show, \ +} + #define __ATTR_RO_MODE(_name, _mode) { \ .attr = { .name = __stringify(_name), \ .mode = VERIFY_OCTAL_PERMISSIONS(_mode) }, \ -- 2.7.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] include: sysfs: Add macro to assign show for RO attributes 2021-01-27 4:19 [PATCH] include: sysfs: Add macro to assign show for RO attributes Orson Zhai @ 2021-01-27 7:50 ` Greg Kroah-Hartman 2021-01-27 12:51 ` Orson Zhai 0 siblings, 1 reply; 4+ messages in thread From: Greg Kroah-Hartman @ 2021-01-27 7:50 UTC (permalink / raw) To: Orson Zhai Cc: Jonathan Corbet, Mauro Carvalho Chehab, Joe Perches, Denis Efremov, David S. Miller, Christian Brauner, Dan Williams, Sourabh Jain, Orson Zhai, linux-doc, linux-kernel On Wed, Jan 27, 2021 at 12:19:22PM +0800, Orson Zhai wrote: > In some circumstances, multiple __ATTR_RO attributes need to be assigned > with a single show function. > > Add this macro to make life easier with simple code. > > Signed-off-by: Orson Zhai <orsonzhai@gmail.com> > --- > Documentation/filesystems/sysfs.rst | 2 ++ > include/linux/sysfs.h | 5 +++++ > 2 files changed, 7 insertions(+) > > diff --git a/Documentation/filesystems/sysfs.rst b/Documentation/filesystems/sysfs.rst > index 004d490..0e2274a 100644 > --- a/Documentation/filesystems/sysfs.rst > +++ b/Documentation/filesystems/sysfs.rst > @@ -141,6 +141,8 @@ __ATTR_RO_MODE(name, mode): > fore more restrictive RO access currently > only use case is the EFI System Resource Table > (see drivers/firmware/efi/esrt.c) > +__ATTR_RO_SHOW(name, show): > + assumes default mode 0444 with specified show. > __ATTR_RW(name): > assumes default name_show, name_store and setting > mode to 0644. > diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h > index 2caa34c..c851592 100644 > --- a/include/linux/sysfs.h > +++ b/include/linux/sysfs.h > @@ -117,6 +117,11 @@ struct attribute_group { > .show = _name##_show, \ > } > > +#define __ATTR_RO_SHOW(_name, _show) { \ > + .attr = { .name = __stringify(_name), .mode = 0444 }, \ > + .show = _show, \ > +} Do you have a real user for this? Using "raw" kobject attributes is rare and should not be used often, so who needs this? thanks, greg k-h ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] include: sysfs: Add macro to assign show for RO attributes 2021-01-27 7:50 ` Greg Kroah-Hartman @ 2021-01-27 12:51 ` Orson Zhai 2021-01-27 13:30 ` Greg Kroah-Hartman 0 siblings, 1 reply; 4+ messages in thread From: Orson Zhai @ 2021-01-27 12:51 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Jonathan Corbet, Mauro Carvalho Chehab, Joe Perches, Denis Efremov, David S. Miller, Christian Brauner, Dan Williams, Sourabh Jain, Orson Zhai, linux-doc, linux-kernel On Wed, Jan 27, 2021 at 08:50:28AM +0100, Greg Kroah-Hartman wrote: > On Wed, Jan 27, 2021 at 12:19:22PM +0800, Orson Zhai wrote: > > In some circumstances, multiple __ATTR_RO attributes need to be assigned > > with a single show function. > > > > Add this macro to make life easier with simple code. > > > > Signed-off-by: Orson Zhai <orsonzhai@gmail.com> > > --- > > Documentation/filesystems/sysfs.rst | 2 ++ > > include/linux/sysfs.h | 5 +++++ > > 2 files changed, 7 insertions(+) > > > > diff --git a/Documentation/filesystems/sysfs.rst b/Documentation/filesystems/sysfs.rst > > index 004d490..0e2274a 100644 > > --- a/Documentation/filesystems/sysfs.rst > > +++ b/Documentation/filesystems/sysfs.rst > > @@ -141,6 +141,8 @@ __ATTR_RO_MODE(name, mode): > > fore more restrictive RO access currently > > only use case is the EFI System Resource Table > > (see drivers/firmware/efi/esrt.c) > > +__ATTR_RO_SHOW(name, show): > > + assumes default mode 0444 with specified show. > > __ATTR_RW(name): > > assumes default name_show, name_store and setting > > mode to 0644. > > diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h > > index 2caa34c..c851592 100644 > > --- a/include/linux/sysfs.h > > +++ b/include/linux/sysfs.h > > @@ -117,6 +117,11 @@ struct attribute_group { > > .show = _name##_show, \ > > } > > > > +#define __ATTR_RO_SHOW(_name, _show) { \ > > + .attr = { .name = __stringify(_name), .mode = 0444 }, \ > > + .show = _show, \ > > +} > > Do you have a real user for this? Using "raw" kobject attributes is Yes, I have found at least one user in current kernel code. Please refer to [1]. The author implemented a similar marcro __ATRR_MRO as mine, plus an __ATRR_MWO with specified restore. If this patch merged, I'd to replace his marcro with mine. > rare and should not be used often, so who needs this? Agree. But for some device drivers it might be useful without side effect. Another example is from Android increment-fs code out there. That driver has 3 sysfs attributes which shared with same show function which only prints "support" to userland. Best Regards, Orson > > thanks, > > greg k-h [1] https://elixir.bootlin.com/linux/v5.11-rc5/source/drivers/media/pci/ddbridge/ddbridge-core.c#L2735 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] include: sysfs: Add macro to assign show for RO attributes 2021-01-27 12:51 ` Orson Zhai @ 2021-01-27 13:30 ` Greg Kroah-Hartman 0 siblings, 0 replies; 4+ messages in thread From: Greg Kroah-Hartman @ 2021-01-27 13:30 UTC (permalink / raw) To: Jonathan Corbet, Mauro Carvalho Chehab, Joe Perches, Denis Efremov, David S. Miller, Christian Brauner, Dan Williams, Sourabh Jain, Orson Zhai, linux-doc, linux-kernel On Wed, Jan 27, 2021 at 08:51:26PM +0800, Orson Zhai wrote: > On Wed, Jan 27, 2021 at 08:50:28AM +0100, Greg Kroah-Hartman wrote: > > On Wed, Jan 27, 2021 at 12:19:22PM +0800, Orson Zhai wrote: > > > In some circumstances, multiple __ATTR_RO attributes need to be assigned > > > with a single show function. > > > > > > Add this macro to make life easier with simple code. > > > > > > Signed-off-by: Orson Zhai <orsonzhai@gmail.com> > > > --- > > > Documentation/filesystems/sysfs.rst | 2 ++ > > > include/linux/sysfs.h | 5 +++++ > > > 2 files changed, 7 insertions(+) > > > > > > diff --git a/Documentation/filesystems/sysfs.rst b/Documentation/filesystems/sysfs.rst > > > index 004d490..0e2274a 100644 > > > --- a/Documentation/filesystems/sysfs.rst > > > +++ b/Documentation/filesystems/sysfs.rst > > > @@ -141,6 +141,8 @@ __ATTR_RO_MODE(name, mode): > > > fore more restrictive RO access currently > > > only use case is the EFI System Resource Table > > > (see drivers/firmware/efi/esrt.c) > > > +__ATTR_RO_SHOW(name, show): > > > + assumes default mode 0444 with specified show. > > > __ATTR_RW(name): > > > assumes default name_show, name_store and setting > > > mode to 0644. > > > diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h > > > index 2caa34c..c851592 100644 > > > --- a/include/linux/sysfs.h > > > +++ b/include/linux/sysfs.h > > > @@ -117,6 +117,11 @@ struct attribute_group { > > > .show = _name##_show, \ > > > } > > > > > > +#define __ATTR_RO_SHOW(_name, _show) { \ > > > + .attr = { .name = __stringify(_name), .mode = 0444 }, \ > > > + .show = _show, \ > > > +} > > > > Do you have a real user for this? Using "raw" kobject attributes is > > Yes, I have found at least one user in current kernel code. > > Please refer to [1]. > > The author implemented a similar marcro __ATRR_MRO as mine, plus an > __ATRR_MWO with specified restore. Ick, no, that should be using DEVICE_ATTR_RO() as it is a struct device attribute, not a "raw" kobject attribute. So that code should be fixed up anyway, no need for this macro :) > > If this patch merged, I'd to replace his marcro with mine. > > > rare and should not be used often, so who needs this? > > Agree. But for some device drivers it might be useful without side effect. Drivers should NOT be ever using __ATTR* macros. That is not what they are there for. > Another example is from Android increment-fs code out there. > That driver has 3 sysfs attributes which shared with same show function > which only prints "support" to userland. I can't take patches for out-of-tree code, sorry, you know this :) thanks, greg k-h ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-01-27 13:34 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-27 4:19 [PATCH] include: sysfs: Add macro to assign show for RO attributes Orson Zhai 2021-01-27 7:50 ` Greg Kroah-Hartman 2021-01-27 12:51 ` Orson Zhai 2021-01-27 13:30 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).