From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx189.postini.com [74.125.245.189]) by kanga.kvack.org (Postfix) with SMTP id 4BABF6B005A for ; Fri, 21 Dec 2012 11:53:55 -0500 (EST) Received: by mail-we0-f182.google.com with SMTP id u54so2264297wey.41 for ; Fri, 21 Dec 2012 08:53:53 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <20121221134740.GC13367@suse.de> References: <1353624594-1118-1-git-send-email-mingo@kernel.org> <1353624594-1118-19-git-send-email-mingo@kernel.org> <20121221134740.GC13367@suse.de> From: Linus Torvalds Date: Fri, 21 Dec 2012 08:53:33 -0800 Message-ID: Subject: Re: [patch] mm, mempolicy: Introduce spinlock to read shared policy tree Content-Type: text/plain; charset=ISO-8859-1 Sender: owner-linux-mm@kvack.org List-ID: To: Mel Gorman Cc: David Rientjes , Ingo Molnar , Linux Kernel Mailing List , linux-mm , Peter Zijlstra , Paul Turner , Lee Schermerhorn , Christoph Lameter , Rik van Riel , Andrew Morton , Andrea Arcangeli , Thomas Gleixner , Johannes Weiner , Hugh Dickins , Sasha Levin , KOSAKI Motohiro On Fri, Dec 21, 2012 at 5:47 AM, Mel Gorman wrote: > On Thu, Dec 20, 2012 at 02:55:22PM -0800, David Rientjes wrote: >> >> This is probably worth discussing now to see if we can't revert >> b22d127a39dd ("mempolicy: fix a race in shared_policy_replace()"), keep it >> only as a spinlock as you suggest, and do what KOSAKI suggested in >> http://marc.info/?l=linux-kernel&m=133940650731255 instead. I don't think >> it's worth trying to optimize this path at the cost of having both a >> spinlock and mutex. > > Jeez, I'm still not keen on that approach for the reasons that are explained > in the changelog for b22d127a39dd. Christ, Mel. Your reasons in b22d127a39dd are weak as hell, and then you come up with *THIS* shit instead: > That leads to this third *ugly* option that conditionally drops the lock > and it's up to the caller to figure out what happened. Fooling around with > how it conditionally releases the lock results in different sorts of ugly. > We now have three ugly sister patches for this. Who wants to be Cinderalla? > > ---8<--- > mm: numa: Release the PTL if calling vm_ops->get_policy during NUMA hinting faults Heck no. In fact, not a f*cking way in hell. Look yourself in the mirror, Mel. This patch is ugly, and *guaranteed* to result in subtle locking issues, and then you have the *gall* to quote the "uhh, that's a bit ugly due to some trivial duplication" thing in commit b22d127a39dd. Reverting commit b22d127a39dd and just having a "ok, if we need to allocate, then drop the lock, allocate, re-get the lock, and see if we still need the new allocation" is *beautiful* code compared to the diseased abortion you just posted. Seriously. Conditional locking is error-prone, and about a million times worse than the trivial fix that Kosaki suggested. Linus -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org