Messages in this thread | | | From | Linus Torvalds <> | Date | Sun, 13 Mar 2011 15:49:15 -0700 | Subject | Re: [PATCH 1/3] futex: do not pagefault_disable in futex_atomic_cmpxchg_inatomic() |
| |
On Thu, Mar 10, 2011 at 6:47 PM, Michel Lespinasse <walken@google.com> wrote: > kernel/futex.c disables page faults before calling > futex_atomic_cmpxchg_inatomic(), so there is no need to do it again > within that function.
This seems totally bogus.
Even the comment is crap.
Sure, the callers may disable preemption, but that has NOTHING to do with "pagefault_disable()". Th epagefault_[en/dis]able functions will touch the preempt count EVEN IF PREEMPTION ISN'T EVEN ENABLED!
So what the f*ck does that "Note that preemption is disabled.." crap even mean?
The thing is made even worse by the fact that as far as I can tell, the comment simply isn't true at all (even if you were to ignore the fundamental confusion about preemption vs the pagefault disable/enable). Not all callers of futex_atomic_cmpxchg_inatomic() do anything of the sort, whether it's preemptibility _or_ the proper pagefault_disable/enable(). Just look at the exit_robust_list() -> handle_futex_death(), for example.
This kind of patch is the kind that personally makes me want to put you on a spam-list. Misleading commit messages with bogus and fundamentally incorrect added comments in the code. WTF?
Did I miss some patch that changed that, or is this really as horribly bad as I think it is? I see it already made it into -tip.
Linus
| |