Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757276AbbHDOwM (ORCPT ); Tue, 4 Aug 2015 10:52:12 -0400 Received: from mail-lb0-f177.google.com ([209.85.217.177]:36138 "EHLO mail-lb0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755885AbbHDOwK (ORCPT ); Tue, 4 Aug 2015 10:52:10 -0400 MIME-Version: 1.0 In-Reply-To: <20150804143344.GA5689@nazgul.tnic> References: <20150728132313.164884020@infradead.org> <55B87E7A.2070509@suse.cz> <20150729084906.GH19282@twins.programming.kicks-ass.net> <20150803150359.0e76b576@gandalf.local.home> <20150803191816.GC25159@twins.programming.kicks-ass.net> <20150803152810.5a7bcf06@gandalf.local.home> <20150803200002.GE25159@twins.programming.kicks-ass.net> <20150803175757.2adf3275@gandalf.local.home> <20150804033733.GB31787@nazgul.tnic> <20150804080645.0f29617d@gandalf.local.home> <20150804143344.GA5689@nazgul.tnic> From: Andy Lutomirski Date: Tue, 4 Aug 2015 07:51:49 -0700 Message-ID: Subject: Re: [PATCH -v2 6/8] jump_label: Add a new static_key interface To: Borislav Petkov Cc: Steven Rostedt , Peter Zijlstra , Vlastimil Babka , "linux-kernel@vger.kernel.org" , Ingo Molnar , Jason Baron , Thomas Gleixner , Will Deacon , Jiang Liu , rabin@rab.in, Ralf Baechle , David Daney , Benjamin Herrenschmidt , Michael Ellerman , Heiko Carstens , "David S. Miller" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1877 Lines: 65 On Tue, Aug 4, 2015 at 7:33 AM, Borislav Petkov wrote: > On Tue, Aug 04, 2015 at 08:06:45AM -0400, Steven Rostedt wrote: >> I just don't like the inconsistency of the initialization and the >> setting. >> >> Either have: >> >> DEFINE_STATIC_KEY_TRUE() >> DEFINE_STATIC_KEY_FALSE() >> >> and >> >> static_branch_set_true() >> static_branch_set_false() >> >> >> or have: >> >> DEFINE_STATIC_KEY_ENABLED() >> DEFINE_STATIC_KEY_DISABLED() >> >> and >> >> static_branch_enable() >> static_branch_disable() >> >> >> But having the DEFINE_STATIC_KEY_TRUE() and static_branch_enable() is >> confusing, as enable does not mean "make true"! >> >> This may seem as bike shedding, but terminology *is* important, and >> being inconsistent just makes it more probable to have bugs. > > I absolutely agree but I read "enable" as enable the branch, so no > confusion there. Now, it's a whole another question where we branch to. > And that can be confusing. > > Now, let's get back to our example: > > +static DEFINE_STATIC_KEY_FALSE(__use_tsc); > > We don't use the TSC by default. And that's correct, we need to > calibrate it first. > > After calibration: > > + static_branch_enable(&__use_tsc); > > Now here we can get confused: we enable the branch but where we branch > to? The key name helps here but it is still not quite 100% clear. I'd > prefer to have: > > static_enable(&__use_tsc); If everything's consistent about "static_key", then I still like "static_key_set_true" or "static_key_set". "static_key_enable" is okay but not fantastic IMO, and "static_branch_enable" is just confusing. --Andy -- 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/