On Tue, 20 Mar 2001, Roger Larsson wrote:
> One little readability thing I found.
> The prev->state TASK_ value is mostly used as a plain value
> but the new TASK_PREEMPTED is or:ed together with whatever was there.
> Later when we switch to check the state it is checked against TASK_PREEMPTED
> only. Since TASK_RUNNING is 0 it works OK but...
Yes, you're right. I had forgotten that TASK_RUNNING is 0 and I think I
was assuming that there could be (rare) cases where a task was preempted
while prev->state was in transition such that no other flags were set.
This is, of course, impossible given that TASK_RUNNING is 0. So your
change makes the common case more obvious (to me, at least!)
> --- sched.c.nigel Tue Mar 20 18:52:43 2001
> +++ sched.c.roger Tue Mar 20 19:03:28 2001
> @@ -553,7 +553,7 @@
> #endif
> del_from_runqueue(prev);
> #ifdef CONFIG_PREEMPT
> - case TASK_PREEMPTED:
> + case TASK_RUNNING | TASK_PREEMPTED:
> #endif
> case TASK_RUNNING:
> }
>
>
> We could add all/(other common) combinations as cases
>
> switch (prev->state) {
> case TASK_INTERRUPTIBLE:
> if (signal_pending(prev)) {
> prev->state = TASK_RUNNING;
> break;
> }
> default:
> #ifdef CONFIG_PREEMPT
> if (prev->state & TASK_PREEMPTED)
> break;
> #endif
> del_from_runqueue(prev);
> #ifdef CONFIG_PREEMPT
> case TASK_RUNNING | TASK_PREEMPTED:
> case TASK_INTERRUPTIBLE | TASK_PREEMPTED:
> case TASK_UNINTERRUPTIBLE | TASK_PREEMPTED:
> #endif
> case TASK_RUNNING:
> }
>
>
> Then the break in default case could almost be replaced with a BUG()...
> (I have not checked the generated code)
The other cases are not very common, as they only happen if a task is
preempted during the short time that it is running while in the process
of changing state while going to sleep or waking up, so the default case
is probably OK for them; and I'd be happier to leave the default case
for reliability reasons anyway.
Nigel Gamble nigel@nrg.org
Mountain View, CA, USA. http://www.nrg.org/
MontaVista Software nigel@mvista.com
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Fri Mar 23 2001 - 21:00:14 EST