Re: [PATCH] kernel/futex.c: Uneeded memory barrier

[Jamie Lokier]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/