lkml.org 
[lkml]   [2011]   [May]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC][PATCH v3 7/10] workqueue: add WQ_IDLEPRI
    On Thu, 26 May 2011 19:30:18 +0900
    KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:

    > On Thu, 26 May 2011 11:38:08 +0200
    > Tejun Heo <tj@kernel.org> 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 ??)
    >

    BTW, my concern is that if f(work) is enough short,effect of SCHED_IDLE will never
    be found because SCHED_OTHER -> SCHED_IDLE -> SCHED_OTHER switch is very fast.
    Changed "weight" of CFQ never affects the next calculation of vruntime..of the
    thread and the work will show the same behavior with SCHED_OTHER.

    I'm sorry if I misunderstand CFQ and setscheduler().

    Thanks,
    -Kame






    \
     
     \ /
      Last update: 2011-05-26 12:59    [W:0.026 / U:31.192 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site