Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 29 Jan 2018 08:45:38 -0800
From: Dan Williams <>
To: Ingo Molnar <>
Cc: Thomas Gleixner <>, linux-arch <>, 
	Cyril Novikov <>, Kernel Hardening <>, 
	Peter Zijlstra <>, Catalin Marinas <>, X86 ML <>, 
	Will Deacon <>, Russell King <>, 
	Ingo Molnar <>, Greg KH <>, 
	"H. Peter Anvin" <>, Linus Torvalds <>, 
	Alan Cox <>, 
	Linux Kernel Mailing List <>
Subject: Re: [PATCH v5 02/12] array_idx: sanitize speculative array de-references

On Sun, Jan 28, 2018 at 10:33 AM, Ingo Molnar <> wrote:
> * Dan Williams <> wrote:
>> Thomas, Peter, and Alexei wanted s/nospec_barrier/ifence/ and
> I just checked past discussions, and I cannot find that part, got any links or
> message-IDs?
> PeterZ's feedback on Jan 8 was:
>> On Sun, Jan 07, 2018 at 06:24:11PM -0800, Alexei Starovoitov wrote:
>> > How about:
>> INSTRUCTION_FENCE if anything. LFENCE for Intel (and now also for AMD as
>> per 0592b0bce169) is a misnomer, IFENCE would be a better name for it.
> Which in that context clearly talked about the config space and how to name the
> instruction semantics in light of the confusion of LFENCE and MFENCE opcodes on
> Intel and AMD CPUs...
> Also, those early reviews were fundamentally low level feedback related to the
> actual functionality of the barriers and their mapping to the hardware.
> But the fact is, the current series introduces an inconsistent barrier namespace
> extension of:
>         barrier()
>         barrier_data()
>         mb()
>         rmb()
>         wmb()
>         store_mb()
>         read_barrier_depends()
>         ...
> +       ifence()
> +       array_idx()
> +       array_idx_mask()
> This isn't bikeshed painting: _ALL_ existing barrier API names have 'barrier' or
> its abbreviation 'mb' (memory barrier) somewhere in their names, which makes it
> pretty easy to recognize them at a glance.
> I'm giving you high level API naming feedback because we are now growing the size
> of the barrier API.
> array_idx() on the other hand is pretty much close to a 'worst possible' name:
>  - it's named in an overly generic, opaque fashion
>  - doesn't indicate it at all that it's a barrier for something
> ... and since we want all kernel developers to use these facilities correctly, we
> want the names to be good and suggestive as well.
> I'd accept pretty much anything else that adds 'barrier' or 'nospec' to the name:
> array_idx_barrier() or array_idx_nospec(). (I'm slightly leaning towards 'nospec'
> because it's more indicative of what is being done, and it also is what we do for
> get uaccess APIs.)
> ifence() is a similar departure from existing barrier naming nomenclature, and I'd
> accept pretty much any other variant:
>         barrier_nospec()
>         ifence_nospec()
> The kernel namespace cleanliness rules are clear and consistent, and there's
> nothing new about them:
>  - the name of the API should unambiguously refer back to the API category. For
>    barriers this common reference is 'barrier' or 'mb'.
>  - use postfixes or prefixes consistently: pick one and don't mix them. If we go
>    with a _nospec() variant for the uaccess API names then we should use a similar
>    naming for array_idx() and for the new barrier as well - no mixing.

This is the feedback I can take action with, thank you.

>> You can always follow on with a patch to fix up the names and placements to your
>> liking. While they'll pick on my name choices, they won't pick on yours, because
>> I simply can't be bothered to care about a bikeshed color at this point after
>> being bounced around for 5 revisions of this patch set.
> Sorry, this kind of dismissive and condescending attitude won't cut it.

I reacted to your "for heaven's sake", I'll send a v6.

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.