lkml.org 
[lkml]   [2011]   [Mar]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 1/3] futex: do not pagefault_disable in futex_atomic_cmpxchg_inatomic()
From
On Sun, Mar 13, 2011 at 3:49 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> 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!

I understand pagefault_disable() and preempt_disable() are different
concepts, but their implementations have a lot in common...

> So what the f*ck does that "Note that preemption is disabled.." crap even mean?

I was thinking that if cmpxchg_futex_value_locked() already raised the
preempt count, it seemed redundant to do it again (and then decrement
it back) in a nested way within futex_atomic_cmpxchg_inatomic on arm.

> 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.

You got me there - clearly I f*cked up. What really irks me is that I
have a retrospectively bogus memory of actually looking at that call
site and seeing a pagefault_disable there... But as is now obvious,
it's never been there :/

--
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.


\
 
 \ /
  Last update: 2011-03-14 10:17    [W:0.097 / U:0.496 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site