Re: [RFC][PATCH v3 7/10] workqueue: add WQ_IDLEPRI

From: KAMEZAWA Hiroyuki
Date: Thu May 26 2011 - 06:37:24 EST


On Thu, 26 May 2011 11:38:08 +0200
Tejun Heo <tj@xxxxxxxxxx> wrote:

> Hello, KAMEZAWA.
>
> On Thu, May 26, 2011 at 02:30:24PM +0900, KAMEZAWA Hiroyuki wrote:
> > When this idea came to me, I wonder which is better to maintain
> > memcg's thread pool or add support in workqueue for generic use. In
> > genral, I feel enhancing genric one is better...so, wrote this one.
>
> Sure, if it's something which can be useful for other users, it makes
> sense to make it generic.
>
Thank you for review.


> > Index: memcg_async/include/linux/workqueue.h
> > ===================================================================
> > --- memcg_async.orig/include/linux/workqueue.h
> > +++ memcg_async/include/linux/workqueue.h
> > @@ -56,7 +56,8 @@ enum {
> >
> > /* special cpu IDs */
> > WORK_CPU_UNBOUND = NR_CPUS,
> > - WORK_CPU_NONE = NR_CPUS + 1,
> > + WORK_CPU_IDLEPRI = NR_CPUS + 1,
> > + WORK_CPU_NONE = NR_CPUS + 2,
> > WORK_CPU_LAST = WORK_CPU_NONE,
>
> Hmmm... so, you're defining another fake CPU a la unbound CPU. I'm
> not sure whether it's really necessary to create its own worker pool
> tho. The reason why SCHED_OTHER is necessary is because it may
> consume large amount of CPU cycles. Workqueue already has UNBOUND -
> for an unbound one, workqueue code simply acts as generic worker pool
> provider and everything other than work item dispatching and worker
> management are deferred to scheduler and the workqueue user.
>
yes.

> Is there any reason memcg can't just use UNBOUND workqueue and set
> scheduling priority when the work item starts and restore it when it's
> done?

I thought of that. But I didn't do that because I wasn't sure how others
will think about changing exisitng workqueue priority...and I was curious
to know how workqueue works.

> If it's gonna be using UNBOUND at all, I don't think changing
> scheduling policy would be a noticeable overhead and I find having
> separate worker pools depending on scheduling priority somewhat silly.
>
ok.

> We can add a mechanism to manage work item scheduler priority to
> workqueue if necessary tho, I think. But that would be per-workqueue
> attribute which is applied during execution, not something per-gcwq.
>

In the next version, I'll try some like..
==
process_one_work(...) {
.....
spin_unlock_irq(&gcwq->lock);
.....
if (cwq->wq->flags & WQ_IDLEPRI) {
set_scheduler(...SCHED_IDLE...)
cond_resched();
scheduler_switched = true;
}
f(work)
if (scheduler_switched)
set_scheduler(...SCHED_OTHER...)
spin_lock_irq(&gcwq->lock);
}
==
Patch size will be much smaller. (Should I do this in memcg's code ??)

Thank you for your advices.

Thanks,
-Kame


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/