lkml.org 
[lkml]   [2011]   [Mar]   [13]   [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
    I have to apologize. These landed right as I was leaving for OSTS and
    I didn't have time to review them properly. I wondered about the
    preempt vs pagefault disable and wondered how exit_robust_list() was
    covered in Michel's comments (perhaps somewhere higher up the call
    chain). I leaned on the review of others when I should have raised the
    questions even if I didn't have the time to dig into them myself.
    Linus shouldn't have had to raise those questions, I'll do better at
    this in the future.

    I noticed that my name is the only one in futex.c with an email
    address in the header - that email address is no longer valid, and it
    delayed the patches getting to my inbox - I will submit a patch to fix
    that. I should catch them sooner now, regardless, with improved LKML
    filters.

    Thanks,

    Darren Hart


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



    --
    Darren Hart
    --
    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/

    \
     
     \ /
      Last update: 2011-03-14 01:57    [W:0.026 / U:29.848 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site