Hi Linus,
When I checked the serial driver integration into 2.3.16, I noticed that
one of the changes was to change
current->state = TASK_INTERRUPTIBLE;
to
set_current_state(TASK_INTERRUPTIBLE);
Looking at the sched.h header file, it's pretty clear that this is there
to protect against race conditions on SMP kernels. Which makes me
wonder why you only changed that one line, but not all of the other
lines in the driver which set current->state. Doing a quick grep over
drivers/char/*.c, I see a lot of drivers which still use "current->state
= ... " and some that use "set_current_state(...)"
Are there any circumstances where we should keep the old usage? Or
should we change them all to use set_current_state()? And if it's the
latter, would you like a patch which does a global replace of
"current->state = ..." to "set_current_state(...)", either in the serial
driver or in all files in drivers/char/*.c (where most of the old usage
seems to be concentrated).
Thanks!
- Ted
Date: Wed, 1 Sep 1999 16:17:07 -0400
From: "Theodore Y. Ts'o" <[email protected]>
Are there any circumstances where we should keep the old usage?
Fundamentally it only matters when you are going into a "sleep until
condition" polling loop such that:
add_wait_queue(...);
for(;;) {
set_current_state(...);
if (some asynchronous state)
break;
schedule();
}
remove_wait_queue(...);
The idea being that you wish the task state to propagate into the view
of all cpus in the system before other cpus can potentially cause the
asynronous event to occur _and_ check your task state while performing
the wake_up(...)
What we're trying to prevent here is the asynchronous state test load
not bypassing the store of the new task state. If the cpu reorders
these, the wakeup event can be missed.
Later,
David S. Miller
[email protected]