Re: [PATCH 07/31] workqueue: restructure pool / pool_workqueue iterations in freeze/thaw functions

Tejun Heo
2013-03-10T12:34:51Z

Hello, Lai.

On Sun, Mar 10, 2013 at 06:09:17PM +0800, Lai Jiangshan wrote:
> > + /* clear FREEZING */
> > + for_each_pool(pool, id) {
> > + spin_lock(&pool->lock);
> > + WARN_ON_ONCE(!(pool->flags & POOL_FREEZING));
> > + pool->flags &= ~POOL_FREEZING;
> > + spin_unlock(&pool->lock);
> > + }
>
>
> I think it would be better if we move this code to ...
>
> >
> > - wake_up_worker(pool);
> > + /* restore max_active and repopulate worklist */
> > + list_for_each_entry(wq, &workqueues, list) {
> > + if (!(wq->flags & WQ_FREEZABLE))
> > + continue;
> >
> > - spin_unlock(&pool->lock);
> > + for_each_pwq(pwq, wq) {
> > + spin_lock(&pwq->pool->lock);
> > + pwq_set_max_active(pwq, wq->saved_max_active);
> > + spin_unlock(&pwq->pool->lock);
> > }
> > }
> >
> > + /* kick workers */
> > + for_each_pool(pool, id) {
> > + spin_lock(&pool->lock);
> > + wake_up_worker(pool);
> > + spin_unlock(&pool->lock);
> > + }
>
>
> ... to here.
>
> clear FREEZING and then kick.

Yeah, that would be prettier but also change the order of operations
which I'd like to keep the same across the conversion. We can create
a separate patch to merge the two loops later. Care to send a
separate patch?

Thanks.

--
tejun