Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 27 Oct 2016 12:08:43 +0100
From: Mark Rutland <mark.rutland@....com>
To: kernel-hardening@...ts.openwall.com
Cc: AKASHI Takahiro <takahiro.akashi@...aro.org>,
	"Reshetova, Elena" <elena.reshetova@...el.com>,
	David Windsor <dave@...gbits.org>,
	Kees Cook <keescook@...omium.org>,
	Hans Liljestrand <ishkamiel@...il.com>
Subject: Re: Re: [RFC 2/2] arm: implementation for
 HARDENED_ATOMIC

On Wed, Oct 26, 2016 at 10:20:16AM +0200, Colin Vidal wrote:
> > My point is that _wrap would be the last suffix of the names of all
> > the functions including _relaxed variants for consistency.
> > 
> > Say, Elena's atomic-long.h defines:
> > ===8<===
> > #define ATOMIC_LONG_ADD_SUB_OP(op, mo, suffix)                          \
> > static inline long                                                      \
> > atomic_long_##op##_return##mo##suffix(long i, atomic_long##suffix##_t *l)\
> > {                                                                       \
> >         ATOMIC_LONG_PFX(suffix##_t) *v = (ATOMIC_LONG_PFX(suffix##_t) *)l;\
> >                                                                         \
> >         return (long)ATOMIC_LONG_PFX(_##op##_return##mo##suffix)(i, v);\
> > }
> > ATOMIC_LONG_ADD_SUB_OP(add,,)
> > ATOMIC_LONG_ADD_SUB_OP(add, _relaxed,)
> >   ...
> > #ifdef CONFIG_HARDENED_ATOMIC
> > ATOMIC_LONG_ADD_SUB_OP(add,,_wrap)
> >   ...
> > ===>8===
> > 
> > It would naturally lead to "atomic_long_add_relaxed_wrap" although
> > this function is not actually defined here.
> > 
> 
> I understand your point, and the need of consistency. "_wrap" is
> appended to any "user" function of the atomic system, and it is
> mandatory to have a consistant name for it, I fully agree.
> 
> However, in the internal implementation of atomic, "_relaxed" is
> appended to any function that will be bounded by memory barriers.
> Therefore, a name like "atomic_add_return_wrap_relaxed" makes it easy
> to see that this function will be embedded in another one. On the other
> hand "atomic_add_return_relaxed_wrap" is less clear, since it suggest
> that is a "user" function. I would involve a kind on inconsistency on
> the naming of relaxed functions.
> 
> That said, I really don't know which is the "best" naming... :-)

Given it looks like there's at least one necessary round of bikeshedding, it
would be best to discuss this with the usual atomic maintainers included, so as
to avoid several more rounds over subsequent postings. i.e.

[mark@...erpostej:~/src/linux]% ./scripts/get_maintainer.pl -f \
	include/asm-generic/atomic.h \
	include/asm-generic/atomic-long.h \
	include/linux/atomic.h
Arnd Bergmann <arnd@...db.de> (maintainer:GENERIC INCLUDE/ASM HEADER FILES)
Ingo Molnar <mingo@...nel.org> (commit_signer:8/11=73%)
Peter Zijlstra <peterz@...radead.org> (commit_signer:6/11=55%,authored:5/11=45%,added_lines:491/671=73%,removed_lines:186/225=83%)
Frederic Weisbecker <fweisbec@...il.com> (commit_signer:3/11=27%,authored:3/11=27%,added_lines:42/671=6%,removed_lines:21/225=9%)
Boqun Feng <boqun.feng@...il.com> (commit_signer:1/11=9%,authored:1/11=9%)
Chris Metcalf <cmetcalf@...hip.com> (commit_signer:1/11=9%)
Davidlohr Bueso <dave@...olabs.net> (authored:1/11=9%,added_lines:128/671=19%)
linux-arch@...r.kernel.org (open list:GENERIC INCLUDE/ASM HEADER FILES)
linux-kernel@...r.kernel.org (open list)

Thanks,
Mark.

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.