Re: [PATCH 4/7 V6] workqueue: fix idle worker depletion

2012-09-09 Thread Tejun Heo
Hello, Lai. On Sun, Sep 09, 2012 at 12:09:58PM +0800, Lai Jiangshan wrote: > > mutex_lock(>manager_mutex); > > if (worker_maybe_bind_and_lock(worker)) > > worker_clr_flags(worker, WORKER_UNBOUND); > > ret = true; > > > > > This code is correct.

Re: [PATCH 4/7 V6] workqueue: fix idle worker depletion

2012-09-09 Thread Tejun Heo
Hello, Lai. On Sun, Sep 09, 2012 at 12:09:58PM +0800, Lai Jiangshan wrote: mutex_lock(pool-manager_mutex); if (worker_maybe_bind_and_lock(worker)) worker_clr_flags(worker, WORKER_UNBOUND); ret = true; This code is correct.

Re: [PATCH 4/7 V6] workqueue: fix idle worker depletion

2012-09-08 Thread Lai Jiangshan
On Sun, Sep 9, 2012 at 3:02 AM, Tejun Heo wrote: > Hello, Lai. > > On Sun, Sep 09, 2012 at 02:34:02AM +0800, Lai Jiangshan wrote: >> in 3.6 busy_worker_rebind() handle WORKER_REBIND bit, >> not WORKER_UNBOUND bit. >> >> busy_worker_rebind() takes struct work_struct *work argument, we have to >>

Re: [PATCH 4/7 V6] workqueue: fix idle worker depletion

2012-09-08 Thread Tejun Heo
Hello, Lai. On Sun, Sep 09, 2012 at 02:34:02AM +0800, Lai Jiangshan wrote: > in 3.6 busy_worker_rebind() handle WORKER_REBIND bit, > not WORKER_UNBOUND bit. > > busy_worker_rebind() takes struct work_struct *work argument, we have to > add new patch to add a helper and restruct it at first.

Re: [PATCH 4/7 V6] workqueue: fix idle worker depletion

2012-09-08 Thread Lai Jiangshan
On Sun, Sep 9, 2012 at 2:11 AM, Tejun Heo wrote: > On Sun, Sep 09, 2012 at 02:07:50AM +0800, Lai Jiangshan wrote: >> when we release gcwq->lock and then grab it, we leave a hole that things >> can be changed. >> >> I don't want to open a hole. if the hole has bug we have to fix it. >> if the hole

Re: [PATCH 4/7 V6] workqueue: fix idle worker depletion

2012-09-08 Thread Tejun Heo
On Sun, Sep 09, 2012 at 02:07:50AM +0800, Lai Jiangshan wrote: > when we release gcwq->lock and then grab it, we leave a hole that things > can be changed. > > I don't want to open a hole. if the hole has bug we have to fix it. > if the hole has no bug, we have to add lot of comments to explain

Re: [PATCH 4/7 V6] workqueue: fix idle worker depletion

2012-09-08 Thread Lai Jiangshan
On Sun, Sep 9, 2012 at 1:53 AM, Tejun Heo wrote: > Hello, > > On Sun, Sep 09, 2012 at 01:50:41AM +0800, Lai Jiangshan wrote: >> >> + if (worker_maybe_bind_and_lock(manager)) >> >> + worker_clr_flags(manager, WORKER_UNBOUND); >> >> + } >> >> +} >> > >> > We can

Re: [PATCH 4/7 V6] workqueue: fix idle worker depletion

2012-09-08 Thread Tejun Heo
Hello, On Sun, Sep 09, 2012 at 01:50:41AM +0800, Lai Jiangshan wrote: > >> + if (worker_maybe_bind_and_lock(manager)) > >> + worker_clr_flags(manager, WORKER_UNBOUND); > >> + } > >> +} > > > > We can reuse busy_worker_rebind_fn(), right? > >

Re: [PATCH 4/7 V6] workqueue: fix idle worker depletion

2012-09-08 Thread Lai Jiangshan
On Sun, Sep 9, 2012 at 1:40 AM, Tejun Heo wrote: > Hello, Lai. > > On Sun, Sep 09, 2012 at 01:12:53AM +0800, Lai Jiangshan wrote: >> +/* does the manager need to be rebind after we just release gcwq->lock */ >> +static void maybe_rebind_manager(struct worker *manager) >> +{ >> + struct

Re: [PATCH 4/7 V6] workqueue: fix idle worker depletion

2012-09-08 Thread Tejun Heo
Hello, Lai. On Sun, Sep 09, 2012 at 01:12:53AM +0800, Lai Jiangshan wrote: > +/* does the manager need to be rebind after we just release gcwq->lock */ > +static void maybe_rebind_manager(struct worker *manager) > +{ > + struct global_cwq *gcwq = manager->pool->gcwq; > + bool assoc =

[PATCH 4/7 V6] workqueue: fix idle worker depletion

2012-09-08 Thread Lai Jiangshan
If hotplug code grabbed the manager_mutex and worker_thread try to create a worker, the manage_worker() will return false and worker_thread go to process work items. Now, on the CPU, all workers are processing work items, no idle_worker left/ready for managing. It breaks the concept of workqueue

[PATCH 4/7 V6] workqueue: fix idle worker depletion

2012-09-08 Thread Lai Jiangshan
If hotplug code grabbed the manager_mutex and worker_thread try to create a worker, the manage_worker() will return false and worker_thread go to process work items. Now, on the CPU, all workers are processing work items, no idle_worker left/ready for managing. It breaks the concept of workqueue

Re: [PATCH 4/7 V6] workqueue: fix idle worker depletion

2012-09-08 Thread Tejun Heo
Hello, Lai. On Sun, Sep 09, 2012 at 01:12:53AM +0800, Lai Jiangshan wrote: +/* does the manager need to be rebind after we just release gcwq-lock */ +static void maybe_rebind_manager(struct worker *manager) +{ + struct global_cwq *gcwq = manager-pool-gcwq; + bool assoc = !(gcwq-flags

Re: [PATCH 4/7 V6] workqueue: fix idle worker depletion

2012-09-08 Thread Lai Jiangshan
On Sun, Sep 9, 2012 at 1:40 AM, Tejun Heo t...@kernel.org wrote: Hello, Lai. On Sun, Sep 09, 2012 at 01:12:53AM +0800, Lai Jiangshan wrote: +/* does the manager need to be rebind after we just release gcwq-lock */ +static void maybe_rebind_manager(struct worker *manager) +{ + struct

Re: [PATCH 4/7 V6] workqueue: fix idle worker depletion

2012-09-08 Thread Tejun Heo
Hello, On Sun, Sep 09, 2012 at 01:50:41AM +0800, Lai Jiangshan wrote: + if (worker_maybe_bind_and_lock(manager)) + worker_clr_flags(manager, WORKER_UNBOUND); + } +} We can reuse busy_worker_rebind_fn(), right? busy_worker_rebind_fn() releases the

Re: [PATCH 4/7 V6] workqueue: fix idle worker depletion

2012-09-08 Thread Lai Jiangshan
On Sun, Sep 9, 2012 at 1:53 AM, Tejun Heo t...@kernel.org wrote: Hello, On Sun, Sep 09, 2012 at 01:50:41AM +0800, Lai Jiangshan wrote: + if (worker_maybe_bind_and_lock(manager)) + worker_clr_flags(manager, WORKER_UNBOUND); + } +} We can reuse

Re: [PATCH 4/7 V6] workqueue: fix idle worker depletion

2012-09-08 Thread Tejun Heo
On Sun, Sep 09, 2012 at 02:07:50AM +0800, Lai Jiangshan wrote: when we release gcwq-lock and then grab it, we leave a hole that things can be changed. I don't want to open a hole. if the hole has bug we have to fix it. if the hole has no bug, we have to add lot of comments to explain it.

Re: [PATCH 4/7 V6] workqueue: fix idle worker depletion

2012-09-08 Thread Lai Jiangshan
On Sun, Sep 9, 2012 at 2:11 AM, Tejun Heo t...@kernel.org wrote: On Sun, Sep 09, 2012 at 02:07:50AM +0800, Lai Jiangshan wrote: when we release gcwq-lock and then grab it, we leave a hole that things can be changed. I don't want to open a hole. if the hole has bug we have to fix it. if the

Re: [PATCH 4/7 V6] workqueue: fix idle worker depletion

2012-09-08 Thread Tejun Heo
Hello, Lai. On Sun, Sep 09, 2012 at 02:34:02AM +0800, Lai Jiangshan wrote: in 3.6 busy_worker_rebind() handle WORKER_REBIND bit, not WORKER_UNBOUND bit. busy_worker_rebind() takes struct work_struct *work argument, we have to add new patch to add a helper and restruct it at first. What's

Re: [PATCH 4/7 V6] workqueue: fix idle worker depletion

2012-09-08 Thread Lai Jiangshan
On Sun, Sep 9, 2012 at 3:02 AM, Tejun Heo t...@kernel.org wrote: Hello, Lai. On Sun, Sep 09, 2012 at 02:34:02AM +0800, Lai Jiangshan wrote: in 3.6 busy_worker_rebind() handle WORKER_REBIND bit, not WORKER_UNBOUND bit. busy_worker_rebind() takes struct work_struct *work argument, we have to