2022-01-06 10:02:38

by Greg KH

[permalink] [raw]
Subject: [PATCH] dm sysfs: use default_groups in kobj_type

There are currently 2 ways to create a set of sysfs files for a
kobj_type, through the default_attrs field, and the default_groups
field. Move the dm sysfs code to use default_groups field which has
been the preferred way since aa30f47cf666 ("kobject: Add support for
default attribute groups to kobj_type") so that we can soon get rid of
the obsolete default_attrs field.

Cc: Alasdair Kergon <[email protected]>
Cc: Mike Snitzer <[email protected]>
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/md/dm-sysfs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm-sysfs.c b/drivers/md/dm-sysfs.c
index a05fcd50e1b9..e28c92478536 100644
--- a/drivers/md/dm-sysfs.c
+++ b/drivers/md/dm-sysfs.c
@@ -112,6 +112,7 @@ static struct attribute *dm_attrs[] = {
&dm_attr_rq_based_seq_io_merge_deadline.attr,
NULL,
};
+ATTRIBUTE_GROUPS(dm);

static const struct sysfs_ops dm_sysfs_ops = {
.show = dm_attr_show,
@@ -120,7 +121,7 @@ static const struct sysfs_ops dm_sysfs_ops = {

static struct kobj_type dm_ktype = {
.sysfs_ops = &dm_sysfs_ops,
- .default_attrs = dm_attrs,
+ .default_groups = dm_groups,
.release = dm_kobject_release,
};

--
2.34.1



2022-01-06 14:25:21

by Mike Snitzer

[permalink] [raw]
Subject: Re: dm sysfs: use default_groups in kobj_type

On Thu, Jan 06 2022 at 5:02P -0500,
Greg Kroah-Hartman <[email protected]> wrote:

> There are currently 2 ways to create a set of sysfs files for a
> kobj_type, through the default_attrs field, and the default_groups
> field. Move the dm sysfs code to use default_groups field which has
> been the preferred way since aa30f47cf666 ("kobject: Add support for
> default attribute groups to kobj_type") so that we can soon get rid of
> the obsolete default_attrs field.
>
> Cc: Alasdair Kergon <[email protected]>
> Cc: Mike Snitzer <[email protected]>
> Cc: [email protected]
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> ---
> drivers/md/dm-sysfs.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/dm-sysfs.c b/drivers/md/dm-sysfs.c
> index a05fcd50e1b9..e28c92478536 100644
> --- a/drivers/md/dm-sysfs.c
> +++ b/drivers/md/dm-sysfs.c
> @@ -112,6 +112,7 @@ static struct attribute *dm_attrs[] = {
> &dm_attr_rq_based_seq_io_merge_deadline.attr,
> NULL,
> };
> +ATTRIBUTE_GROUPS(dm);

Bit strange to pass "dm" but then have ATTRIBUTE_GROUPS assume dm_attrs defined.
Feels like it'll invite janitors sending patches, that they never
compile, to remove dm_attrs.

>
> static const struct sysfs_ops dm_sysfs_ops = {
> .show = dm_attr_show,
> @@ -120,7 +121,7 @@ static const struct sysfs_ops dm_sysfs_ops = {
>
> static struct kobj_type dm_ktype = {
> .sysfs_ops = &dm_sysfs_ops,
> - .default_attrs = dm_attrs,
> + .default_groups = dm_groups,
> .release = dm_kobject_release,
> };
>
> --
> 2.34.1
>

But I've picked this patch up for 5.17. Thanks.


2022-01-06 15:04:35

by Greg KH

[permalink] [raw]
Subject: Re: dm sysfs: use default_groups in kobj_type

On Thu, Jan 06, 2022 at 09:25:11AM -0500, Mike Snitzer wrote:
> On Thu, Jan 06 2022 at 5:02P -0500,
> Greg Kroah-Hartman <[email protected]> wrote:
>
> > There are currently 2 ways to create a set of sysfs files for a
> > kobj_type, through the default_attrs field, and the default_groups
> > field. Move the dm sysfs code to use default_groups field which has
> > been the preferred way since aa30f47cf666 ("kobject: Add support for
> > default attribute groups to kobj_type") so that we can soon get rid of
> > the obsolete default_attrs field.
> >
> > Cc: Alasdair Kergon <[email protected]>
> > Cc: Mike Snitzer <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> > ---
> > drivers/md/dm-sysfs.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/md/dm-sysfs.c b/drivers/md/dm-sysfs.c
> > index a05fcd50e1b9..e28c92478536 100644
> > --- a/drivers/md/dm-sysfs.c
> > +++ b/drivers/md/dm-sysfs.c
> > @@ -112,6 +112,7 @@ static struct attribute *dm_attrs[] = {
> > &dm_attr_rq_based_seq_io_merge_deadline.attr,
> > NULL,
> > };
> > +ATTRIBUTE_GROUPS(dm);
>
> Bit strange to pass "dm" but then have ATTRIBUTE_GROUPS assume dm_attrs defined.
> Feels like it'll invite janitors sending patches, that they never
> compile, to remove dm_attrs.

0-day would instantly catch that :)

That's the way the ATTRIBUTE_GROUPS() macro works. It's tricky, yes,
and I don't like it all that much, but couldn't come up with a better
way at the time. It saves lots and lots of boiler-plate code from
having to be typed all the time.

thanks,

greg k-h

2022-01-06 15:20:59

by Mike Snitzer

[permalink] [raw]
Subject: Re: dm sysfs: use default_groups in kobj_type

On Thu, Jan 06 2022 at 10:04P -0500,
Greg Kroah-Hartman <[email protected]> wrote:

> On Thu, Jan 06, 2022 at 09:25:11AM -0500, Mike Snitzer wrote:
> > On Thu, Jan 06 2022 at 5:02P -0500,
> > Greg Kroah-Hartman <[email protected]> wrote:
> >
> > > There are currently 2 ways to create a set of sysfs files for a
> > > kobj_type, through the default_attrs field, and the default_groups
> > > field. Move the dm sysfs code to use default_groups field which has
> > > been the preferred way since aa30f47cf666 ("kobject: Add support for
> > > default attribute groups to kobj_type") so that we can soon get rid of
> > > the obsolete default_attrs field.
> > >
> > > Cc: Alasdair Kergon <[email protected]>
> > > Cc: Mike Snitzer <[email protected]>
> > > Cc: [email protected]
> > > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> > > ---
> > > drivers/md/dm-sysfs.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/md/dm-sysfs.c b/drivers/md/dm-sysfs.c
> > > index a05fcd50e1b9..e28c92478536 100644
> > > --- a/drivers/md/dm-sysfs.c
> > > +++ b/drivers/md/dm-sysfs.c
> > > @@ -112,6 +112,7 @@ static struct attribute *dm_attrs[] = {
> > > &dm_attr_rq_based_seq_io_merge_deadline.attr,
> > > NULL,
> > > };
> > > +ATTRIBUTE_GROUPS(dm);
> >
> > Bit strange to pass "dm" but then have ATTRIBUTE_GROUPS assume dm_attrs defined.
> > Feels like it'll invite janitors sending patches, that they never
> > compile, to remove dm_attrs.
>
> 0-day would instantly catch that :)
>
> That's the way the ATTRIBUTE_GROUPS() macro works. It's tricky, yes,
> and I don't like it all that much, but couldn't come up with a better
> way at the time. It saves lots and lots of boiler-plate code from
> having to be typed all the time.

OK, thanks for the insight.

Mike