Wrap the cpumask attribute in an ATTRIBUTE_GROUPS() macro, so that
subsys_virtual_register() can add it to the dev_root device for us.
Signed-off-by: Julian Wiedmann <[email protected]>
---
kernel/workqueue.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 104e3ef04e33..817dc2d7438a 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -5625,15 +5625,15 @@ static struct device_attribute wq_sysfs_cpumask_attr =
__ATTR(cpumask, 0644, wq_unbound_cpumask_show,
wq_unbound_cpumask_store);
+static struct attribute *wq_sysfs_dev_root_attrs[] = {
+ &wq_sysfs_cpumask_attr.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(wq_sysfs_dev_root);
+
static int __init wq_sysfs_init(void)
{
- int err;
-
- err = subsys_virtual_register(&wq_subsys, NULL);
- if (err)
- return err;
-
- return device_create_file(wq_subsys.dev_root, &wq_sysfs_cpumask_attr);
+ return subsys_virtual_register(&wq_subsys, wq_sysfs_dev_root_groups);
}
core_initcall(wq_sysfs_init);
--
2.25.1
Wrap the attributes for a WQ_UNBOUND workqueue in ATTRIBUTE_GROUPS(),
and wire them up in dev->groups so that the device core can manage them
for us.
As device_add() will add such attributes _prior_ to raising the KOBJ_ADD
uevent, this also makes the initial uevent suppression unnecessary.
Signed-off-by: Julian Wiedmann <[email protected]>
---
kernel/workqueue.c | 51 ++++++++++++++++++++--------------------------
1 file changed, 22 insertions(+), 29 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 817dc2d7438a..629859ac5262 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -5449,6 +5449,9 @@ static ssize_t wq_pool_ids_show(struct device *dev,
return written;
}
+static struct device_attribute wq_sysfs_unbound_attr_pool_ids =
+ __ATTR(pool_ids, 0444, wq_pool_ids_show, NULL);
+
static ssize_t wq_nice_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
@@ -5502,6 +5505,9 @@ static ssize_t wq_nice_store(struct device *dev, struct device_attribute *attr,
return ret ?: count;
}
+static struct device_attribute wq_sysfs_unbound_attr_nice =
+ __ATTR(nice, 0644, wq_nice_show, wq_nice_store);
+
static ssize_t wq_cpumask_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -5539,6 +5545,9 @@ static ssize_t wq_cpumask_store(struct device *dev,
return ret ?: count;
}
+static struct device_attribute wq_sysfs_unbound_attr_cpumask =
+ __ATTR(cpumask, 0644, wq_cpumask_show, wq_cpumask_store);
+
static ssize_t wq_numa_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
@@ -5578,13 +5587,17 @@ static ssize_t wq_numa_store(struct device *dev, struct device_attribute *attr,
return ret ?: count;
}
-static struct device_attribute wq_sysfs_unbound_attrs[] = {
- __ATTR(pool_ids, 0444, wq_pool_ids_show, NULL),
- __ATTR(nice, 0644, wq_nice_show, wq_nice_store),
- __ATTR(cpumask, 0644, wq_cpumask_show, wq_cpumask_store),
- __ATTR(numa, 0644, wq_numa_show, wq_numa_store),
- __ATTR_NULL,
+static struct device_attribute wq_sysfs_unbound_attr_numa =
+ __ATTR(numa, 0644, wq_numa_show, wq_numa_store);
+
+static struct attribute *wq_sysfs_unbound_attrs[] = {
+ &wq_sysfs_unbound_attr_pool_ids.attr,
+ &wq_sysfs_unbound_attr_nice.attr,
+ &wq_sysfs_unbound_attr_cpumask.attr,
+ &wq_sysfs_unbound_attr_numa.attr,
+ NULL,
};
+ATTRIBUTE_GROUPS(wq_sysfs_unbound);
static struct bus_type wq_subsys = {
.name = "workqueue",
@@ -5679,37 +5692,17 @@ int workqueue_sysfs_register(struct workqueue_struct *wq)
wq_dev->wq = wq;
wq_dev->dev.bus = &wq_subsys;
wq_dev->dev.release = wq_device_release;
+ if (wq->flags & WQ_UNBOUND)
+ wq_dev->dev.groups = wq_sysfs_unbound_groups;
dev_set_name(&wq_dev->dev, "%s", wq->name);
- /*
- * unbound_attrs are created separately. Suppress uevent until
- * everything is ready.
- */
- dev_set_uevent_suppress(&wq_dev->dev, true);
-
ret = device_register(&wq_dev->dev);
if (ret) {
put_device(&wq_dev->dev);
wq->wq_dev = NULL;
- return ret;
- }
-
- if (wq->flags & WQ_UNBOUND) {
- struct device_attribute *attr;
-
- for (attr = wq_sysfs_unbound_attrs; attr->attr.name; attr++) {
- ret = device_create_file(&wq_dev->dev, attr);
- if (ret) {
- device_unregister(&wq_dev->dev);
- wq->wq_dev = NULL;
- return ret;
- }
- }
}
- dev_set_uevent_suppress(&wq_dev->dev, false);
- kobject_uevent(&wq_dev->dev.kobj, KOBJ_ADD);
- return 0;
+ return ret;
}
/**
--
2.25.1
On Mon, Jun 07, 2021 at 11:44:20AM +0200, Julian Wiedmann wrote:
> Wrap the attributes for a WQ_UNBOUND workqueue in ATTRIBUTE_GROUPS(),
> and wire them up in dev->groups so that the device core can manage them
> for us.
>
> As device_add() will add such attributes _prior_ to raising the KOBJ_ADD
> uevent, this also makes the initial uevent suppression unnecessary.
>
> Signed-off-by: Julian Wiedmann <[email protected]>
> ---
> kernel/workqueue.c | 51 ++++++++++++++++++++--------------------------
> 1 file changed, 22 insertions(+), 29 deletions(-)
>
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 817dc2d7438a..629859ac5262 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -5449,6 +5449,9 @@ static ssize_t wq_pool_ids_show(struct device *dev,
> return written;
> }
>
> +static struct device_attribute wq_sysfs_unbound_attr_pool_ids =
> + __ATTR(pool_ids, 0444, wq_pool_ids_show, NULL);
__ATTR_RO() please.
> +
> static ssize_t wq_nice_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> @@ -5502,6 +5505,9 @@ static ssize_t wq_nice_store(struct device *dev, struct device_attribute *attr,
> return ret ?: count;
> }
>
> +static struct device_attribute wq_sysfs_unbound_attr_nice =
> + __ATTR(nice, 0644, wq_nice_show, wq_nice_store);
__ATTR_RW()
> +
> static ssize_t wq_cpumask_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> @@ -5539,6 +5545,9 @@ static ssize_t wq_cpumask_store(struct device *dev,
> return ret ?: count;
> }
>
> +static struct device_attribute wq_sysfs_unbound_attr_cpumask =
> + __ATTR(cpumask, 0644, wq_cpumask_show, wq_cpumask_store);
__ATTR_RW()
thanks,
greg k-h
On 07.06.21 13:27, Greg Kroah-Hartman wrote:
> On Mon, Jun 07, 2021 at 11:44:20AM +0200, Julian Wiedmann wrote:
>> Wrap the attributes for a WQ_UNBOUND workqueue in ATTRIBUTE_GROUPS(),
>> and wire them up in dev->groups so that the device core can manage them
>> for us.
>>
>> As device_add() will add such attributes _prior_ to raising the KOBJ_ADD
>> uevent, this also makes the initial uevent suppression unnecessary.
>>
>> Signed-off-by: Julian Wiedmann <[email protected]>
>> ---
>> kernel/workqueue.c | 51 ++++++++++++++++++++--------------------------
>> 1 file changed, 22 insertions(+), 29 deletions(-)
>>
>> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
>> index 817dc2d7438a..629859ac5262 100644
>> --- a/kernel/workqueue.c
>> +++ b/kernel/workqueue.c
>> @@ -5449,6 +5449,9 @@ static ssize_t wq_pool_ids_show(struct device *dev,
>> return written;
>> }
>>
>> +static struct device_attribute wq_sysfs_unbound_attr_pool_ids =
>> + __ATTR(pool_ids, 0444, wq_pool_ids_show, NULL);
>
> __ATTR_RO() please.
>
I did consider using DEVICE_ATTR_*(), yes. But renaming all the _show and _store
functions added too much unrelated churn for my taste. So let's maybe do this
as a follow-on?
>> +
>> static ssize_t wq_nice_show(struct device *dev, struct device_attribute *attr,
>> char *buf)
>> {
>> @@ -5502,6 +5505,9 @@ static ssize_t wq_nice_store(struct device *dev, struct device_attribute *attr,
>> return ret ?: count;
>> }
>>
>> +static struct device_attribute wq_sysfs_unbound_attr_nice =
>> + __ATTR(nice, 0644, wq_nice_show, wq_nice_store);
>
> __ATTR_RW()
>
>> +
>> static ssize_t wq_cpumask_show(struct device *dev,
>> struct device_attribute *attr, char *buf)
>> {
>> @@ -5539,6 +5545,9 @@ static ssize_t wq_cpumask_store(struct device *dev,
>> return ret ?: count;
>> }
>>
>> +static struct device_attribute wq_sysfs_unbound_attr_cpumask =
>> + __ATTR(cpumask, 0644, wq_cpumask_show, wq_cpumask_store);
>
> __ATTR_RW()
>
> thanks,
>
> greg k-h
>
On Mon, Jun 07, 2021 at 01:34:49PM +0300, Julian Wiedmann wrote:
> On 07.06.21 13:27, Greg Kroah-Hartman wrote:
> > On Mon, Jun 07, 2021 at 11:44:20AM +0200, Julian Wiedmann wrote:
> >> Wrap the attributes for a WQ_UNBOUND workqueue in ATTRIBUTE_GROUPS(),
> >> and wire them up in dev->groups so that the device core can manage them
> >> for us.
> >>
> >> As device_add() will add such attributes _prior_ to raising the KOBJ_ADD
> >> uevent, this also makes the initial uevent suppression unnecessary.
> >>
> >> Signed-off-by: Julian Wiedmann <[email protected]>
> >> ---
> >> kernel/workqueue.c | 51 ++++++++++++++++++++--------------------------
> >> 1 file changed, 22 insertions(+), 29 deletions(-)
> >>
> >> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> >> index 817dc2d7438a..629859ac5262 100644
> >> --- a/kernel/workqueue.c
> >> +++ b/kernel/workqueue.c
> >> @@ -5449,6 +5449,9 @@ static ssize_t wq_pool_ids_show(struct device *dev,
> >> return written;
> >> }
> >>
> >> +static struct device_attribute wq_sysfs_unbound_attr_pool_ids =
> >> + __ATTR(pool_ids, 0444, wq_pool_ids_show, NULL);
> >
> > __ATTR_RO() please.
> >
>
> I did consider using DEVICE_ATTR_*(), yes. But renaming all the _show and _store
> functions added too much unrelated churn for my taste. So let's maybe do this
> as a follow-on?
Wait, oops, yes, do NOT use __ATTR() for a device attribute, use
DEVICE_ATTR_RO() please.
And yes, if you want to do it as a pater patch in this series that is
fine, just say you are moving the attribute definitions closer to the
callbacks in this patch.
thanks,
greg k-h