Re: [RFC PATCH 1/4] workqueue: support pausing ordered workqueues

From: Johannes Berg
Date: Wed May 10 2023 - 14:40:45 EST


Hi,

> > - /* for @wq->saved_max_active */
> > + /* for @wq->saved_max_active and @wq->flags */
> > lockdep_assert_held(&wq->mutex);
> >
> > + if (wq->flags & __WQ_PAUSED)
> > + new_max_active = 0;
> > + else
> > + new_max_active = wq->saved_max_active;
>
> Nothing is using new_max_active and I think we can probably combine this
> with the freezing test.

Err, yikes. Of course this was meant to be used in the remainder of the
code, oops.

Regarding the freezing test, yeah, maybe. It seemed harder to follow,
but I'll take another look.

> > +void __workqueue_pause_resume(struct workqueue_struct *wq, bool pause)
> > +{
> > + struct pool_workqueue *pwq;
> > +
> > + mutex_lock(&wq->mutex);
> > + if (pause)
> > + wq->flags |= __WQ_PAUSED;
> > + else
> > + wq->flags &= ~__WQ_PAUSED;
> > +
> > + for_each_pwq(pwq, wq)
> > + pwq_adjust_max_active(pwq);
> > + mutex_unlock(&wq->mutex);
> > +
> > + if (pause)
> > + flush_workqueue(wq);
> > +}
> > +EXPORT_SYMBOL_GPL(__workqueue_pause_resume);
>
> I'd just make pause and resume separate functions. The sharing ratio doesn't
> seem that high.
>

Yeah, I wasn't really sure about that either. I keep thinking the
EXPORT_SYMBOL_GPL() itself is really big, but I'm not even sure about
that, and it's probably not a great reason anyway.

johannes