2024-03-08 05:39:42

by Dan Williams

Subject: [PATCH] workqueue: Cleanup subsys attribute registration

While reviewing users of subsys_virtual_register() I noticed that
wq_sysfs_init() ignores the @groups argument. This looks like a
historical artifact as the original wq_subsys only had one attribute to
register.

On the way to building up an @groups argument to pass to
subsys_virtual_register() a few more cleanups fell out:

* Use DEVICE_ATTR_RO() and DEVICE_ATTR_RW() for
cpumask_{isolated,requested} and cpumask respectively. Rename the
@show and @store methods accordingly.

* Co-locate the attribute definition with the methods. This required
moving wq_unbound_cpumask_show down next to wq_unbound_cpumask_store
(renamed to cpumask_show() and cpumask_store())

* Use ATTRIBUTE_GROUPS() to skip some boilerplate declarations

Cc: Tejun Heo <[email protected]>
Cc: Lai Jiangshan <[email protected]>
Signed-off-by: Dan Williams <[email protected]>
---
Hi Tejun, just a drive-by cleanup while poking around sysfs.

kernel/workqueue.c | 49 +++++++++++++++++--------------------------------
1 file changed, 17 insertions(+), 32 deletions(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 7b482a26d741..7aa970b2f0d6 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -6208,25 +6208,27 @@ static ssize_t __wq_cpumask_show(struct device *dev,
return written;
}

-static ssize_t wq_unbound_cpumask_show(struct device *dev,
+static ssize_t cpumask_requested_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return __wq_cpumask_show(dev, attr, buf, wq_unbound_cpumask);
+ return __wq_cpumask_show(dev, attr, buf, wq_requested_unbound_cpumask);
}
+static DEVICE_ATTR_RO(cpumask_requested);

-static ssize_t wq_requested_cpumask_show(struct device *dev,
+static ssize_t cpumask_isolated_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return __wq_cpumask_show(dev, attr, buf, wq_requested_unbound_cpumask);
+ return __wq_cpumask_show(dev, attr, buf, wq_isolated_cpumask);
}
+static DEVICE_ATTR_RO(cpumask_isolated);

-static ssize_t wq_isolated_cpumask_show(struct device *dev,
+static ssize_t cpumask_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return __wq_cpumask_show(dev, attr, buf, wq_isolated_cpumask);
+ return __wq_cpumask_show(dev, attr, buf, wq_unbound_cpumask);
}

-static ssize_t wq_unbound_cpumask_store(struct device *dev,
+static ssize_t cpumask_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t count)
{
cpumask_var_t cpumask;
@@ -6242,36 +6244,19 @@ static ssize_t wq_unbound_cpumask_store(struct device *dev,
free_cpumask_var(cpumask);
return ret ? ret : count;
}
+static DEVICE_ATTR_RW(cpumask);

-static struct device_attribute wq_sysfs_cpumask_attrs[] = {
- __ATTR(cpumask, 0644, wq_unbound_cpumask_show,
- wq_unbound_cpumask_store),
- __ATTR(cpumask_requested, 0444, wq_requested_cpumask_show, NULL),
- __ATTR(cpumask_isolated, 0444, wq_isolated_cpumask_show, NULL),
- __ATTR_NULL,
+static struct attribute *wq_sysfs_cpumask_attrs[] = {
+ &dev_attr_cpumask.attr,
+ &dev_attr_cpumask_requested.attr,
+ &dev_attr_cpumask_isolated.attr,
+ NULL,
};
+ATTRIBUTE_GROUPS(wq_sysfs_cpumask);

static int __init wq_sysfs_init(void)
{
- struct device *dev_root;
- int err;
-
- err = subsys_virtual_register(&wq_subsys, NULL);
- if (err)
- return err;
-
- dev_root = bus_get_dev_root(&wq_subsys);
- if (dev_root) {
- struct device_attribute *attr;
-
- for (attr = wq_sysfs_cpumask_attrs; attr->attr.name; attr++) {
- err = device_create_file(dev_root, attr);
- if (err)
- break;
- }
- put_device(dev_root);
- }
- return err;
+ return subsys_virtual_register(&wq_subsys, wq_sysfs_cpumask_groups);
}
core_initcall(wq_sysfs_init);

2024-03-11 02:12:56

by Lai Jiangshan

Subject: Re: [PATCH] workqueue: Cleanup subsys attribute registration

On Fri, Mar 8, 2024 at 1:39 PM Dan Williams <[email protected]> wrote:

Thanks for the clean.

Reviewed-by: Lai Jiangshan <[email protected]>

Thanks
Lai

2024-03-25 19:39:55

by Tejun Heo

Subject: Re: [PATCH] workqueue: Cleanup subsys attribute registration

On Thu, Mar 07, 2024 at 09:39:32PM -0800, Dan Williams wrote:
> While reviewing users of subsys_virtual_register() I noticed that
> wq_sysfs_init() ignores the @groups argument. This looks like a
> historical artifact as the original wq_subsys only had one attribute to
> register.
>
> On the way to building up an @groups argument to pass to
> subsys_virtual_register() a few more cleanups fell out:
>
> * Use DEVICE_ATTR_RO() and DEVICE_ATTR_RW() for
> cpumask_{isolated,requested} and cpumask respectively. Rename the
> @show and @store methods accordingly.
>
> * Co-locate the attribute definition with the methods. This required
> moving wq_unbound_cpumask_show down next to wq_unbound_cpumask_store
> (renamed to cpumask_show() and cpumask_store())
>
> * Use ATTRIBUTE_GROUPS() to skip some boilerplate declarations
>
> Cc: Tejun Heo <[email protected]>
> Cc: Lai Jiangshan <[email protected]>
> Signed-off-by: Dan Williams <[email protected]>
> ---
> Hi Tejun, just a drive-by cleanup while poking around sysfs.

Applied to wq/for-6.10. Thank you so much.

--
tejun