Messages in this thread | | | Date | Sun, 14 Sep 2003 15:08:39 +0100 | From | Jamie Lokier <> | Subject | Re: [PATCH] kernel/futex.c: Uneeded memory barrier |
| |
Rusty Russell wrote: > I personally *HATE* the set_task_state()/__set_task_state() macros. > Simple assignments shouldn't be hidden behind macros, unless there's > something really subtle involved.
There _is_ something subtle involved. Back in ye olde days, folk would set current->state directly. Andrea Arcangeli noticed a subtle race condition, where a memory barrier is needed after assigning the state and before testing whatever condition the task is waiting on, otherwise the state could be written after the condition was tested.
That's when all assignments to current->state were changed to set_task_state().
Sprinkling special kinds of memory barrier into all the drivers is not the kind of thing driver writers get right. Also if you look at the details, the barrier is quite an unusual kind on i386, using the "xchg" instruction, and it only needs to be a CPU barrier on SMP systems.
> Personally, when there's a normal and a __ version of a function, I > use the normal version unless there's a real (performance or > correctness) reason. (ie. I prefer the "think less" version 8).
It's always correct to use the "normal" version of set_task_state().
The places where __set_task_state() is used are performance tweaks, because the memory barrier is not for free. That's why you see it throughout "core" kernel code, where the authors devote more energy to thinking about the SMP subtleties.
-- Jamie - 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/
| |