lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 20 Jun 2019 08:23:25 +0100
From:   Kieran Bingham <kieran.bingham@...asonboard.com>
To:     Ethan Sommer <e5ten.arch@...il.com>
Cc:     linux-kernel@...r.kernel.org,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Ingo Molnar <mingo@...nel.org>,
        Heiko Carstens <heiko.carstens@...ibm.com>,
        Borislav Petkov <bp@...e.de>,
        Mark Rutland <mark.rutland@....com>,
        Kees Cook <keescook@...omium.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Corey Minyard <cminyard@...sta.com>,
        John Stultz <john.stultz@...aro.org>
Subject: Re: [PATCH] replace timeconst bc script with an sh script

Hi Ethan,

Thank you for the patch,

On 20/06/2019 07:22, Ethan Sommer wrote:
> removes the bc build dependency introduced when timeconst.pl was
> replaced by timeconst.bc

Does this introduction of bc cause you problems when building?

Documentation/process/changes.rst states that "You will need bc to build
kernels 3.10 and higher"

Is bc used elsewhere in the kernel?

If this is the only use of BC and it is no longer a dependency - the
process document should be updated.


Though I see uses at:
   tools/testing/selftests/net/forwarding/lib.sh


There is a small issue with quotes highlighted below, and the only other
(really minor) comment is performance.

time (echo 1000 | bc -q ./kernel/time/timeconst.bc)
  real	0m0.006s
  user	0m0.006s
  sys	0m0.000s

vs
time /tmp/timeconst.sh 1000
  real	0m0.176s
  user	0m0.141s
  sys	0m0.050s


So that's 176 milliseconds vs 6. (on an i7 gen8 laptop) which probably
isn't going to affect things too much on the scale of building a kernel.
But I measured it so I thought it was worth posting the results.


--
Regards

Kieran


> Signed-off-by: Ethan Sommer <e5ten.arch@...il.com>
> ---
>  Kbuild                   |   4 +-
>  kernel/time/timeconst.bc | 117 --------------------------------------
>  kernel/time/timeconst.sh | 118 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 120 insertions(+), 119 deletions(-)
>  delete mode 100644 kernel/time/timeconst.bc
>  create mode 100755 kernel/time/timeconst.sh
> 
> diff --git a/Kbuild b/Kbuild
> index 8637fd14135f..2b5f2957cf04 100644
> --- a/Kbuild
> +++ b/Kbuild
> @@ -20,9 +20,9 @@ timeconst-file := include/generated/timeconst.h
>  
>  targets += $(timeconst-file)
>  
> -filechk_gentimeconst = echo $(CONFIG_HZ) | bc -q $<
> +filechk_gentimeconst = $(CONFIG_SHELL) $< $(CONFIG_HZ)
>  
> -$(timeconst-file): kernel/time/timeconst.bc FORCE
> +$(timeconst-file): kernel/time/timeconst.sh FORCE
>  	$(call filechk,gentimeconst)
>  
>  #####
> diff --git a/kernel/time/timeconst.bc b/kernel/time/timeconst.bc
> deleted file mode 100644
> index 7ed0e0fb5831..000000000000
> --- a/kernel/time/timeconst.bc
> +++ /dev/null
> @@ -1,117 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -
> -scale=0
> -
> -define gcd(a,b) {
> -	auto t;
> -	while (b) {
> -		t = b;
> -		b = a % b;
> -		a = t;
> -	}
> -	return a;
> -}
> -
> -/* Division by reciprocal multiplication. */
> -define fmul(b,n,d) {
> -       return (2^b*n+d-1)/d;
> -}
> -
> -/* Adjustment factor when a ceiling value is used.  Use as:
> -   (imul * n) + (fmulxx * n + fadjxx) >> xx) */
> -define fadj(b,n,d) {
> -	auto v;
> -	d = d/gcd(n,d);
> -	v = 2^b*(d-1)/d;
> -	return v;
> -}
> -
> -/* Compute the appropriate mul/adj values as well as a shift count,
> -   which brings the mul value into the range 2^b-1 <= x < 2^b.  Such
> -   a shift value will be correct in the signed integer range and off
> -   by at most one in the upper half of the unsigned range. */
> -define fmuls(b,n,d) {
> -	auto s, m;
> -	for (s = 0; 1; s++) {
> -		m = fmul(s,n,d);
> -		if (m >= 2^(b-1))
> -			return s;
> -	}
> -	return 0;
> -}
> -
> -define timeconst(hz) {
> -	print "/* Automatically generated by kernel/time/timeconst.bc */\n"
> -	print "/* Time conversion constants for HZ == ", hz, " */\n"
> -	print "\n"
> -
> -	print "#ifndef KERNEL_TIMECONST_H\n"
> -	print "#define KERNEL_TIMECONST_H\n\n"
> -
> -	print "#include <linux/param.h>\n"
> -	print "#include <linux/types.h>\n\n"
> -
> -	print "#if HZ != ", hz, "\n"
> -	print "#error \qinclude/generated/timeconst.h has the wrong HZ value!\q\n"
> -	print "#endif\n\n"
> -
> -	if (hz < 2) {
> -		print "#error Totally bogus HZ value!\n"
> -	} else {
> -		s=fmuls(32,1000,hz)
> -		obase=16
> -		print "#define HZ_TO_MSEC_MUL32\tU64_C(0x", fmul(s,1000,hz), ")\n"
> -		print "#define HZ_TO_MSEC_ADJ32\tU64_C(0x", fadj(s,1000,hz), ")\n"
> -		obase=10
> -		print "#define HZ_TO_MSEC_SHR32\t", s, "\n"
> -
> -		s=fmuls(32,hz,1000)
> -		obase=16
> -		print "#define MSEC_TO_HZ_MUL32\tU64_C(0x", fmul(s,hz,1000), ")\n"
> -		print "#define MSEC_TO_HZ_ADJ32\tU64_C(0x", fadj(s,hz,1000), ")\n"
> -		obase=10
> -		print "#define MSEC_TO_HZ_SHR32\t", s, "\n"
> -
> -		obase=10
> -		cd=gcd(hz,1000)
> -		print "#define HZ_TO_MSEC_NUM\t\t", 1000/cd, "\n"
> -		print "#define HZ_TO_MSEC_DEN\t\t", hz/cd, "\n"
> -		print "#define MSEC_TO_HZ_NUM\t\t", hz/cd, "\n"
> -		print "#define MSEC_TO_HZ_DEN\t\t", 1000/cd, "\n"
> -		print "\n"
> -
> -		s=fmuls(32,1000000,hz)
> -		obase=16
> -		print "#define HZ_TO_USEC_MUL32\tU64_C(0x", fmul(s,1000000,hz), ")\n"
> -		print "#define HZ_TO_USEC_ADJ32\tU64_C(0x", fadj(s,1000000,hz), ")\n"
> -		obase=10
> -		print "#define HZ_TO_USEC_SHR32\t", s, "\n"
> -
> -		s=fmuls(32,hz,1000000)
> -		obase=16
> -		print "#define USEC_TO_HZ_MUL32\tU64_C(0x", fmul(s,hz,1000000), ")\n"
> -		print "#define USEC_TO_HZ_ADJ32\tU64_C(0x", fadj(s,hz,1000000), ")\n"
> -		obase=10
> -		print "#define USEC_TO_HZ_SHR32\t", s, "\n"
> -
> -		obase=10
> -		cd=gcd(hz,1000000)
> -		print "#define HZ_TO_USEC_NUM\t\t", 1000000/cd, "\n"
> -		print "#define HZ_TO_USEC_DEN\t\t", hz/cd, "\n"
> -		print "#define USEC_TO_HZ_NUM\t\t", hz/cd, "\n"
> -		print "#define USEC_TO_HZ_DEN\t\t", 1000000/cd, "\n"
> -
> -		cd=gcd(hz,1000000000)
> -		print "#define HZ_TO_NSEC_NUM\t\t", 1000000000/cd, "\n"
> -		print "#define HZ_TO_NSEC_DEN\t\t", hz/cd, "\n"
> -		print "#define NSEC_TO_HZ_NUM\t\t", hz/cd, "\n"
> -		print "#define NSEC_TO_HZ_DEN\t\t", 1000000000/cd, "\n"
> -		print "\n"
> -
> -		print "#endif /* KERNEL_TIMECONST_H */\n"
> -	}
> -	halt
> -}
> -
> -hz = read();
> -timeconst(hz)
> diff --git a/kernel/time/timeconst.sh b/kernel/time/timeconst.sh
> new file mode 100755
> index 000000000000..df821988acbf
> --- /dev/null
> +++ b/kernel/time/timeconst.sh
> @@ -0,0 +1,118 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +
> +if [ -z "$1" ]; then
> +	printf '%s <HZ>\n' "$0"
> +	exit 1
> +else
> +	hz="$1"
> +fi
> +
> +# 2 to the power of n
> +pot() {
> +	local i=1
> +    local j=1
> +	while [ "${j}" -le "$1" ]; do
> +		i="$((i * 2))"
> +        j="$((j + 1))"
> +	done
> +	printf '%s\n' "${i}"
> +}
> +
> +# Greatest common denominator
> +gcd() {
> +	local i="$1"
> +	local j="$2"
> +	local k
> +	while [ "${j}" -ne 0 ]; do
> +		k="${j}"
> +		j="$((i % j))"
> +		i="${k}"
> +	done
> +	printf '%s\n' "${i}"
> +}
> +
> +# Division by reciprocal multiplication.
> +fmul() {
> +	printf '%s\n' "$((($(pot "$1") * $2 + $3 - 1) / $3))"
> +}
> +
> +# Adjustment factor when a ceiling value is used.
> +fadj() {
> +	local i="$(gcd "$2" "$3")"
> +	printf '%s\n' "$(($(pot "$1") * ($3 / i - 1) / ($3 / i)))"
> +}
> +
> +# Compute the appropriate mul/adj values as well as a shift count,
> +# which brings the mul value into the range 2^b-1 <= x < 2^b.  Such
> +# a shift value will be correct in the signed integer range and off
> +# by at most one in the upper half of the unsigned range.
> +fmuls() {
> +	local i=0
> +    local j
> +	while true; do
> +        j="$(fmul "${i}" "$2" "$3")"
> +		if [ "${j}" -ge "$(pot "$(($1 - 1))")" ]; then
> +			printf '%s\n' "${i}" && return
> +		fi
> +		i="$((i + 1))"
> +	done
> +}
> +
> +printf '/* Automatically generated by %s */\n' "$0"
> +printf '/* Time conversion constants for HZ == %s */\n\n' "$1"
> +
> +printf '#ifndef KERNEL_TIMECONST_H\n'
> +printf '#define KERNEL_TIMECONST_H\n\n'
> +
> +printf '#include <linux/param.h>\n'
> +printf '#include <linux/types.h>\n\n'
> +
> +printf '#if HZ != %s\n' "$1"
> +printf '#error \qinclude/generated/timeconst.h has the wrong HZ value!\q\n'

This generates an incorrect output:

diff /tmp/bc.timed /tmp/sh.timed
1c1
< /* Automatically generated by kernel/time/timeconst.bc */
---
> /* Automatically generated by /tmp/timeconst.sh */
11c11
< #error "include/generated/timeconst.h has the wrong HZ value!"
---
> #error \qinclude/generated/timeconst.h has the wrong HZ value!\q
    here  ^                                             and here ^



> +printf '#endif\n\n'
> +
> +if [ "$1" -lt 2 ]; then
> +	printf '#error Totally bogus HZ value!\n'
> +	exit 1
> +fi
> +
> +s="$(fmuls 32 1000 "$1")"
> +printf '#define HZ_TO_MSEC_MUL32\tU64_C(0x%X)\n' "$(fmul "${s}" 1000 "$1")"
> +printf '#define HZ_TO_MSEC_ADJ32\tU64_C(0x%X)\n' "$(fadj "${s}" 1000 "$1")"
> +printf '#define HZ_TO_MSEC_SHR32\t%s\n' "${s}"
> +
> +s="$(fmuls 32 "$1" 1000)"
> +printf '#define MSEC_TO_HZ_MUL32\tU64_C(0x%X)\n' "$(fmul "${s}" "$1" 1000)"
> +printf '#define MSEC_TO_HZ_ADJ32\tU64_C(0x%X)\n' "$(fadj "${s}" "$1" 1000)"
> +printf '#define MSEC_TO_HZ_SHR32\t%s\n' "${s}"
> +
> +cd="$(gcd "$1" 1000)"
> +printf '#define HZ_TO_MSEC_NUM\t\t%s\n' "$((1000 / cd))"
> +printf '#define HZ_TO_MSEC_DEN\t\t%s\n' "$((hz / cd))"
> +printf '#define MSEC_TO_HZ_NUM\t\t%s\n' "$((hz / cd))"
> +printf '#define MSEC_TO_HZ_DEN\t\t%s\n\n' "$((1000 / cd))"
> +
> +s="$(fmuls 32 1000000 "$1")"
> +printf '#define HZ_TO_USEC_MUL32\tU64_C(0x%X)\n' "$(fmul "${s}" 1000000 "$1")"
> +printf '#define HZ_TO_USEC_ADJ32\tU64_C(0x%X)\n' "$(fadj "${s}" 1000000 "$1")"
> +printf '#define HZ_TO_USEC_SHR32\t%s\n' "${s}"
> +
> +s="$(fmuls 32 "$1" 1000000)"
> +printf '#define USEC_TO_HZ_MUL32\tU64_C(0x%X)\n' "$(fmul "${s}" "$1" 1000000)"
> +printf '#define USEC_TO_HZ_ADJ32\tU64_C(0x%X)\n' "$(fadj "${s}" "$1" 1000000)"
> +printf '#define USEC_TO_HZ_SHR32\t%s\n' "${s}"
> +
> +cd="$(gcd "$1" 1000000)"
> +printf '#define HZ_TO_USEC_NUM\t\t%s\n' "$((1000000 / cd))"
> +printf '#define HZ_TO_USEC_DEN\t\t%s\n' "$((hz / cd))"
> +printf '#define USEC_TO_HZ_NUM\t\t%s\n' "$((hz / cd))"
> +printf '#define USEC_TO_HZ_DEN\t\t%s\n' "$((1000000 / cd))"
> +
> +cd="$(gcd "$1" 1000000000)"
> +printf '#define HZ_TO_NSEC_NUM\t\t%s\n' "$((1000000000 / cd))"
> +printf '#define HZ_TO_NSEC_DEN\t\t%s\n' "$((hz / cd))"
> +printf '#define NSEC_TO_HZ_NUM\t\t%s\n' "$((hz / cd))"
> +printf '#define NSEC_TO_HZ_DEN\t\t%s\n' "$((1000000000 / cd))"
> +
> +printf '\n#endif /* KERNEL_TIMECONST_H */\n'
> 

-- 
Regards
--
Kieran

Powered by blists - more mailing lists