Re: [PATCH] workqueue: Remove the unbound release work from the per-cpu type

[richard clark]

Hi Lai,

On Mon, Nov 28, 2022 at 11:24 AM Lai Jiangshan <jiangshanlai@xxxxxxxxx> wrote:
>
> On Fri, Nov 25, 2022 at 11:02 AM richard clark
> <richard.xnu.clark@xxxxxxxxx> wrote:
> >
> > Hi Tejun,
> >
> > Would you pls help take a look at this? Or point to someone who is
> > maintaining the workqueue subsystem, I can ping him/her...
> >
> > Richard
>
> Hello, Richard
>
> Thank you for reviewing the code of workqueue and trying to improve it.
>
> But INIT_WORK() has no unwanted effect and it is better to initialize
> the field even if it is unused.
>
IMO, what you said is a perfect 'logical paradox', you think it 'even
if it is unused', then the interesting conclusion is 'it is better to'
use it, would you pls help to justify the underlying logic here?
'unused' but 'better', hmmm...
>
> The patch would not be accepted.
That's definitely fine, forgive me for being a perfectionist to see
the codes... Please note that I've no intention to offend you, trust
me :)

Thanks,
Richard

>
> Thanks.
> Lai
>
> >
> > On Wed, Nov 23, 2022 at 5:51 PM Richard Clark
> > <richard.xnu.clark@xxxxxxxxx> wrote:
> > >
> > > Both the per-cpu and unbound workqueue will call init_pwq(...) currently,
> > > the latter will init an unbound release work for the pwq which is unnecessary
> > > for the per-cpu type workqueue.
> > > This commit will remove this work item from the per-cpu workqueue by checking the
> > > wq->flags in init_pwq(...), the work is still reserved for the unbound workqueue.
> > >
> > > Signed-off-by: Richard Clark <richard.xnu.clark@xxxxxxxxx>
> > > Cc: Tejun Heo <tj@xxxxxxxxxx>
> > > Cc: Lai Jiangshan <jiangshanlai@xxxxxxxxx>
> > > ---
> > >  kernel/workqueue.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> > > index 7cd5f5e7e0a1..01bdfb74081e 100644
> > > --- a/kernel/workqueue.c
> > > +++ b/kernel/workqueue.c
> > > @@ -3807,7 +3807,8 @@ static void init_pwq(struct pool_workqueue *pwq, struct workqueue_struct *wq,
> > >         INIT_LIST_HEAD(&pwq->inactive_works);
> > >         INIT_LIST_HEAD(&pwq->pwqs_node);
> > >         INIT_LIST_HEAD(&pwq->mayday_node);
> > > -       INIT_WORK(&pwq->unbound_release_work, pwq_unbound_release_workfn);
> > > +       if (wq->flags & WQ_UNBOUND)
> > > +               INIT_WORK(&pwq->unbound_release_work, pwq_unbound_release_workfn);
> > >  }
> > >
> > >  /* sync @pwq with the current state of its associated wq and link it */
> > > --
> > > 2.37.2
> > >