Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755667AbZAIVfj (ORCPT ); Fri, 9 Jan 2009 16:35:39 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754212AbZAIVfQ (ORCPT ); Fri, 9 Jan 2009 16:35:16 -0500 Received: from mx2.mail.elte.hu ([157.181.151.9]:35665 "EHLO mx2.mail.elte.hu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758189AbZAIVfM (ORCPT ); Fri, 9 Jan 2009 16:35:12 -0500 Date: Fri, 9 Jan 2009 22:34:42 +0100 From: Ingo Molnar To: Linus Torvalds Cc: "H. Peter Anvin" , Andi Kleen , Chris Mason , Peter Zijlstra , Steven Rostedt , paulmck@linux.vnet.ibm.com, Gregory Haskins , Matthew Wilcox , Andrew Morton , Linux Kernel Mailing List , linux-fsdevel , linux-btrfs , Thomas Gleixner , Nick Piggin , Peter Morreale , Sven Dietrich Subject: Re: [PATCH -v7][RFC]: mutex: implement adaptive spinning Message-ID: <20090109213442.GA20051@elte.hu> References: <1231434515.14304.27.camel@think.oraclecorp.com> <20090108183306.GA22916@elte.hu> <20090108190038.GH496@one.firstfloor.org> <4966AB74.2090104@zytor.com> <20090109133710.GB31845@elte.hu> <20090109204103.GA17212@elte.hu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.18 (2008-05-17) X-ELTE-VirusStatus: clean X-ELTE-SpamScore: -1.5 X-ELTE-SpamLevel: X-ELTE-SpamCheck: no X-ELTE-SpamVersion: ELTE 2.0 X-ELTE-SpamCheck-Details: score=-1.5 required=5.9 tests=BAYES_00 autolearn=no SpamAssassin version=3.2.3 -1.5 BAYES_00 BODY: Bayesian spam probability is 0 to 1% [score: 0.0000] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4008 Lines: 89 * Linus Torvalds wrote: > On Fri, 9 Jan 2009, Ingo Molnar wrote: > > > > So, should we not remove CONFIG_OPTIMIZE_INLINING, then the correct > > one would be to mark it __always_inline [__asm_inline is senseless > > there], or the second patch below that changes the bit parameter to > > unsigned int. > > Well, I certainly don't want to _remove_ the "inline" like your patch > did. Other gcc versions will care. But I committed the pure "change to > unsigned" part. > > But we should fix the cmpxchg (and perhaps plain xchg too), shouldn't > we? > > That your gcc version gets it right doesn't change the fact that Chris' > gcc version didn't, and out-of-lined it all. So we'll need some > __always_inlines there too.. Yeah. I'll dig out an older version of gcc (latest distros are all 4.3.x based) and run the checks to see which inlines make a difference. > And no, I don't think it makes any sense to call them "__asm_inline". > Even when there are asms hidden in between the C statements, what's the > difference between "always" and "asm"? None, really. Well, the difference is small, nitpicky and insignificant: the thing is there are two logically separate categories of __always_inline: 1) the places where __always_inline means that in this universe no sane compiler ever can end up thinking to move that function out of line. 2) inlining for_correctness_ reasons: things like vreads or certain paravirt items. Stuff where the kernel actually crashes if we dont inline. Here if we do not inline we've got a materially crashy kernel. The original intention of __always_inline was to only cover the second category above - and thus self-document all the 'correctness inlines'. This notion has become bitrotten somewhat: we do use __always_inline in a few other places like the ticket spinlock inlines for non-correctness reasons. That bitrot happened because we simply have no separate symbol for the first category. So hpa suggested __asm_inline (yesterday, well before all the analysis was conducted) under the assumption that there would be many such annotations needed and that they would be all about cases where GCC's inliner gets confused by inline assembly. This theory turned out to be a red herring today - asm()s do not seem to confuse latest GCC. (although they certain confuse earlier versions, so it's still a practical issue, so i agree that we do need to annotate a few more places.) In any case, the __asm_inline name - even if it made some marginal sense originally - is totally moot now, no argument about that. The naming problem remains though: - Perhaps we could introduce a name for the first category: __must_inline? __should_inline? Not because it wouldnt mean 'always', but because it is 'always inline' for another reason than the correctless __always_inline. - Another possible approach wuld be to rename the second category to __force_inline. That would signal it rather forcefully that the inlining there is an absolute correctness issue. - Or we could go with the status quo and just conflate those two categories (as it is happening currently) and document the correctness inlines via in-source comments? But these are really nuances that pale in comparison to the fundamental questions that were asked in this thread, about the pure existence of this feature. If the optimize-inlining feature looks worthwile and maintainable to remain upstream then i'd simply like to see the information of these two categories preserved in a structured way (in 5 years i'm not sure i'd remember all the paravirt inlining details), and i dont feel too strongly about the style how we preserve that information. Ingo -- 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/