[PATCH 0/2] locking/atomics: fix and improvement

[PATCH 0/2] locking/atomics: fix and improvement

[Mark Rutland]

Hi Ingo,

I hope that these atomic scripting patches address your concerns with
the atomics scripting infrastructure. These are based on the tip
locking/core branch, leaving the headers checked-in.

The primary improvement is using a hash to detect when the headers have
been erroneously modified, meaning check-atomics.sh takes ~0.15s, which
I believe should be palatable.

It also turns out that some people are building without coreutils, so
I've picked up Anders' fix for this. Since sha1sum is part of coreutils,
the checks are skipped when this isn't present, on the assumption that
anyone actually sending patches will have a working coreutils.

Are you happy to pick these up?

Thanks,
Mark.

Anders Roxell (1):
  locking/atomics: Change 'fold' to 'grep'

Mark Rutland (1):
  locking/atomics: Check atomic headers with sha1sum

 include/asm-generic/atomic-instrumented.h |  1 +
 include/asm-generic/atomic-long.h         |  1 +
 include/linux/atomic-fallback.h           |  1 +
 scripts/atomic/atomic-tbl.sh              |  2 +-
 scripts/atomic/check-atomics.sh           | 26 ++++++++++++++++++++------
 scripts/atomic/gen-atomics.sh             | 20 ++++++++++++++++++++
 6 files changed, 44 insertions(+), 7 deletions(-)
 create mode 100755 scripts/atomic/gen-atomics.sh

-- 
2.11.0

[PATCH 1/2] locking/atomics: Change 'fold' to 'grep'

[Mark Rutland]

From: Anders Roxell <anders.roxell@linaro.org>

Some distibutions and build systems doesn't include 'fold' from
coreutils default.

.../scripts/atomic/atomic-tbl.sh: line 183: fold: command not found

Rework to use 'grep' instead of 'fold' to use a dependency that is
already used a lot in the kernel.

Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Suggested-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
[Mark: rework commit message]
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 scripts/atomic/atomic-tbl.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/atomic/atomic-tbl.sh b/scripts/atomic/atomic-tbl.sh
index 9d6be538a987..81d5c32039dd 100755
--- a/scripts/atomic/atomic-tbl.sh
+++ b/scripts/atomic/atomic-tbl.sh
@@ -180,7 +180,7 @@ gen_proto_variants()
 #gen_proto(meta, ...)
 gen_proto() {
 	local meta="$1"; shift
-	for m in $(echo "${meta}" | fold -w1); do
+	for m in $(echo "${meta}" | grep -o .); do
 		gen_proto_variants "${m}" "$@"
 	done
 }
-- 
2.11.0

Re: [PATCH 1/2] locking/atomics: Change 'fold' to 'grep'

[Will Deacon]

On Mon, Dec 10, 2018 at 05:50:34PM +0000, Mark Rutland wrote:
> From: Anders Roxell <anders.roxell@linaro.org>
> 
> Some distibutions and build systems doesn't include 'fold' from
> coreutils default.
> 
> .../scripts/atomic/atomic-tbl.sh: line 183: fold: command not found
> 
> Rework to use 'grep' instead of 'fold' to use a dependency that is
> already used a lot in the kernel.
> 
> Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> Suggested-by: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> [Mark: rework commit message]
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> ---
>  scripts/atomic/atomic-tbl.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/atomic/atomic-tbl.sh b/scripts/atomic/atomic-tbl.sh
> index 9d6be538a987..81d5c32039dd 100755
> --- a/scripts/atomic/atomic-tbl.sh
> +++ b/scripts/atomic/atomic-tbl.sh
> @@ -180,7 +180,7 @@ gen_proto_variants()
>  #gen_proto(meta, ...)
>  gen_proto() {
>  	local meta="$1"; shift
> -	for m in $(echo "${meta}" | fold -w1); do
> +	for m in $(echo "${meta}" | grep -o .); do
>  		gen_proto_variants "${m}" "$@"
>  	done
>  }

Acked-by: Will Deacon <will.deacon@arm.com>

Will

[PATCH 2/2] locking/atomics: Check atomic headers with sha1sum

[Mark Rutland]

We currently check the atomic headers at build-time to ensure they
haven't been modified directly, and these checks require regenerating
the headers in full. As this takes a few seconds, even when
parallelized, this is too slow to run for every kernel build.

Instead, we can generate a hash of each header as we generate them,
which we can cheaply check at build time (~0.16s for all headers).

This patch does so, updating headers with their hashes using the new
gen-atomics.sh script. As some users apparently build the kernel wihout
coreutils, lacking sha1sum, the checks are skipped in this case.
Presumably, most developers have a working coreutils installation.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will.deacon@arm.com>
---
 include/asm-generic/atomic-instrumented.h |  1 +
 include/asm-generic/atomic-long.h         |  1 +
 include/linux/atomic-fallback.h           |  1 +
 scripts/atomic/check-atomics.sh           | 26 ++++++++++++++++++++------
 scripts/atomic/gen-atomics.sh             | 20 ++++++++++++++++++++
 5 files changed, 43 insertions(+), 6 deletions(-)
 create mode 100755 scripts/atomic/gen-atomics.sh

diff --git a/include/asm-generic/atomic-instrumented.h b/include/asm-generic/atomic-instrumented.h
index b8f5b35216e1..e8730c6b9fe2 100644
--- a/include/asm-generic/atomic-instrumented.h
+++ b/include/asm-generic/atomic-instrumented.h
@@ -1785,3 +1785,4 @@ atomic64_dec_if_positive(atomic64_t *v)
 })
 
 #endif /* _ASM_GENERIC_ATOMIC_INSTRUMENTED_H */
+// b29b625d5de9280f680e42c7be859b55b15e5f6a
diff --git a/include/asm-generic/atomic-long.h b/include/asm-generic/atomic-long.h
index a833d385a70b..881c7e27af28 100644
--- a/include/asm-generic/atomic-long.h
+++ b/include/asm-generic/atomic-long.h
@@ -1010,3 +1010,4 @@ atomic_long_dec_if_positive(atomic_long_t *v)
 
 #endif /* CONFIG_64BIT */
 #endif /* _ASM_GENERIC_ATOMIC_LONG_H */
+// 77558968132ce4f911ad53f6f52ce423006f6268
diff --git a/include/linux/atomic-fallback.h b/include/linux/atomic-fallback.h
index 1c02c0112fbb..a7d240e465c0 100644
--- a/include/linux/atomic-fallback.h
+++ b/include/linux/atomic-fallback.h
@@ -2292,3 +2292,4 @@ atomic64_dec_if_positive(atomic64_t *v)
 #define atomic64_cond_read_relaxed(v, c) smp_cond_load_relaxed(&(v)->counter, (c))
 
 #endif /* _LINUX_ATOMIC_FALLBACK_H */
+// 25de4a2804d70f57e994fe3b419148658bb5378a
diff --git a/scripts/atomic/check-atomics.sh b/scripts/atomic/check-atomics.sh
index c30101cddf2d..cfa0c2f71c84 100755
--- a/scripts/atomic/check-atomics.sh
+++ b/scripts/atomic/check-atomics.sh
@@ -7,13 +7,27 @@ ATOMICDIR=$(dirname $0)
 ATOMICTBL=${ATOMICDIR}/atomics.tbl
 LINUXDIR=${ATOMICDIR}/../..
 
+echo '' | sha1sum - > /dev/null 2>&1
+if [ $? -ne 0 ]; then
+	printf "sha1sum not available, skipping atomic header checks.\n"
+	exit 0
+fi
+
 cat <<EOF |
-gen-atomic-instrumented.sh      asm-generic/atomic-instrumented.h
-gen-atomic-long.sh              asm-generic/atomic-long.h
-gen-atomic-fallback.sh          linux/atomic-fallback.h
+asm-generic/atomic-instrumented.h
+asm-generic/atomic-long.h
+linux/atomic-fallback.h
 EOF
-while read script header; do
-	if ! (${ATOMICDIR}/${script} ${ATOMICTBL} | diff - ${LINUXDIR}/include/${header} > /dev/null); then
-		printf "warning: include/${header} is out-of-date.\n"
+while read header; do
+	OLDSUM="$(tail -n 1 ${LINUXDIR}/include/${header})"
+	OLDSUM="${OLDSUM#// }"
+
+	NEWSUM="$(head -n -1 ${LINUXDIR}/include/${header} | sha1sum)"
+	NEWSUM="${NEWSUM%% *}"
+
+	if [ "${OLDSUM}" != "${NEWSUM}" ]; then
+		printf "warning: generated include/${header} has been modified.\n"
 	fi
 done
+
+exit 0
diff --git a/scripts/atomic/gen-atomics.sh b/scripts/atomic/gen-atomics.sh
new file mode 100755
index 000000000000..27400b0cd732
--- /dev/null
+++ b/scripts/atomic/gen-atomics.sh
@@ -0,0 +1,20 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+#
+# Generate atomic headers
+
+ATOMICDIR=$(dirname $0)
+ATOMICTBL=${ATOMICDIR}/atomics.tbl
+LINUXDIR=${ATOMICDIR}/../..
+
+cat <<EOF |
+gen-atomic-instrumented.sh      asm-generic/atomic-instrumented.h
+gen-atomic-long.sh              asm-generic/atomic-long.h
+gen-atomic-fallback.sh          linux/atomic-fallback.h
+EOF
+while read script header; do
+	${ATOMICDIR}/${script} ${ATOMICTBL} > ${LINUXDIR}/include/${header}
+	HASH="$(sha1sum ${LINUXDIR}/include/${header})"
+	HASH="${HASH%% *}"
+	printf "// %s\n" "${HASH}" >> ${LINUXDIR}/include/${header}
+done
-- 
2.11.0

Re: [PATCH 2/2] locking/atomics: Check atomic headers with sha1sum

[Will Deacon]

On Mon, Dec 10, 2018 at 05:50:35PM +0000, Mark Rutland wrote:
> We currently check the atomic headers at build-time to ensure they
> haven't been modified directly, and these checks require regenerating
> the headers in full. As this takes a few seconds, even when
> parallelized, this is too slow to run for every kernel build.
> 
> Instead, we can generate a hash of each header as we generate them,
> which we can cheaply check at build time (~0.16s for all headers).
> 
> This patch does so, updating headers with their hashes using the new
> gen-atomics.sh script. As some users apparently build the kernel wihout
> coreutils, lacking sha1sum, the checks are skipped in this case.
> Presumably, most developers have a working coreutils installation.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Will Deacon <will.deacon@arm.com>
> ---
>  include/asm-generic/atomic-instrumented.h |  1 +
>  include/asm-generic/atomic-long.h         |  1 +
>  include/linux/atomic-fallback.h           |  1 +
>  scripts/atomic/check-atomics.sh           | 26 ++++++++++++++++++++------
>  scripts/atomic/gen-atomics.sh             | 20 ++++++++++++++++++++
>  5 files changed, 43 insertions(+), 6 deletions(-)
>  create mode 100755 scripts/atomic/gen-atomics.sh
> 
> diff --git a/include/asm-generic/atomic-instrumented.h b/include/asm-generic/atomic-instrumented.h
> index b8f5b35216e1..e8730c6b9fe2 100644
> --- a/include/asm-generic/atomic-instrumented.h
> +++ b/include/asm-generic/atomic-instrumented.h
> @@ -1785,3 +1785,4 @@ atomic64_dec_if_positive(atomic64_t *v)
>  })
>  
>  #endif /* _ASM_GENERIC_ATOMIC_INSTRUMENTED_H */
> +// b29b625d5de9280f680e42c7be859b55b15e5f6a
> diff --git a/include/asm-generic/atomic-long.h b/include/asm-generic/atomic-long.h
> index a833d385a70b..881c7e27af28 100644
> --- a/include/asm-generic/atomic-long.h
> +++ b/include/asm-generic/atomic-long.h
> @@ -1010,3 +1010,4 @@ atomic_long_dec_if_positive(atomic_long_t *v)
>  
>  #endif /* CONFIG_64BIT */
>  #endif /* _ASM_GENERIC_ATOMIC_LONG_H */
> +// 77558968132ce4f911ad53f6f52ce423006f6268
> diff --git a/include/linux/atomic-fallback.h b/include/linux/atomic-fallback.h
> index 1c02c0112fbb..a7d240e465c0 100644
> --- a/include/linux/atomic-fallback.h
> +++ b/include/linux/atomic-fallback.h
> @@ -2292,3 +2292,4 @@ atomic64_dec_if_positive(atomic64_t *v)
>  #define atomic64_cond_read_relaxed(v, c) smp_cond_load_relaxed(&(v)->counter, (c))
>  
>  #endif /* _LINUX_ATOMIC_FALLBACK_H */
> +// 25de4a2804d70f57e994fe3b419148658bb5378a
> diff --git a/scripts/atomic/check-atomics.sh b/scripts/atomic/check-atomics.sh
> index c30101cddf2d..cfa0c2f71c84 100755
> --- a/scripts/atomic/check-atomics.sh
> +++ b/scripts/atomic/check-atomics.sh
> @@ -7,13 +7,27 @@ ATOMICDIR=$(dirname $0)
>  ATOMICTBL=${ATOMICDIR}/atomics.tbl
>  LINUXDIR=${ATOMICDIR}/../..
>  
> +echo '' | sha1sum - > /dev/null 2>&1
> +if [ $? -ne 0 ]; then
> +	printf "sha1sum not available, skipping atomic header checks.\n"
> +	exit 0
> +fi
> +
>  cat <<EOF |
> -gen-atomic-instrumented.sh      asm-generic/atomic-instrumented.h
> -gen-atomic-long.sh              asm-generic/atomic-long.h
> -gen-atomic-fallback.sh          linux/atomic-fallback.h
> +asm-generic/atomic-instrumented.h
> +asm-generic/atomic-long.h
> +linux/atomic-fallback.h
>  EOF
> -while read script header; do
> -	if ! (${ATOMICDIR}/${script} ${ATOMICTBL} | diff - ${LINUXDIR}/include/${header} > /dev/null); then
> -		printf "warning: include/${header} is out-of-date.\n"
> +while read header; do
> +	OLDSUM="$(tail -n 1 ${LINUXDIR}/include/${header})"
> +	OLDSUM="${OLDSUM#// }"
> +
> +	NEWSUM="$(head -n -1 ${LINUXDIR}/include/${header} | sha1sum)"
> +	NEWSUM="${NEWSUM%% *}"

Here we're relying on head and tail being available, but it looks like
they're also part of coreutils, so that should be fine.

> +
> +	if [ "${OLDSUM}" != "${NEWSUM}" ]; then
> +		printf "warning: generated include/${header} has been modified.\n"
>  	fi
>  done
> +
> +exit 0
> diff --git a/scripts/atomic/gen-atomics.sh b/scripts/atomic/gen-atomics.sh
> new file mode 100755
> index 000000000000..27400b0cd732
> --- /dev/null
> +++ b/scripts/atomic/gen-atomics.sh
> @@ -0,0 +1,20 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Generate atomic headers
> +
> +ATOMICDIR=$(dirname $0)
> +ATOMICTBL=${ATOMICDIR}/atomics.tbl
> +LINUXDIR=${ATOMICDIR}/../..
> +
> +cat <<EOF |
> +gen-atomic-instrumented.sh      asm-generic/atomic-instrumented.h
> +gen-atomic-long.sh              asm-generic/atomic-long.h
> +gen-atomic-fallback.sh          linux/atomic-fallback.h
> +EOF
> +while read script header; do
> +	${ATOMICDIR}/${script} ${ATOMICTBL} > ${LINUXDIR}/include/${header}
> +	HASH="$(sha1sum ${LINUXDIR}/include/${header})"
> +	HASH="${HASH%% *}"
> +	printf "// %s\n" "${HASH}" >> ${LINUXDIR}/include/${header}
> +done

Thanks, this looks sensible to me:

Acked-by: Will Deacon <will.deacon@arm.com>

Will

Re: [PATCH 0/2] locking/atomics: fix and improvement

[Mark Rutland]

On Mon, Dec 10, 2018 at 05:50:33PM +0000, Mark Rutland wrote:
> Hi Ingo,
> 
> I hope that these atomic scripting patches address your concerns with
> the atomics scripting infrastructure. These are based on the tip
> locking/core branch, leaving the headers checked-in.
> 
> The primary improvement is using a hash to detect when the headers have
> been erroneously modified, meaning check-atomics.sh takes ~0.15s, which
> I believe should be palatable.

As a heads-up, it looks like my prior attempt [1] is in today's linux-next
(next-20181212), via Andrew's tree.

Ingo, could you please let us know what you'd prefer? Are you happy to take
this version, and should Andrew drop that patch?

Thanks,
Mark.

[1] https://lkml.kernel.org/r/20181128085455.1164-1-mark.rutland@arm.com