diff options
author | Steven Rostedt <srostedt@redhat.com> | 2010-12-03 23:12:33 -0500 |
---|---|---|
committer | Tejun Heo <tj@kernel.org> | 2010-12-14 09:05:54 -0500 |
commit | 2d64672ed38721b7a3815009d79bfb90a1f34a17 (patch) | |
tree | 91a52918b036a07bf8008eeae6e7dccf967fa4e0 /kernel/workqueue.c | |
parent | 3e6cd7a4b6a04cf354a18c9d2e7ecec8fa1772fb (diff) |
workqueue: It is likely that WORKER_NOT_RUNNING is true
Running the annotate branch profiler on three boxes, including my
main box that runs firefox, evolution, xchat, and is part of the distcc farm,
showed this with the likelys in the workqueue code:
correct incorrect % Function File Line
------- --------- - -------- ---- ----
96 996253 99 wq_worker_sleeping workqueue.c 703
96 996247 99 wq_worker_waking_up workqueue.c 677
The likely()s in this case were assuming that WORKER_NOT_RUNNING will
most likely be false. But this is not the case. The reason is
(and shown by adding trace_printks and testing it) that most of the time
WORKER_PREP is set.
In worker_thread() we have:
worker_clr_flags(worker, WORKER_PREP);
[ do work stuff ]
worker_set_flags(worker, WORKER_PREP, false);
(that 'false' means not to wake up an idle worker)
The wq_worker_sleeping() is called from schedule when a worker thread
is putting itself to sleep. Which happens most of the time outside
of that [ do work stuff ].
The wq_worker_waking_up is called by the wakeup worker code, which
is also callod outside that [ do work stuff ].
Thus, the likely and unlikely used by those two functions are actually
backwards.
Remove the annotation and let gcc figure it out.
Acked-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Tejun Heo <tj@kernel.org>
Diffstat (limited to 'kernel/workqueue.c')
-rw-r--r-- | kernel/workqueue.c | 4 |
1 files changed, 2 insertions, 2 deletions
diff --git a/kernel/workqueue.c b/kernel/workqueue.c index ca017ce8bc6b..e785b0f2aea5 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c | |||
@@ -661,7 +661,7 @@ void wq_worker_waking_up(struct task_struct *task, unsigned int cpu) | |||
661 | { | 661 | { |
662 | struct worker *worker = kthread_data(task); | 662 | struct worker *worker = kthread_data(task); |
663 | 663 | ||
664 | if (likely(!(worker->flags & WORKER_NOT_RUNNING))) | 664 | if (!(worker->flags & WORKER_NOT_RUNNING)) |
665 | atomic_inc(get_gcwq_nr_running(cpu)); | 665 | atomic_inc(get_gcwq_nr_running(cpu)); |
666 | } | 666 | } |
667 | 667 | ||
@@ -687,7 +687,7 @@ struct task_struct *wq_worker_sleeping(struct task_struct *task, | |||
687 | struct global_cwq *gcwq = get_gcwq(cpu); | 687 | struct global_cwq *gcwq = get_gcwq(cpu); |
688 | atomic_t *nr_running = get_gcwq_nr_running(cpu); | 688 | atomic_t *nr_running = get_gcwq_nr_running(cpu); |
689 | 689 | ||
690 | if (unlikely(worker->flags & WORKER_NOT_RUNNING)) | 690 | if (worker->flags & WORKER_NOT_RUNNING) |
691 | return NULL; | 691 | return NULL; |
692 | 692 | ||
693 | /* this can only happen on the local cpu */ | 693 | /* this can only happen on the local cpu */ |