Messages in this thread | | | Date | Wed, 11 Oct 2023 10:42:58 -0400 | Subject | Re: [PATCH] workqueue: Override implicit ordered attribute in workqueue_apply_unbound_cpumask() | From | Waiman Long <> |
| |
On 10/11/23 08:16, Lai Jiangshan wrote: > On Wed, Oct 11, 2023 at 10:49 AM Waiman Long <longman@redhat.com> wrote: >> Commit 5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 >> to be ordered") enabled implicit ordered attribute to be added to >> WQ_UNBOUND workqueues with max_active of 1. This prevented the changing >> of attributes to these workqueues leading to fix commit 0a94efb5acbb >> ("workqueue: implicit ordered attribute should be overridable"). >> >> However, workqueue_apply_unbound_cpumask() was not updated at that time. >> So sysfs changes to wq_unbound_cpumask has no effect on WQ_UNBOUND >> workqueues with implicit ordered attribute. Since not all WQ_UNBOUND >> workqueues are visible on sysfs, we are not able to make all the >> necessary cpumask changes even if we iterates all the workqueue cpumasks >> in sysfs and changing them one by one. >> >> Fix this problem by applying the corresponding change made >> to apply_workqueue_attrs_locked() in the fix commit to >> workqueue_apply_unbound_cpumask(). >> >> Fixes: 5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered") >> Signed-off-by: Waiman Long <longman@redhat.com> > Hello Waiman Long > > Thanks for the fix. > >> --- >> kernel/workqueue.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/kernel/workqueue.c b/kernel/workqueue.c >> index d141bd8eb2b7..19d403aa41b0 100644 >> --- a/kernel/workqueue.c >> +++ b/kernel/workqueue.c >> @@ -5785,9 +5785,13 @@ static int workqueue_apply_unbound_cpumask(const cpumask_var_t unbound_cpumask) >> list_for_each_entry(wq, &workqueues, list) { >> if (!(wq->flags & WQ_UNBOUND)) >> continue; >> + >> /* creating multiple pwqs breaks ordering guarantee */ >> - if (wq->flags & __WQ_ORDERED) >> - continue; >> + if (!list_empty(&wq->pwqs)) { > I don't remember why the same test is needed in 0a94efb5acbb. > And I can't figure it out now. > > I think it needs some comments or to be removed.
Is it because there will be no active work if there is no pool workqueue? Anyway, I just make it to be the same as that in apply_workqueue_attrs_locked() as the function call sequence are similar. If we remove the list_empty() test, we will have to remove it in both.
Cheers, Longman
> > Thanks > Lai >
| |