workqueue: minor cleanup
cover workqueue: minor cleanup
This patchset do minor cleanup for workqueue code.
First patch makes minor behavior change, however, it is trivial. Others doesn't makes any functional difference.
These are based on v3.7-rc1
Joonsoo Kim (3):
workqueue: optimize mod_delayed_work_on() when @delay == 0
workqueue: trivial fix for return statement in work_busy()
workqueue: remove unused argument of wq_worker_waking_up()
kernel/sched/core.c | 2 +- kernel/workqueue.c | 11 +++++++---- kernel/workqueue_sched.h | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-)
Signature · 1 line
1.7.9.5
PATCH 1/3 workqueue: optimize mod_delayed_work_on() when @delay == 0
After try_to_grab_pending(), __queue_delayed_work() is invoked in mod_delayed_work_on(). When @delay == 0, we can call __queue_work() directly in order to avoid setting useless timer.
kernel/workqueue.c +5 -1
@@ -1477,7 +1477,11 @@ bool mod_delayed_work_on(int cpu, struct workqueue_struct *wq, }while (unlikely (ret == -EAGAIN ));if (likely (ret >=0 )) { -__queue_delayed_work (cpu, wq, dwork, delay); +if (!delay) +__queue_work (cpu, wq, &dwork->work); +else +__queue_delayed_work (cpu, wq, dwork, delay); +local_irq_restore (flags); }
Signature · 1 line
1.7.9.5
PATCH 2/3 workqueue: trivial fix for return statement in work_busy()
In reply to Joonsoo KimReturn type of work_busy() is unsigned int.
There is return statement returning boolean value, 'false' in work_busy(). It is not problem, because 'false' may be treated '0'. However, fixing it would make code robust.
kernel/workqueue.c +1 -1
@@ -3479,7 +3479,7 @@ unsigned int work_busy(struct work_struct *work)unsigned int ret =0 ;if (!gcwq) -return false; +return 0 ;spin_lock_irqsave (&gcwq->lock, flags);
Signature · 1 line
1.7.9.5
Tejun Heo
maintainerRe: [PATCH 2/3] workqueue: trivial fix for return statement in work_busy()
Quoted 6 lines · Joonsoo Kim
Return type of
work_busy()is unsigned int.
There is return statement returning boolean value, 'false' inwork_busy(). It is not problem, because 'false' may be treated '0'. However, fixing it would make code robust.Signed-off-by Joonsoo Kim <[email protected]>
Applied 1-2 to wq/for-3.8.
Thanks.
Signature · 1 line
tejun
PATCH 3/3 workqueue: remove unused argument of wq_worker_waking_up()
In reply to Joonsoo KimCommit 63d95a91 ('workqueue: use @pool instead of @gcwq or @cpu where applicable') changes an approach to access nr_running. Thus, wq_worker_waking_up() doesn't use @cpu anymore. Remove it and remove comment related to it.
kernel/sched/core.c +1 -1
@@ -1243,7 +1243,7 @@ static void ttwu_activate(struct rq *rq, struct task_struct *p, int en_flags)/* if a worker is waking up, notify workqueue */ if (p->flags &PF_WQ_WORKER ) -wq_worker_waking_up (p,cpu_of (rq)); +wq_worker_waking_up (p); }/*
kernel/workqueue.c +1 -2
@@ -727,7 +727,6 @@ static void wake_up_worker(struct worker_pool *pool)/** * wq_worker_waking_up - a worker is waking up * @task: task waking up -* @cpu: CPU @task is waking up to * * This function is called during try_to_wake_up() when a worker is * being awoken.
@@ -735,7 +734,7 @@ static void wake_up_worker(struct worker_pool *pool)* CONTEXT: * spin_lock_irq(rq->lock) */ -void wq_worker_waking_up (struct task_struct *task,unsigned int cpu) +void wq_worker_waking_up (struct task_struct *task) {struct worker *worker =kthread_data (task);
kernel/workqueue_sched.h +1 -1
@@ -4,6 +4,6 @@* Scheduler hooks for concurrency managed workqueue. Only to be * included from sched.c and workqueue.c. */ -void wq_worker_waking_up (struct task_struct *task,unsigned int cpu); +void wq_worker_waking_up (struct task_struct *task);struct task_struct *wq_worker_sleeping (struct task_struct *task,unsigned int cpu);
Signature · 1 line
1.7.9.5
Tejun Heo
maintainerRe: [PATCH 3/3] workqueue: remove unused argument of wq_worker_waking_up()
Quoted 6 lines · Joonsoo Kim
Commit
63d95a91('workqueue: use @pool instead of @gcwq or @cpu where applicable') changes an approach to accessnr_running. Thus,wq_worker_waking_up()doesn't use @cpu anymore. Remove it and remove comment related to it.Signed-off-by Joonsoo Kim <[email protected]>
I'm not sure whether I wanna remove or add WARN_ON_ONCE() on it. That part has gone through some changes and seen some bugs. Can we please do the following instead at least for now?
if (!(worker->flags & WORKER_NOT_RUNNING)) {
WARN_ON_ONCE(worker->pool->gcwq->cpu != cpu);
atomic_inc(get_pool_nr_running(worker->pool));
}Thanks.
Signature · 1 line
tejun
Re: [PATCH 3/3] workqueue: remove unused argument of wq_worker_waking_up()
2012/10/21 Tejun Heo <[email protected]>:
Quoted 8 lines · Joonsoo Kim
Quoted 6 lines
Commit
63d95a91('workqueue: use @pool instead of @gcwq or @cpu where applicable') changes an approach to accessnr_running. Thus,wq_worker_waking_up()doesn't use @cpu anymore. Remove it and remove comment related to it.Signed-off-by Joonsoo Kim <[email protected]>
Quoted 9 lines
I'm not sure whether I wanna remove or add
WARN_ON_ONCE()on it. That part has gone through some changes and seen some bugs. Can we please do the following instead at least for now?if (!(worker->flags & WORKER_NOT_RUNNING)) { WARN_ON_ONCE(worker->pool->gcwq->cpu != cpu); atomic_inc(get_pool_nr_running(worker->pool)); }
I have no objection to do this for now.
Thanks.
Tejun Heo
maintainerRe: [PATCH 3/3] workqueue: remove unused argument of wq_worker_waking_up()
Quoted 7 lines · JoonSoo Kim
Quoted 5 lines
if (!(worker->flags & WORKER_NOT_RUNNING)) { WARN_ON_ONCE(worker->pool->gcwq->cpu != cpu); atomic_inc(get_pool_nr_running(worker->pool)); }I have no objection to do this for now.
Care to cook up a patch?
Thanks.
--
tejun