Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756687Ab1CNAzr (ORCPT ); Sun, 13 Mar 2011 20:55:47 -0400 Received: from mail-qy0-f174.google.com ([209.85.216.174]:54027 "EHLO mail-qy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755894Ab1CNAzq convert rfc822-to-8bit (ORCPT ); Sun, 13 Mar 2011 20:55:46 -0400 MIME-Version: 1.0 In-Reply-To: References: <20110307021127.GB31188@google.com> <20110309112550.GA3050@google.com> <20110311021654.GA26122@google.com> <20110311024731.GB26122@google.com> Date: Sun, 13 Mar 2011 17:55:45 -0700 X-Google-Sender-Auth: S8EvMZxM7jbCibGoEa92H6pwcMI Message-ID: Subject: Re: [PATCH 1/3] futex: do not pagefault_disable in futex_atomic_cmpxchg_inatomic() From: Darren Hart To: Linus Torvalds Cc: Michel Lespinasse , Thomas Gleixner , Ingo Molnar , Peter Zijlstra , Matt Turner , Russell King , David Howells , Tony Luck , Michal Simek , Ralf Baechle , "James E.J. Bottomley" , Benjamin Herrenschmidt , Martin Schwidefsky , Paul Mundt , "David S. Miller" , Chris Metcalf , Andrew Morton , LKML Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2560 Lines: 64 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 wrote: > On Thu, Mar 10, 2011 at 6:47 PM, Michel Lespinasse 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/