Re: [PATCH 2/2] workqueue: remove the argument @wakeup from worker_set_flags()
On Fri, Jul 18, 2014 at 06:38:26PM -0400, Tejun Heo wrote: > On Wed, Jul 16, 2014 at 06:09:59PM +0800, Lai Jiangshan wrote: > > worker_set_flags() doesn't necessarily wake next worker and the @wakeup > > can be removed, the caller can use the following conbination instead > > when needed: > > > > worker_set_flags(); > > if (need_more_worker(pool)) > > wake_up_worker(pool); > > Hmmm, yeah, there were more places where worker_set_flags() was used > but it does seem excessive now. > > > @@ -2045,7 +2032,7 @@ __acquires(>lock) > > * management. They're the scheduler's responsibility. > > */ > > if (unlikely(cpu_intensive)) > > - worker_set_flags(worker, WORKER_CPU_INTENSIVE, true); > > + worker_set_flags(worker, WORKER_CPU_INTENSIVE); > > But let's do this separately. Please drop the previous patch and > perform need_more_worker() test explicitly after setting > CPU_INTENSIVE. So, we can do it together at need_more_workers() but let's please explain how different cases would behave there. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] workqueue: remove the argument @wakeup from worker_set_flags()
On Wed, Jul 16, 2014 at 06:09:59PM +0800, Lai Jiangshan wrote: > worker_set_flags() doesn't necessarily wake next worker and the @wakeup > can be removed, the caller can use the following conbination instead > when needed: > > worker_set_flags(); > if (need_more_worker(pool)) > wake_up_worker(pool); Hmmm, yeah, there were more places where worker_set_flags() was used but it does seem excessive now. > @@ -2045,7 +2032,7 @@ __acquires(>lock) >* management. They're the scheduler's responsibility. >*/ > if (unlikely(cpu_intensive)) > - worker_set_flags(worker, WORKER_CPU_INTENSIVE, true); > + worker_set_flags(worker, WORKER_CPU_INTENSIVE); But let's do this separately. Please drop the previous patch and perform need_more_worker() test explicitly after setting CPU_INTENSIVE. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] workqueue: remove the argument @wakeup from worker_set_flags()
On Wed, Jul 16, 2014 at 06:09:59PM +0800, Lai Jiangshan wrote: worker_set_flags() doesn't necessarily wake next worker and the @wakeup can be removed, the caller can use the following conbination instead when needed: worker_set_flags(); if (need_more_worker(pool)) wake_up_worker(pool); Hmmm, yeah, there were more places where worker_set_flags() was used but it does seem excessive now. @@ -2045,7 +2032,7 @@ __acquires(pool-lock) * management. They're the scheduler's responsibility. */ if (unlikely(cpu_intensive)) - worker_set_flags(worker, WORKER_CPU_INTENSIVE, true); + worker_set_flags(worker, WORKER_CPU_INTENSIVE); But let's do this separately. Please drop the previous patch and perform need_more_worker() test explicitly after setting CPU_INTENSIVE. Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] workqueue: remove the argument @wakeup from worker_set_flags()
On Fri, Jul 18, 2014 at 06:38:26PM -0400, Tejun Heo wrote: On Wed, Jul 16, 2014 at 06:09:59PM +0800, Lai Jiangshan wrote: worker_set_flags() doesn't necessarily wake next worker and the @wakeup can be removed, the caller can use the following conbination instead when needed: worker_set_flags(); if (need_more_worker(pool)) wake_up_worker(pool); Hmmm, yeah, there were more places where worker_set_flags() was used but it does seem excessive now. @@ -2045,7 +2032,7 @@ __acquires(pool-lock) * management. They're the scheduler's responsibility. */ if (unlikely(cpu_intensive)) - worker_set_flags(worker, WORKER_CPU_INTENSIVE, true); + worker_set_flags(worker, WORKER_CPU_INTENSIVE); But let's do this separately. Please drop the previous patch and perform need_more_worker() test explicitly after setting CPU_INTENSIVE. So, we can do it together at need_more_workers() but let's please explain how different cases would behave there. Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] workqueue: remove the argument @wakeup from worker_set_flags()
worker_set_flags() doesn't necessarily wake next worker and the @wakeup can be removed, the caller can use the following conbination instead when needed: worker_set_flags(); if (need_more_worker(pool)) wake_up_worker(pool); Signed-off-by: Lai Jiangshan --- kernel/workqueue.c | 25 ++--- 1 files changed, 6 insertions(+), 19 deletions(-) diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 6d11b9a..1a14f18 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -867,35 +867,22 @@ struct task_struct *wq_worker_sleeping(struct task_struct *task, int cpu) * worker_set_flags - set worker flags and adjust nr_running accordingly * @worker: self * @flags: flags to set - * @wakeup: wakeup an idle worker if necessary * - * Set @flags in @worker->flags and adjust nr_running accordingly. If - * nr_running becomes zero and @wakeup is %true, an idle worker is - * woken up. + * Set @flags in @worker->flags and adjust nr_running accordingly. * * CONTEXT: * spin_lock_irq(pool->lock) */ -static inline void worker_set_flags(struct worker *worker, unsigned int flags, - bool wakeup) +static inline void worker_set_flags(struct worker *worker, unsigned int flags) { struct worker_pool *pool = worker->pool; WARN_ON_ONCE(worker->task != current); - /* -* If transitioning into NOT_RUNNING, adjust nr_running and -* wake up an idle worker as necessary if requested by -* @wakeup. -*/ + /* If transitioning into NOT_RUNNING, adjust nr_running. */ if ((flags & WORKER_NOT_RUNNING) && !(worker->flags & WORKER_NOT_RUNNING)) { - if (wakeup) { - if (atomic_dec_and_test(>nr_running) && - !list_empty(>worklist)) - wake_up_worker(pool); - } else - atomic_dec(>nr_running); + atomic_dec(>nr_running); } worker->flags |= flags; @@ -2045,7 +2032,7 @@ __acquires(>lock) * management. They're the scheduler's responsibility. */ if (unlikely(cpu_intensive)) - worker_set_flags(worker, WORKER_CPU_INTENSIVE, true); + worker_set_flags(worker, WORKER_CPU_INTENSIVE); /* Wake up another worker if necessary. */ if (need_more_worker(pool)) @@ -2204,7 +2191,7 @@ recheck: } } while (keep_working(pool)); - worker_set_flags(worker, WORKER_PREP, false); + worker_set_flags(worker, WORKER_PREP); sleep: /* * pool->lock is held and there's no work to process and no need to -- 1.7.4.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] workqueue: remove the argument @wakeup from worker_set_flags()
worker_set_flags() doesn't necessarily wake next worker and the @wakeup can be removed, the caller can use the following conbination instead when needed: worker_set_flags(); if (need_more_worker(pool)) wake_up_worker(pool); Signed-off-by: Lai Jiangshan la...@cn.fujitsu.com --- kernel/workqueue.c | 25 ++--- 1 files changed, 6 insertions(+), 19 deletions(-) diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 6d11b9a..1a14f18 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -867,35 +867,22 @@ struct task_struct *wq_worker_sleeping(struct task_struct *task, int cpu) * worker_set_flags - set worker flags and adjust nr_running accordingly * @worker: self * @flags: flags to set - * @wakeup: wakeup an idle worker if necessary * - * Set @flags in @worker-flags and adjust nr_running accordingly. If - * nr_running becomes zero and @wakeup is %true, an idle worker is - * woken up. + * Set @flags in @worker-flags and adjust nr_running accordingly. * * CONTEXT: * spin_lock_irq(pool-lock) */ -static inline void worker_set_flags(struct worker *worker, unsigned int flags, - bool wakeup) +static inline void worker_set_flags(struct worker *worker, unsigned int flags) { struct worker_pool *pool = worker-pool; WARN_ON_ONCE(worker-task != current); - /* -* If transitioning into NOT_RUNNING, adjust nr_running and -* wake up an idle worker as necessary if requested by -* @wakeup. -*/ + /* If transitioning into NOT_RUNNING, adjust nr_running. */ if ((flags WORKER_NOT_RUNNING) !(worker-flags WORKER_NOT_RUNNING)) { - if (wakeup) { - if (atomic_dec_and_test(pool-nr_running) - !list_empty(pool-worklist)) - wake_up_worker(pool); - } else - atomic_dec(pool-nr_running); + atomic_dec(pool-nr_running); } worker-flags |= flags; @@ -2045,7 +2032,7 @@ __acquires(pool-lock) * management. They're the scheduler's responsibility. */ if (unlikely(cpu_intensive)) - worker_set_flags(worker, WORKER_CPU_INTENSIVE, true); + worker_set_flags(worker, WORKER_CPU_INTENSIVE); /* Wake up another worker if necessary. */ if (need_more_worker(pool)) @@ -2204,7 +2191,7 @@ recheck: } } while (keep_working(pool)); - worker_set_flags(worker, WORKER_PREP, false); + worker_set_flags(worker, WORKER_PREP); sleep: /* * pool-lock is held and there's no work to process and no need to -- 1.7.4.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/