Re: [PATCH 1/5] 2.6.0 fix preempt ctx switch accounting

Ingo Molnar
2003-12-20T21:33:29Z

* Linus Torvalds <torvalds@osdl.org> wrote:

> That patch still gets several cases wrong: we don't update any
> counters at all for the case where we were TASK_INTERRUPTIBLE and we
> got made TASK_RUNNING because of having a signal pending.
>
> Also, we shouldn't update the context switch counter just because we
> entered the scheduler. If we don't actually end up switching to
> anything else, it shouldn't count as a context switch.
>
> So how about something like this?
>
> Totally untested. Comments?

looks good. The indirect pointer nicely gets rid of complexity. I've
done some additional cleanups: fixed a compilation warning on UP and
cleaned up the goto pick_next_task code. Moved the 'unlikely' to the
test as a whole. I've test-booted this patch and the context-switch
stats look OK. Updated patch attached.

Ingo

--- linux/kernel/sched.c.orig
+++ linux/kernel/sched.c
@@ -1477,6 +1477,7 @@ void scheduling_functions_start_here(voi
*/
asmlinkage void schedule(void)
{
+ long *switch_count;
task_t *prev, *next;
runqueue_t *rq;
prio_array_t *array;
@@ -1523,32 +1524,25 @@ need_resched:
* if entering off of a kernel preemption go straight
* to picking the next task.
*/
- if (unlikely(preempt_count() & PREEMPT_ACTIVE))
- goto pick_next_task;
-
- switch (prev->state) {
- case TASK_INTERRUPTIBLE:
- if (unlikely(signal_pending(prev))) {
+ switch_count = &prev->nivcsw;
+ if (prev->state && !(preempt_count() & PREEMPT_ACTIVE)) {
+ switch_count = &prev->nvcsw;
+ if (unlikely((prev->state & TASK_INTERRUPTIBLE) &&
+ unlikely(signal_pending(prev))))
prev->state = TASK_RUNNING;
- break;
- }
- default:
- deactivate_task(prev, rq);
- prev->nvcsw++;
- break;
- case TASK_RUNNING:
- prev->nivcsw++;
+ else
+ deactivate_task(prev, rq);
}
-pick_next_task:
+
if (unlikely(!rq->nr_running)) {
#ifdef CONFIG_SMP
load_balance(rq, 1, cpu_to_node_mask(smp_processor_id()));
- if (rq->nr_running)
- goto pick_next_task;
#endif
- next = rq->idle;
- rq->expired_timestamp = 0;
- goto switch_tasks;
+ if (!rq->nr_running) {
+ next = rq->idle;
+ rq->expired_timestamp = 0;
+ goto switch_tasks;
+ }
}

array = rq->active;
@@ -1596,6 +1590,7 @@ switch_tasks:
next->timestamp = now;
rq->nr_switches++;
rq->curr = next;
+ ++*switch_count;

prepare_arch_switch(rq, next);
prev = context_switch(rq, prev, next);