Re: [PATCH 0/7] atomics: generate atomic headers
From: Dmitry Vyukov
Date: Mon Jun 04 2018 - 12:21:50 EST
On Tue, May 29, 2018 at 8:07 PM, Mark Rutland <mark.rutland@xxxxxxx> 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!
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.
> [1] https://lkml.kernel.org/r/20180529154346.3168-1-mark.rutland@xxxxxxx
> [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@xxxxxxxxxxxxxx
>
> Mark Rutland (7):
> atomics/tty: add missing atomic_long_t * cast
> atomics/treewide: rework ordering barriers
> atomics: separate basic {cmp,}xchg()
> atomics: add common header generation files
> atomics: switch to generated fallbacks
> atomics: switch to generated atomic-long
> atomics: switch to generated instrumentation
>
> MAINTAINERS | 1 +
> arch/alpha/include/asm/atomic.h | 8 +-
> arch/powerpc/include/asm/atomic.h | 17 +-
> arch/riscv/include/asm/atomic.h | 17 +-
> drivers/tty/tty_ldsem.c | 2 +-
> include/asm-generic/atomic-instrumented-atomic.h | 1629 ++++++++++++++++
> include/asm-generic/atomic-instrumented.h | 394 +---
> include/asm-generic/atomic-long.h | 1150 +++++++++--
> include/linux/atomic-fallback.h | 2223 ++++++++++++++++++++++
> include/linux/atomic.h | 1244 +-----------
> scripts/atomic/atomic-tbl.sh | 186 ++
> scripts/atomic/atomics.tbl | 41 +
> scripts/atomic/fallbacks/acquire | 9 +
> scripts/atomic/fallbacks/add_negative | 16 +
> scripts/atomic/fallbacks/add_unless | 16 +
> scripts/atomic/fallbacks/andnot | 7 +
> scripts/atomic/fallbacks/dec | 7 +
> scripts/atomic/fallbacks/dec_and_test | 15 +
> scripts/atomic/fallbacks/dec_if_positive | 15 +
> scripts/atomic/fallbacks/dec_unless_positive | 14 +
> scripts/atomic/fallbacks/fence | 11 +
> scripts/atomic/fallbacks/fetch_add_unless | 23 +
> scripts/atomic/fallbacks/inc | 7 +
> scripts/atomic/fallbacks/inc_and_test | 15 +
> scripts/atomic/fallbacks/inc_not_zero | 14 +
> scripts/atomic/fallbacks/inc_unless_negative | 14 +
> scripts/atomic/fallbacks/read_acquire | 7 +
> scripts/atomic/fallbacks/release | 8 +
> scripts/atomic/fallbacks/set_release | 7 +
> scripts/atomic/fallbacks/sub_and_test | 16 +
> scripts/atomic/fallbacks/try_cmpxchg | 11 +
> scripts/atomic/gen-atomic-fallback.sh | 144 ++
> scripts/atomic/gen-atomic-instrumented.sh | 122 ++
> scripts/atomic/gen-atomic-long.sh | 98 +
> 34 files changed, 5671 insertions(+), 1837 deletions(-)
> create mode 100644 include/asm-generic/atomic-instrumented-atomic.h
> create mode 100644 include/linux/atomic-fallback.h
> create mode 100755 scripts/atomic/atomic-tbl.sh
> create mode 100644 scripts/atomic/atomics.tbl
> create mode 100644 scripts/atomic/fallbacks/acquire
> create mode 100644 scripts/atomic/fallbacks/add_negative
> create mode 100644 scripts/atomic/fallbacks/add_unless
> create mode 100644 scripts/atomic/fallbacks/andnot
> create mode 100644 scripts/atomic/fallbacks/dec
> create mode 100644 scripts/atomic/fallbacks/dec_and_test
> create mode 100644 scripts/atomic/fallbacks/dec_if_positive
> create mode 100644 scripts/atomic/fallbacks/dec_unless_positive
> create mode 100644 scripts/atomic/fallbacks/fence
> create mode 100644 scripts/atomic/fallbacks/fetch_add_unless
> create mode 100644 scripts/atomic/fallbacks/inc
> create mode 100644 scripts/atomic/fallbacks/inc_and_test
> create mode 100644 scripts/atomic/fallbacks/inc_not_zero
> create mode 100644 scripts/atomic/fallbacks/inc_unless_negative
> create mode 100644 scripts/atomic/fallbacks/read_acquire
> create mode 100644 scripts/atomic/fallbacks/release
> create mode 100644 scripts/atomic/fallbacks/set_release
> create mode 100644 scripts/atomic/fallbacks/sub_and_test
> create mode 100644 scripts/atomic/fallbacks/try_cmpxchg
> create mode 100755 scripts/atomic/gen-atomic-fallback.sh
> create mode 100755 scripts/atomic/gen-atomic-instrumented.sh
> create mode 100755 scripts/atomic/gen-atomic-long.sh
>
> --
> 2.11.0
>