* Dan Williams <dan.j.williams@xxxxxxxxx> 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: > > CONFIG_SPECTRE1_WORKAROUND_INDEX_MASK > > CONFIG_SPECTRE1_WORKAROUND_LOAD_FENCE > > 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. > 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. Thanks, Ingo
