Re: [PATCH 0/7] atomics: generate atomic headers

[Mark Rutland <mark.rutland@arm.com>]

On Mon, Jun 04, 2018 at 06:21:22PM +0200, Dmitry Vyukov wrote:
> On Tue, May 29, 2018 at 8:07 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> > This series scripts the generation of the various atomic headers, to
> > ensure that the various atomic APIs remain consistent, reducing the risk
> > of human error, and simplifying future rework.
> >
> > The series is based on my atomic API cleanup patches [1,2], and can be
> > found on my atomics/generated branch [3] on kernel.org.
> >
> > The first three patches are preparatory rework, with patch four
> > introducing the infrastructure. The final three patches migrate to
> > generated headers. The scripts themselves are mostly POSIX sh (modulo
> > local), without bashisms, and work in dash and bash.
> >
> > Per Linus request that it is possible to use git grep to inspect the
> > atomic headers [3], the headers are committed (and not generated by
> > kbuild). Since we now expand the fallback definitions inline, each
> > *should* be easier to find with grep. Each header also has a comment at
> > the top with a path to the script used to generate it.
> >
> > While the diff stat looks like a huge addition, the scripting comes in
> > at ~800 lines in total, including the fallback definitions, so we're
> > removing ~1000 lines of hand-written code. At the same time, we fill in
> > gaps in the atomic_long API, and the instrumented atomic wrappers.
> >
> > Longer-term, I think things could be simplified if we were to rework the
> > headers such that we have:
> >
> > * arch/*/include/asm/atomic.h providing arch_atomic_*().
> >
> > * include/linux/atomic-raw.h building raw_atomic_*() atop of the
> >   arch_atomic_*() definitions, filling in gaps in the API. Having
> >   separate arch_ and raw_ namespaces would simplify the ifdeffery.
> >
> > * include/linux/atomic.h building atomic_*() atop of the raw_atomic_*()
> >   definitions, complete with any instrumentation. Instrumenting at this
> >   level would lower the instrumentation overhead, and would not require
> >   any ifdeffery as the whole raw_atomic_*() API would be available.
> >
> > ... I've avoided this for the time being due to the necessary churn in
> > arch code.
> >
> > Thanks,
> > Mark.
> 
> Wow! Nice work!

Thanks!

> I can say that besides KASAN, we will need hooking into atomics for
> KMSAN and KTSAN in future. Without this it would be lots of manual
> work with possibility of copy-paste errors.

Indeed!

This was largely driven by wanting the arm64 atomics instrumented, and
generating that does end up easier to get right than manually expanding all the
acquire/release/relaxed ifdeffery manually.

First we need to get the preparatory patches [1,2] merged. Those have had a few
fixups since they were last posted, so I'll send an updated version come
v4.18-rc1 unless the atomics maintainers really want to queue those ASAP.

There'll be some additional prep work for instrumentation on arm64, too (e.g.
fiddling with the (cmp)xchg instrumentation), so I'll look into that in the
mean time.

Thanks,
Mark.

> > [1] https://lkml.kernel.org/r/20180529154346.3168-1-mark.rutland@arm.com
> > [2] git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git atomics/api-cleanup
> > [3] git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git atomics/generated
> > [4] https://lkml.kernel.org/r/CA+55aFxjU0op8QLLu0n-RjHBs7gQsLvD8jcyedgw6jeZFN7B+Q@mail.gmail.com