Hello! This series contains torture-test updates: 1. Suppress CPU stall warnings during shutdown ftrace dump. 2. Prepare scripting for shift from %p to %pK because NULL-pointers printout changes. 3. Change printk() %p to %pK. 4. Reduce #ifdefs for preempt_schedule(). 5. Preempt RCU-preempt readers more vigorously. 6. Fix build directory error message, courtesy of SeongJae Park. 7. Remove the unused script config2frag.sh, courtesy of SeongJae Park. 8. Remove the unused variable `alldone` from kvm.sh, courtesy of SeongJae Park. 9. Use consistent help text for --qemu-args in kvm.sh, courtesy of SeongJae Park. 10. Support running kvm.sh from any directory, courtesy of SeongJae Park. 11. Improve result directory readability check, courtesy of SeongJae Park. 12. Simplify logging, courtesy of SeongJae Park. 13. Simplify functions.sh include path, courtesy of SeongJae Park. 14. Skip redundant build-directory check, courtesy of SeongJae Park. 15. Place all torture-test modules in one MAINTAINERS group. 16. Fix locktorture rwsem reader_delay, courtesy of Davidlohr Bueso. 17. Fix locktorture reader/writer-number corner cases, courtesy of Davidlohr Bueso. 18. Make stutter less vulnerable to compilers and races. 19. Eliminate torture_runnable and perf_runnable. 20. Save a line in stutter_wait(): while -> for. Thanx, Paul ------------------------------------------------------------------------ b/Documentation/admin-guide/kernel-parameters.txt | 9 b/Documentation/locking/locktorture.txt | 5 b/MAINTAINERS | 22 +- b/include/linux/torture.h | 8 b/kernel/locking/locktorture.c | 108 ++++------ b/kernel/rcu/rcuperf.c | 6 b/kernel/rcu/rcutorture.c | 14 - b/kernel/torture.c | 45 ++-- b/tools/testing/selftests/rcutorture/bin/configinit.sh | 2 b/tools/testing/selftests/rcutorture/bin/kvm-build.sh | 5 b/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh | 2 b/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh | 4 b/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcuperf-ftrace.sh | 2 b/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcuperf.sh | 4 b/tools/testing/selftests/rcutorture/bin/kvm-recheck.sh | 2 b/tools/testing/selftests/rcutorture/bin/kvm-test-1-run.sh | 6 b/tools/testing/selftests/rcutorture/bin/kvm.sh | 42 +-- b/tools/testing/selftests/rcutorture/bin/parse-torture.sh | 2 b/tools/testing/selftests/rcutorture/configs/lock/ver_functions.sh | 1 b/tools/testing/selftests/rcutorture/configs/rcu/ver_functions.sh | 1 b/tools/testing/selftests/rcutorture/configs/rcuperf/ver_functions.sh | 1 tools/testing/selftests/rcutorture/bin/config2frag.sh | 25 -- 22 files changed, 126 insertions(+), 190 deletions(-)
The torture_shutdown() function directly invokes ftrace_dump(), which can result in RCU CPU stall warnings when the ftrace buffer is large, which it usually is. This commit therefore invoks rcu_ftrace_dump() in place of ftrace_dump(), suppressing RCU CPU stall warnings during this time. Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> --- kernel/torture.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kernel/torture.c b/kernel/torture.c index 637e172835d8..52781e838541 100644 --- a/kernel/torture.c +++ b/kernel/torture.c @@ -47,6 +47,7 @@ #include <linux/ktime.h> #include <asm/byteorder.h> #include <linux/torture.h> +#include "rcu/rcu.h" MODULE_LICENSE("GPL"); MODULE_AUTHOR("Paul E. McKenney <paulmck@us.ibm.com>"); @@ -500,7 +501,7 @@ static int torture_shutdown(void *arg) torture_shutdown_hook(); else VERBOSE_TOROUT_STRING("No torture_shutdown_hook(), skipping."); - ftrace_dump(DUMP_ALL); + rcu_ftrace_dump(DUMP_ALL); kernel_power_off(); /* Shut down the system. */ return 0; } -- 2.5.2
Because %p prints "(null)" and %pK prints "0000000000000000" or (on 32-bit systems) "00000000", this commit adjusts torture-test scripting accordingly. Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> --- tools/testing/selftests/rcutorture/bin/parse-torture.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/rcutorture/bin/parse-torture.sh b/tools/testing/selftests/rcutorture/bin/parse-torture.sh index f12c38909b00..5987e50cfeb4 100755 --- a/tools/testing/selftests/rcutorture/bin/parse-torture.sh +++ b/tools/testing/selftests/rcutorture/bin/parse-torture.sh @@ -55,7 +55,7 @@ then exit fi -grep --binary-files=text 'torture:.*ver:' $file | grep --binary-files=text -v '(null)' | sed -e 's/^(initramfs)[^]]*] //' -e 's/^\[[^]]*] //' | +grep --binary-files=text 'torture:.*ver:' $file | egrep --binary-files=text -v '\(null\)|rtc: 000000000* ' | sed -e 's/^(initramfs)[^]]*] //' -e 's/^\[[^]]*] //' | awk ' BEGIN { ver = 0; -- 2.5.2
This commit changes the %p printk() in rcutorture to %pK. This could be considered irrelevant, given that any user able to start rcutorture could inflict far heavier damage on the system, but it doesn't hurt to change it and doing so avoids script-generated noise. Reported-by: Tobin C. Harding <me@tobin.cc> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> --- kernel/rcu/rcutorture.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c index 74f6b0146b98..b6fbbeb5a7da 100644 --- a/kernel/rcu/rcutorture.c +++ b/kernel/rcu/rcutorture.c @@ -1256,7 +1256,7 @@ rcu_torture_stats_print(void) } pr_alert("%s%s ", torture_type, TORTURE_FLAG); - pr_cont("rtc: %p ver: %lu tfle: %d rta: %d rtaf: %d rtf: %d ", + pr_cont("rtc: %pK ver: %lu tfle: %d rta: %d rtaf: %d rtf: %d ", rcu_torture_current, rcu_torture_current_version, list_empty(&rcu_torture_freelist), -- 2.5.2
This commit adds a torture_preempt_schedule() that is nothingness in !PREEMPT builds and is preempt_schedule() otherwise. Then torture_preempt_schedule() is used to eliminate several ugly #ifdefs, both in rcutorture and in locktorture. Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> --- include/linux/torture.h | 6 ++++++ kernel/locking/locktorture.c | 24 ++++++------------------ kernel/rcu/rcutorture.c | 4 +--- 3 files changed, 13 insertions(+), 21 deletions(-) diff --git a/include/linux/torture.h b/include/linux/torture.h index a45702eb3e7b..907d266aaddc 100644 --- a/include/linux/torture.h +++ b/include/linux/torture.h @@ -96,4 +96,10 @@ void _torture_stop_kthread(char *m, struct task_struct **tp); #define torture_stop_kthread(n, tp) \ _torture_stop_kthread("Stopping " #n " task", &(tp)) +#ifdef CONFIG_PREEMPT +#define torture_preempt_schedule() preempt_schedule() +#else +#define torture_preempt_schedule() +#endif + #endif /* __LINUX_TORTURE_H */ diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c index f24582d4dad3..617cea2520b3 100644 --- a/kernel/locking/locktorture.c +++ b/kernel/locking/locktorture.c @@ -130,10 +130,8 @@ static void torture_lock_busted_write_delay(struct torture_random_state *trsp) if (!(torture_random(trsp) % (cxt.nrealwriters_stress * 2000 * longdelay_ms))) mdelay(longdelay_ms); -#ifdef CONFIG_PREEMPT if (!(torture_random(trsp) % (cxt.nrealwriters_stress * 20000))) - preempt_schedule(); /* Allow test to be preempted. */ -#endif + torture_preempt_schedule(); /* Allow test to be preempted. */ } static void torture_lock_busted_write_unlock(void) @@ -179,10 +177,8 @@ static void torture_spin_lock_write_delay(struct torture_random_state *trsp) if (!(torture_random(trsp) % (cxt.nrealwriters_stress * 2 * shortdelay_us))) udelay(shortdelay_us); -#ifdef CONFIG_PREEMPT if (!(torture_random(trsp) % (cxt.nrealwriters_stress * 20000))) - preempt_schedule(); /* Allow test to be preempted. */ -#endif + torture_preempt_schedule(); /* Allow test to be preempted. */ } static void torture_spin_lock_write_unlock(void) __releases(torture_spinlock) @@ -352,10 +348,8 @@ static void torture_mutex_delay(struct torture_random_state *trsp) mdelay(longdelay_ms * 5); else mdelay(longdelay_ms / 5); -#ifdef CONFIG_PREEMPT if (!(torture_random(trsp) % (cxt.nrealwriters_stress * 20000))) - preempt_schedule(); /* Allow test to be preempted. */ -#endif + torture_preempt_schedule(); /* Allow test to be preempted. */ } static void torture_mutex_unlock(void) __releases(torture_mutex) @@ -507,10 +501,8 @@ static void torture_rtmutex_delay(struct torture_random_state *trsp) if (!(torture_random(trsp) % (cxt.nrealwriters_stress * 2 * shortdelay_us))) udelay(shortdelay_us); -#ifdef CONFIG_PREEMPT if (!(torture_random(trsp) % (cxt.nrealwriters_stress * 20000))) - preempt_schedule(); /* Allow test to be preempted. */ -#endif + torture_preempt_schedule(); /* Allow test to be preempted. */ } static void torture_rtmutex_unlock(void) __releases(torture_rtmutex) @@ -547,10 +539,8 @@ static void torture_rwsem_write_delay(struct torture_random_state *trsp) mdelay(longdelay_ms * 10); else mdelay(longdelay_ms / 10); -#ifdef CONFIG_PREEMPT if (!(torture_random(trsp) % (cxt.nrealwriters_stress * 20000))) - preempt_schedule(); /* Allow test to be preempted. */ -#endif + torture_preempt_schedule(); /* Allow test to be preempted. */ } static void torture_rwsem_up_write(void) __releases(torture_rwsem) @@ -574,10 +564,8 @@ static void torture_rwsem_read_delay(struct torture_random_state *trsp) mdelay(longdelay_ms * 2); else mdelay(longdelay_ms / 2); -#ifdef CONFIG_PREEMPT if (!(torture_random(trsp) % (cxt.nrealreaders_stress * 20000))) - preempt_schedule(); /* Allow test to be preempted. */ -#endif + torture_preempt_schedule(); /* Allow test to be preempted. */ } static void torture_rwsem_up_read(void) __releases(torture_rwsem) diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c index b6fbbeb5a7da..61fb95612125 100644 --- a/kernel/rcu/rcutorture.c +++ b/kernel/rcu/rcutorture.c @@ -315,11 +315,9 @@ static void rcu_read_delay(struct torture_random_state *rrsp) } if (!(torture_random(rrsp) % (nrealreaders * 2 * shortdelay_us))) udelay(shortdelay_us); -#ifdef CONFIG_PREEMPT if (!preempt_count() && !(torture_random(rrsp) % (nrealreaders * 20000))) - preempt_schedule(); /* No QS if preempt_disable() in effect */ -#endif + torture_preempt_schedule(); /* QS only if preemptible. */ } static void rcu_torture_read_unlock(int idx) __releases(RCU) -- 2.5.2
This commit attempts to make a very rare rcutorture failure happen more often by increasing the fraction of RCU-preempt read-side critical sections that are preempted. Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> --- kernel/rcu/rcutorture.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c index 61fb95612125..2e810443975d 100644 --- a/kernel/rcu/rcutorture.c +++ b/kernel/rcu/rcutorture.c @@ -316,7 +316,7 @@ static void rcu_read_delay(struct torture_random_state *rrsp) if (!(torture_random(rrsp) % (nrealreaders * 2 * shortdelay_us))) udelay(shortdelay_us); if (!preempt_count() && - !(torture_random(rrsp) % (nrealreaders * 20000))) + !(torture_random(rrsp) % (nrealreaders * 500))) torture_preempt_schedule(); /* QS only if preemptible. */ } -- 2.5.2
From: SeongJae Park <sj38.park@gmail.com> The 'configinit.sh' script checks the format of optional argument for the build directory, printing an error message if the format is not valid. However, the error message uses the wrong variable, indicating an empty string even though the user entered a non-empty (but erroneous) string. This commit fixes the script to use the correct variable. Fixes: c87b9c601ac8 ("rcutorture: Add KVM-based test framework") Signed-off-by: SeongJae Park <sj38.park@gmail.com> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> --- tools/testing/selftests/rcutorture/bin/configinit.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/rcutorture/bin/configinit.sh b/tools/testing/selftests/rcutorture/bin/configinit.sh index 51f66a7ce876..c15f270e121d 100755 --- a/tools/testing/selftests/rcutorture/bin/configinit.sh +++ b/tools/testing/selftests/rcutorture/bin/configinit.sh @@ -51,7 +51,7 @@ then mkdir $builddir fi else - echo Bad build directory: \"$builddir\" + echo Bad build directory: \"$buildloc\" exit 2 fi fi -- 2.5.2
From: SeongJae Park <sj38.park@gmail.com> The 'config2frag.sh' script is not used, so this commit removes it. Fixes: c87b9c601ac8 ("rcutorture: Add KVM-based test framework") Signed-off-by: SeongJae Park <sj38.park@gmail.com> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> --- .../selftests/rcutorture/bin/config2frag.sh | 25 ---------------------- 1 file changed, 25 deletions(-) delete mode 100755 tools/testing/selftests/rcutorture/bin/config2frag.sh diff --git a/tools/testing/selftests/rcutorture/bin/config2frag.sh b/tools/testing/selftests/rcutorture/bin/config2frag.sh deleted file mode 100755 index 56f51ae13d73..000000000000 --- a/tools/testing/selftests/rcutorture/bin/config2frag.sh +++ /dev/null @@ -1,25 +0,0 @@ -#!/bin/bash -# Usage: config2frag.sh < .config > configfrag -# -# Converts the "# CONFIG_XXX is not set" to "CONFIG_XXX=n" so that the -# resulting file becomes a legitimate Kconfig fragment. -# -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 2 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program; if not, you can access it online at -# http://www.gnu.org/licenses/gpl-2.0.html. -# -# Copyright (C) IBM Corporation, 2013 -# -# Authors: Paul E. McKenney <paulmck@linux.vnet.ibm.com> - -LANG=C sed -e 's/^# CONFIG_\([a-zA-Z0-9_]*\) is not set$/CONFIG_\1=n/' -- 2.5.2
From: SeongJae Park <sj38.park@gmail.com> The variable `alldone` is defined but not used within an awk script. This commit therefore removes it. Fixes:53954671033d ("rcutorture: Do better bin packing") Signed-off-by: SeongJae Park <sj38.park@gmail.com> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> --- tools/testing/selftests/rcutorture/bin/kvm.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/testing/selftests/rcutorture/bin/kvm.sh b/tools/testing/selftests/rcutorture/bin/kvm.sh index ccd49e958fd2..7eb8d14e2aab 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm.sh @@ -238,7 +238,6 @@ BEGIN { } END { - alldone = 0; batch = 0; nc = -1; -- 2.5.2
From: SeongJae Park <sj38.park@gmail.com> The '--qemu-args' option's help text is wrongly copied from '--qemu-cmd' option and its argument type description message format is inconsistent with other arguments. This commit fixes the usage and type messages to be consistent with others. Fixes: e9ce640001c6 ("rcutorture: Add --qemu-args argument to kvm.sh") Signed-off-by: SeongJae Park <sj38.park@gmail.com> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> --- tools/testing/selftests/rcutorture/bin/kvm.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/rcutorture/bin/kvm.sh b/tools/testing/selftests/rcutorture/bin/kvm.sh index 7eb8d14e2aab..64d96fc3dd62 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm.sh @@ -70,7 +70,7 @@ usage () { echo " --kmake-arg kernel-make-arguments" echo " --mac nn:nn:nn:nn:nn:nn" echo " --no-initrd" - echo " --qemu-args qemu-system-..." + echo " --qemu-args qemu-arguments" echo " --qemu-cmd qemu-system-..." echo " --results absolute-pathname" echo " --torture rcu" @@ -150,7 +150,7 @@ do TORTURE_INITRD=""; export TORTURE_INITRD ;; --qemu-args|--qemu-arg) - checkarg --qemu-args "-qemu args" $# "$2" '^-' '^error' + checkarg --qemu-args "(qemu arguments)" $# "$2" '^-' '^error' TORTURE_QEMU_ARG="$2" shift ;; -- 2.5.2
From: SeongJae Park <sj38.park@gmail.com> The 'kvm.sh' rcutorture script requires that it be invoked from the top of Linux-kernel source tree. It is just a subtle restriction, but users using it for the first time could forget the restriction and be confused. Moreover, it makes commands a little longer, which can be frustrating. This commit therefore lets users invoke the script from any location. Signed-off-by: SeongJae Park <sj38.park@gmail.com> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> --- tools/testing/selftests/rcutorture/bin/kvm.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/rcutorture/bin/kvm.sh b/tools/testing/selftests/rcutorture/bin/kvm.sh index 64d96fc3dd62..d2a4fd94de6a 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm.sh @@ -1,8 +1,7 @@ #!/bin/bash # # Run a series of 14 tests under KVM. These are not particularly -# well-selected or well-tuned, but are the current set. Run from the -# top level of the source tree. +# well-selected or well-tuned, but are the current set. # # Edit the definitions below to set the locations of the various directories, # as well as the test duration. @@ -34,6 +33,8 @@ T=${TMPDIR-/tmp}/kvm.sh.$$ trap 'rm -rf $T' 0 mkdir $T +cd `dirname $scriptname`/../../../../../ + dur=$((30*60)) dryrun="" KVM="`pwd`/tools/testing/selftests/rcutorture"; export KVM -- 2.5.2
From: SeongJae Park <sj38.park@gmail.com> The kvm-recheck-(lock|rcu|rcuperf).sh scripts check whether the user-specified results directory exists. If not, it prints out error message that says the specified directory is unreadable. To make the message more precise, this commit adds a readability check. Fixes: 2193e1604eac ("rcutorture: Abstract kvm-recheck.sh") Signed-off-by: SeongJae Park <sj38.park@gmail.com> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> --- tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm-recheck-rcuperf.sh | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh b/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh index 43f764098e50..2de92f43ee8c 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh @@ -23,7 +23,7 @@ # Authors: Paul E. McKenney <paulmck@linux.vnet.ibm.com> i="$1" -if test -d $i +if test -d "$i" -a -r "$i" then : else diff --git a/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh b/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh index 559e01ac86be..9e34656bf659 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh @@ -23,7 +23,7 @@ # Authors: Paul E. McKenney <paulmck@linux.vnet.ibm.com> i="$1" -if test -d $i +if test -d "$i" -a -r "$i" then : else diff --git a/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcuperf.sh b/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcuperf.sh index 8f3121afc716..6138fd94abfe 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcuperf.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcuperf.sh @@ -23,7 +23,7 @@ # Authors: Paul E. McKenney <paulmck@linux.vnet.ibm.com> i="$1" -if test -d $i +if test -d "$i" -a -r "$i" then : else -- 2.5.2
From: SeongJae Park <sj38.park@gmail.com> Both the 'kvm.sh' and 'kvm-test-1-run.sh' scripts log messages by printing the message to 'stdout' and then also printing it into the log file. Generation of the message thus occurs twice, once for 'stdout' and once for the log file. Moreover, many of the messages contain 'date' output, which results in date being invoked twice (once for stdout print, once for log file write). As a result, the date information in stdout and log file can differ, which could cause confusion. This commit therefore simplifies the logging procedure by using 'tee'. Signed-off-by: SeongJae Park <sj38.park@gmail.com> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> --- .../selftests/rcutorture/bin/kvm-test-1-run.sh | 4 +-- tools/testing/selftests/rcutorture/bin/kvm.sh | 32 ++++++++-------------- 2 files changed, 12 insertions(+), 24 deletions(-) diff --git a/tools/testing/selftests/rcutorture/bin/kvm-test-1-run.sh b/tools/testing/selftests/rcutorture/bin/kvm-test-1-run.sh index ab14b97c942c..0406c67378cb 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-test-1-run.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm-test-1-run.sh @@ -154,9 +154,7 @@ cpu_count=`configfrag_boot_cpus "$boot_args" "$config_template" "$cpu_count"` vcpus=`identify_qemu_vcpus` if test $cpu_count -gt $vcpus then - echo CPU count limited from $cpu_count to $vcpus - touch $resdir/Warnings - echo CPU count limited from $cpu_count to $vcpus >> $resdir/Warnings + echo CPU count limited from $cpu_count to $vcpus | tee -a $resdir/Warnings cpu_count=$vcpus fi qemu_args="`specify_qemu_cpus "$QEMU" "$qemu_args" "$cpu_count"`" diff --git a/tools/testing/selftests/rcutorture/bin/kvm.sh b/tools/testing/selftests/rcutorture/bin/kvm.sh index d2a4fd94de6a..7d1f607f0f76 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm.sh @@ -331,8 +331,7 @@ awk < $T/cfgcpu.pack \ # Dump out the scripting required to run one test batch. function dump(first, pastlast, batchnum) { - print "echo ----Start batch " batchnum ": `date`"; - print "echo ----Start batch " batchnum ": `date` >> " rd "/log"; + print "echo ----Start batch " batchnum ": `date` | tee -a " rd "log"; print "needqemurun=" jn=1 for (j = first; j < pastlast; j++) { @@ -349,21 +348,18 @@ function dump(first, pastlast, batchnum) ovf = "-ovf"; else ovf = ""; - print "echo ", cfr[jn], cpusr[jn] ovf ": Starting build. `date`"; - print "echo ", cfr[jn], cpusr[jn] ovf ": Starting build. `date` >> " rd "/log"; + print "echo ", cfr[jn], cpusr[jn] ovf ": Starting build. `date` | tee -a " rd "log"; print "rm -f " builddir ".*"; print "touch " builddir ".wait"; print "mkdir " builddir " > /dev/null 2>&1 || :"; print "mkdir " rd cfr[jn] " || :"; print "kvm-test-1-run.sh " CONFIGDIR cf[j], builddir, rd cfr[jn], dur " \"" TORTURE_QEMU_ARG "\" \"" TORTURE_BOOTARGS "\" > " rd cfr[jn] "/kvm-test-1-run.sh.out 2>&1 &" - print "echo ", cfr[jn], cpusr[jn] ovf ": Waiting for build to complete. `date`"; - print "echo ", cfr[jn], cpusr[jn] ovf ": Waiting for build to complete. `date` >> " rd "/log"; + print "echo ", cfr[jn], cpusr[jn] ovf ": Waiting for build to complete. `date` | tee -a " rd "log"; print "while test -f " builddir ".wait" print "do" print "\tsleep 1" print "done" - print "echo ", cfr[jn], cpusr[jn] ovf ": Build complete. `date`"; - print "echo ", cfr[jn], cpusr[jn] ovf ": Build complete. `date` >> " rd "/log"; + print "echo ", cfr[jn], cpusr[jn] ovf ": Build complete. `date` | tee -a " rd "log"; jn++; } for (j = 1; j < jn; j++) { @@ -371,8 +367,7 @@ function dump(first, pastlast, batchnum) print "rm -f " builddir ".ready" print "if test -f \"" rd cfr[j] "/builtkernel\"" print "then" - print "\techo ----", cfr[j], cpusr[j] ovf ": Kernel present. `date`"; - print "\techo ----", cfr[j], cpusr[j] ovf ": Kernel present. `date` >> " rd "/log"; + print "\techo ----", cfr[j], cpusr[j] ovf ": Kernel present. `date` | tee -a " rd "log"; print "\tneedqemurun=1" print "fi" } @@ -386,31 +381,26 @@ function dump(first, pastlast, batchnum) njitter = ja[1]; if (TORTURE_BUILDONLY && njitter != 0) { njitter = 0; - print "echo Build-only run, so suppressing jitter >> " rd "/log" + print "echo Build-only run, so suppressing jitter | tee -a " rd "log" } if (TORTURE_BUILDONLY) { print "needqemurun=" } print "if test -n \"$needqemurun\"" print "then" - print "\techo ---- Starting kernels. `date`"; - print "\techo ---- Starting kernels. `date` >> " rd "/log"; + print "\techo ---- Starting kernels. `date` | tee -a " rd "log"; for (j = 0; j < njitter; j++) print "\tjitter.sh " j " " dur " " ja[2] " " ja[3] "&" print "\twait" - print "\techo ---- All kernel runs complete. `date`"; - print "\techo ---- All kernel runs complete. `date` >> " rd "/log"; + print "\techo ---- All kernel runs complete. `date` | tee -a " rd "log"; print "else" print "\twait" - print "\techo ---- No kernel runs. `date`"; - print "\techo ---- No kernel runs. `date` >> " rd "/log"; + print "\techo ---- No kernel runs. `date` | tee -a " rd "log"; print "fi" for (j = 1; j < jn; j++) { builddir=KVM "/b" j - print "echo ----", cfr[j], cpusr[j] ovf ": Build/run results:"; - print "echo ----", cfr[j], cpusr[j] ovf ": Build/run results: >> " rd "/log"; - print "cat " rd cfr[j] "/kvm-test-1-run.sh.out"; - print "cat " rd cfr[j] "/kvm-test-1-run.sh.out >> " rd "/log"; + print "echo ----", cfr[j], cpusr[j] ovf ": Build/run results: | tee -a " rd "log"; + print "cat " rd cfr[j] "/kvm-test-1-run.sh.out | tee -a " rd "log"; } } -- 2.5.2
From: SeongJae Park <sj38.park@gmail.com> Inclusions of 'functions.sh' from 'kvm-test-1-run.sh' and 'kvm-recheck*.sh' use its absolute path. Because the directory containing 'functions.sh' is already in PATH, the full path is unnecessary. This commit therefore simplifies the inclusions to use the short relative path. Signed-off-by: SeongJae Park <sj38.park@gmail.com> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> --- tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm-recheck-rcuperf-ftrace.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm-recheck-rcuperf.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm-recheck.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm-test-1-run.sh | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh b/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh index 9e34656bf659..c2e1bb6d0cba 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh @@ -30,7 +30,7 @@ else echo Unreadable results directory: $i exit 1 fi -. tools/testing/selftests/rcutorture/bin/functions.sh +. functions.sh configfile=`echo $i | sed -e 's/^.*\///'` ngps=`grep ver: $i/console.log 2> /dev/null | tail -1 | sed -e 's/^.* ver: //' -e 's/ .*$//'` diff --git a/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcuperf-ftrace.sh b/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcuperf-ftrace.sh index f79b0e9e84fc..963f71289d22 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcuperf-ftrace.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcuperf-ftrace.sh @@ -26,7 +26,7 @@ # Authors: Paul E. McKenney <paulmck@linux.vnet.ibm.com> i="$1" -. tools/testing/selftests/rcutorture/bin/functions.sh +. functions.sh if test "`grep -c 'rcu_exp_grace_period.*start' < $i/console.log`" -lt 100 then diff --git a/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcuperf.sh b/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcuperf.sh index 6138fd94abfe..ccebf772fa1e 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcuperf.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcuperf.sh @@ -31,7 +31,7 @@ else exit 1 fi PATH=`pwd`/tools/testing/selftests/rcutorture/bin:$PATH; export PATH -. tools/testing/selftests/rcutorture/bin/functions.sh +. functions.sh if kvm-recheck-rcuperf-ftrace.sh $i then diff --git a/tools/testing/selftests/rcutorture/bin/kvm-recheck.sh b/tools/testing/selftests/rcutorture/bin/kvm-recheck.sh index f659346d3358..f7e988f369dd 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-recheck.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm-recheck.sh @@ -25,7 +25,7 @@ # Authors: Paul E. McKenney <paulmck@linux.vnet.ibm.com> PATH=`pwd`/tools/testing/selftests/rcutorture/bin:$PATH; export PATH -. tools/testing/selftests/rcutorture/bin/functions.sh +. functions.sh for rd in "$@" do firsttime=1 diff --git a/tools/testing/selftests/rcutorture/bin/kvm-test-1-run.sh b/tools/testing/selftests/rcutorture/bin/kvm-test-1-run.sh index 0406c67378cb..1b78a12740e5 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-test-1-run.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm-test-1-run.sh @@ -42,7 +42,7 @@ T=${TMPDIR-/tmp}/kvm-test-1-run.sh.$$ trap 'rm -rf $T' 0 mkdir $T -. $KVM/bin/functions.sh +. functions.sh . $CONFIGFRAG/ver_functions.sh config_template=${1} -- 2.5.2
From: SeongJae Park <sj38.park@gmail.com> Check for build-directory existence and write permissions are provided in both 'kvm-test-1-run.sh' an 'kvm-build.sh'. Because the 'kvm-build.sh' is dependent on 'kvm-test-1-run.sh' ('kvm-build.sh' uses variables that defined from its caller.), these checks are unnecessarily duplicated. This commit therefore removes the check in from the 'kvm-build.sh' script. Signed-off-by: SeongJae Park <sj38.park@gmail.com> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> --- tools/testing/selftests/rcutorture/bin/kvm-build.sh | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tools/testing/selftests/rcutorture/bin/kvm-build.sh b/tools/testing/selftests/rcutorture/bin/kvm-build.sh index fb66d0173638..34d126734cde 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-build.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm-build.sh @@ -29,11 +29,6 @@ then exit 1 fi builddir=${2} -if test -z "$builddir" -o ! -d "$builddir" -o ! -w "$builddir" -then - echo "kvm-build.sh :$builddir: Not a writable directory, cannot build into it" - exit 1 -fi T=${TMPDIR-/tmp}/test-linux.sh.$$ trap 'rm -rf $T' 0 -- 2.5.2
There is some confusion about where patches to kernel/torture.c and kernel/locking/locktorture.c should be sent. This commit therefore updates MAINTAINERS appropriately. Reported-by: Peter Zijlstra <peterz@infradead.org> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- MAINTAINERS | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index aa71ab52fd76..bdeb64f8bf54 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8194,6 +8194,7 @@ F: arch/*/include/asm/rwsem.h F: include/linux/seqlock.h F: lib/locking*.[ch] F: kernel/locking/ +X: kernel/locking/locktorture.c LOGICAL DISK MANAGER SUPPORT (LDM, Windows 2000/XP/Vista Dynamic Disks) M: "Richard Russon (FlatCap)" <ldm@flatcap.org> @@ -11451,15 +11452,6 @@ L: linux-wireless@vger.kernel.org S: Orphan F: drivers/net/wireless/ray* -RCUTORTURE MODULE -M: Josh Triplett <josh@joshtriplett.org> -M: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> -L: linux-kernel@vger.kernel.org -S: Supported -T: git git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git -F: Documentation/RCU/torture.txt -F: kernel/rcu/rcutorture.c - RCUTORTURE TEST FRAMEWORK M: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> M: Josh Triplett <josh@joshtriplett.org> @@ -13748,6 +13740,18 @@ L: platform-driver-x86@vger.kernel.org S: Maintained F: drivers/platform/x86/topstar-laptop.c +TORTURE-TEST MODULES +M: Davidlohr Bueso <dave@stgolabs.net> +M: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> +M: Josh Triplett <josh@joshtriplett.org> +L: linux-kernel@vger.kernel.org +S: Supported +T: git git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git +F: Documentation/RCU/torture.txt +F: kernel/torture.c +F: kernel/rcu/rcutorture.c +F: kernel/locking/locktorture.c + TOSHIBA ACPI EXTRAS DRIVER M: Azael Avalos <coproscefalo@gmail.com> L: platform-driver-x86@vger.kernel.org -- 2.5.2
From: Davidlohr Bueso <dave@stgolabs.net> We should account for nreader threads, not writers in this callback. Could even trigger a div by 0 if the user explicitly disables writers. Signed-off-by: Davidlohr Bueso <dbueso@suse.de> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> --- kernel/locking/locktorture.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c index 617cea2520b3..a307a79e6b0b 100644 --- a/kernel/locking/locktorture.c +++ b/kernel/locking/locktorture.c @@ -560,7 +560,7 @@ static void torture_rwsem_read_delay(struct torture_random_state *trsp) /* We want a long delay occasionally to force massive contention. */ if (!(torture_random(trsp) % - (cxt.nrealwriters_stress * 2000 * longdelay_ms))) + (cxt.nrealreaders_stress * 2000 * longdelay_ms))) mdelay(longdelay_ms * 2); else mdelay(longdelay_ms / 2); -- 2.5.2
From: Davidlohr Bueso <dave@stgolabs.net> Things can explode for locktorture if the user does combinations of nwriters_stress=0 nreaders_stress=0. Fix this by not assuming we always want to torture writer threads. Reported-by: Jeremy Linton <jeremy.linton@arm.com> Signed-off-by: Davidlohr Bueso <dbueso@suse.de> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> Reviewed-by: Jeremy Linton <jeremy.linton@arm.com> Tested-by: Jeremy Linton <jeremy.linton@arm.com> --- kernel/locking/locktorture.c | 76 +++++++++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 32 deletions(-) diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c index a307a79e6b0b..2a1fc2a58910 100644 --- a/kernel/locking/locktorture.c +++ b/kernel/locking/locktorture.c @@ -703,8 +703,7 @@ static void __torture_print_stats(char *page, { bool fail = 0; int i, n_stress; - long max = 0; - long min = statp[0].n_lock_acquired; + long max = 0, min = statp ? statp[0].n_lock_acquired : 0; long long sum = 0; n_stress = write ? cxt.nrealwriters_stress : cxt.nrealreaders_stress; @@ -811,7 +810,7 @@ static void lock_torture_cleanup(void) * such, only perform the underlying torture-specific cleanups, * and avoid anything related to locktorture. */ - if (!cxt.lwsa) + if (!cxt.lwsa && !cxt.lrsa) goto end; if (writer_tasks) { @@ -886,6 +885,13 @@ static int __init lock_torture_init(void) firsterr = -EINVAL; goto unwind; } + + if (nwriters_stress == 0 && nreaders_stress == 0) { + pr_alert("lock-torture: must run at least one locking thread\n"); + firsterr = -EINVAL; + goto unwind; + } + if (cxt.cur_ops->init) cxt.cur_ops->init(); @@ -909,17 +915,19 @@ static int __init lock_torture_init(void) #endif /* Initialize the statistics so that each run gets its own numbers. */ + if (nwriters_stress) { + lock_is_write_held = 0; + cxt.lwsa = kmalloc(sizeof(*cxt.lwsa) * cxt.nrealwriters_stress, GFP_KERNEL); + if (cxt.lwsa == NULL) { + VERBOSE_TOROUT_STRING("cxt.lwsa: Out of memory"); + firsterr = -ENOMEM; + goto unwind; + } - lock_is_write_held = 0; - cxt.lwsa = kmalloc(sizeof(*cxt.lwsa) * cxt.nrealwriters_stress, GFP_KERNEL); - if (cxt.lwsa == NULL) { - VERBOSE_TOROUT_STRING("cxt.lwsa: Out of memory"); - firsterr = -ENOMEM; - goto unwind; - } - for (i = 0; i < cxt.nrealwriters_stress; i++) { - cxt.lwsa[i].n_lock_fail = 0; - cxt.lwsa[i].n_lock_acquired = 0; + for (i = 0; i < cxt.nrealwriters_stress; i++) { + cxt.lwsa[i].n_lock_fail = 0; + cxt.lwsa[i].n_lock_acquired = 0; + } } if (cxt.cur_ops->readlock) { @@ -936,19 +944,21 @@ static int __init lock_torture_init(void) cxt.nrealreaders_stress = cxt.nrealwriters_stress; } - lock_is_read_held = 0; - cxt.lrsa = kmalloc(sizeof(*cxt.lrsa) * cxt.nrealreaders_stress, GFP_KERNEL); - if (cxt.lrsa == NULL) { - VERBOSE_TOROUT_STRING("cxt.lrsa: Out of memory"); - firsterr = -ENOMEM; - kfree(cxt.lwsa); - cxt.lwsa = NULL; - goto unwind; - } - - for (i = 0; i < cxt.nrealreaders_stress; i++) { - cxt.lrsa[i].n_lock_fail = 0; - cxt.lrsa[i].n_lock_acquired = 0; + if (nreaders_stress) { + lock_is_read_held = 0; + cxt.lrsa = kmalloc(sizeof(*cxt.lrsa) * cxt.nrealreaders_stress, GFP_KERNEL); + if (cxt.lrsa == NULL) { + VERBOSE_TOROUT_STRING("cxt.lrsa: Out of memory"); + firsterr = -ENOMEM; + kfree(cxt.lwsa); + cxt.lwsa = NULL; + goto unwind; + } + + for (i = 0; i < cxt.nrealreaders_stress; i++) { + cxt.lrsa[i].n_lock_fail = 0; + cxt.lrsa[i].n_lock_acquired = 0; + } } } @@ -978,12 +988,14 @@ static int __init lock_torture_init(void) goto unwind; } - writer_tasks = kzalloc(cxt.nrealwriters_stress * sizeof(writer_tasks[0]), - GFP_KERNEL); - if (writer_tasks == NULL) { - VERBOSE_TOROUT_ERRSTRING("writer_tasks: Out of memory"); - firsterr = -ENOMEM; - goto unwind; + if (nwriters_stress) { + writer_tasks = kzalloc(cxt.nrealwriters_stress * sizeof(writer_tasks[0]), + GFP_KERNEL); + if (writer_tasks == NULL) { + VERBOSE_TOROUT_ERRSTRING("writer_tasks: Out of memory"); + firsterr = -ENOMEM; + goto unwind; + } } if (cxt.cur_ops->readlock) { -- 2.5.2
The stutter_wait() function repeatedly fetched stutter_pause_test, and should really just fetch it once on each pass. The races should be harmless, but why have the races? Also, the whole point of the value "2" for stutter_pause_test is to get everyone to start at very nearly the same time, but the value "2" was the first jiffy of the stutter rather than the last jiffy of the stutter. This commit rearranges the code to be more sensible. Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> --- kernel/torture.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/kernel/torture.c b/kernel/torture.c index 52781e838541..3bcbd4fbfe18 100644 --- a/kernel/torture.c +++ b/kernel/torture.c @@ -573,18 +573,21 @@ static int stutter; */ void stutter_wait(const char *title) { + int spt; + cond_resched_rcu_qs(); - while (READ_ONCE(stutter_pause_test) || - (torture_runnable && !READ_ONCE(*torture_runnable))) { - if (stutter_pause_test) - if (READ_ONCE(stutter_pause_test) == 1) - schedule_timeout_interruptible(1); - else - while (READ_ONCE(stutter_pause_test)) - cond_resched(); - else + spt = READ_ONCE(stutter_pause_test); + while (spt || (torture_runnable && !READ_ONCE(*torture_runnable))) { + if (spt == 1) { + schedule_timeout_interruptible(1); + } else if (spt == 2) { + while (READ_ONCE(stutter_pause_test)) + cond_resched(); + } else { schedule_timeout_interruptible(round_jiffies_relative(HZ)); + } torture_shutdown_absorb(title); + spt = READ_ONCE(stutter_pause_test); } } EXPORT_SYMBOL_GPL(stutter_wait); @@ -597,17 +600,15 @@ static int torture_stutter(void *arg) { VERBOSE_TOROUT_STRING("torture_stutter task started"); do { - if (!torture_must_stop()) { - if (stutter > 1) { - schedule_timeout_interruptible(stutter - 1); - WRITE_ONCE(stutter_pause_test, 2); - } - schedule_timeout_interruptible(1); + if (!torture_must_stop() && stutter > 1) { WRITE_ONCE(stutter_pause_test, 1); + schedule_timeout_interruptible(stutter - 1); + WRITE_ONCE(stutter_pause_test, 2); + schedule_timeout_interruptible(1); } + WRITE_ONCE(stutter_pause_test, 0); if (!torture_must_stop()) schedule_timeout_interruptible(stutter); - WRITE_ONCE(stutter_pause_test, 0); torture_shutdown_absorb("torture_stutter"); } while (!torture_must_stop()); torture_kthread_stopping("torture_stutter"); -- 2.5.2
The purpose of torture_runnable is to allow rcutorture and locktorture to be started and stopped via sysfs when they are built into the kernel (as in not compiled as loadable modules). However, the 0444 permissions for both instances of torture_runnable prevent this use case from ever being put into practice. Given that there have been no complaints about this deficiency, it is reasonable to conclude that no one actually makes use of this sysfs capability. The perf_runnable module parameter for rcuperf is in the same situation. This commit therefore removes both torture_runnable instances as well as perf_runnable. Reported-by: Thomas Gleixner <tglx@linutronix.de> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> --- Documentation/admin-guide/kernel-parameters.txt | 9 --------- Documentation/locking/locktorture.txt | 5 ----- include/linux/torture.h | 2 +- kernel/locking/locktorture.c | 6 +----- kernel/rcu/rcuperf.c | 6 +----- kernel/rcu/rcutorture.c | 6 +----- kernel/torture.c | 6 ++---- tools/testing/selftests/rcutorture/configs/lock/ver_functions.sh | 1 - tools/testing/selftests/rcutorture/configs/rcu/ver_functions.sh | 1 - .../selftests/rcutorture/configs/rcuperf/ver_functions.sh | 1 - 10 files changed, 6 insertions(+), 37 deletions(-) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 6571fbfdb2a1..66d471f0b92e 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -2049,9 +2049,6 @@ This tests the locking primitive's ability to transition abruptly to and from idle. - locktorture.torture_runnable= [BOOT] - Start locktorture running at boot time. - locktorture.torture_type= [KNL] Specify the locking implementation to test. @@ -3459,9 +3456,6 @@ the same as for rcuperf.nreaders. N, where N is the number of CPUs - rcuperf.perf_runnable= [BOOT] - Start rcuperf running at boot time. - rcuperf.perf_type= [KNL] Specify the RCU implementation to test. @@ -3595,9 +3589,6 @@ Test RCU's dyntick-idle handling. See also the rcutorture.shuffle_interval parameter. - rcutorture.torture_runnable= [BOOT] - Start rcutorture running at boot time. - rcutorture.torture_type= [KNL] Specify the RCU implementation to test. diff --git a/Documentation/locking/locktorture.txt b/Documentation/locking/locktorture.txt index a2ef3a929bf1..6a8df4cd19bf 100644 --- a/Documentation/locking/locktorture.txt +++ b/Documentation/locking/locktorture.txt @@ -57,11 +57,6 @@ torture_type Type of lock to torture. By default, only spinlocks will o "rwsem_lock": read/write down() and up() semaphore pairs. -torture_runnable Start locktorture at boot time in the case where the - module is built into the kernel, otherwise wait for - torture_runnable to be set via sysfs before starting. - By default it will begin once the module is loaded. - ** Torture-framework (RCU + locking) ** diff --git a/include/linux/torture.h b/include/linux/torture.h index 907d266aaddc..66272862070b 100644 --- a/include/linux/torture.h +++ b/include/linux/torture.h @@ -79,7 +79,7 @@ void stutter_wait(const char *title); int torture_stutter_init(int s); /* Initialization and cleanup. */ -bool torture_init_begin(char *ttype, bool v, int *runnable); +bool torture_init_begin(char *ttype, bool v); void torture_init_end(void); bool torture_cleanup_begin(void); void torture_cleanup_end(void); diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c index 2a1fc2a58910..6850ffd69125 100644 --- a/kernel/locking/locktorture.c +++ b/kernel/locking/locktorture.c @@ -77,10 +77,6 @@ struct lock_stress_stats { long n_lock_acquired; }; -int torture_runnable = IS_ENABLED(MODULE); -module_param(torture_runnable, int, 0444); -MODULE_PARM_DESC(torture_runnable, "Start locktorture at module init"); - /* Forward reference. */ static void lock_torture_cleanup(void); @@ -866,7 +862,7 @@ static int __init lock_torture_init(void) &percpu_rwsem_lock_ops, }; - if (!torture_init_begin(torture_type, verbose, &torture_runnable)) + if (!torture_init_begin(torture_type, verbose)) return -EBUSY; /* Process args and tell the world that the torturer is on the job. */ diff --git a/kernel/rcu/rcuperf.c b/kernel/rcu/rcuperf.c index 1f87a02c3399..d1ebdf9868bb 100644 --- a/kernel/rcu/rcuperf.c +++ b/kernel/rcu/rcuperf.c @@ -106,10 +106,6 @@ static int rcu_perf_writer_state; #define MAX_MEAS 10000 #define MIN_MEAS 100 -static int perf_runnable = IS_ENABLED(MODULE); -module_param(perf_runnable, int, 0444); -MODULE_PARM_DESC(perf_runnable, "Start rcuperf at boot"); - /* * Operations vector for selecting different types of tests. */ @@ -646,7 +642,7 @@ rcu_perf_init(void) &tasks_ops, }; - if (!torture_init_begin(perf_type, verbose, &perf_runnable)) + if (!torture_init_begin(perf_type, verbose)) return -EBUSY; /* Process args and tell the world that the perf'er is on the job. */ diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c index 2e810443975d..468a9c1577ed 100644 --- a/kernel/rcu/rcutorture.c +++ b/kernel/rcu/rcutorture.c @@ -187,10 +187,6 @@ static const char *rcu_torture_writer_state_getname(void) return rcu_torture_writer_state_names[i]; } -static int torture_runnable = IS_ENABLED(MODULE); -module_param(torture_runnable, int, 0444); -MODULE_PARM_DESC(torture_runnable, "Start rcutorture at boot"); - #if defined(CONFIG_RCU_BOOST) && !defined(CONFIG_HOTPLUG_CPU) #define rcu_can_boost() 1 #else /* #if defined(CONFIG_RCU_BOOST) && !defined(CONFIG_HOTPLUG_CPU) */ @@ -1729,7 +1725,7 @@ rcu_torture_init(void) &sched_ops, &tasks_ops, }; - if (!torture_init_begin(torture_type, verbose, &torture_runnable)) + if (!torture_init_begin(torture_type, verbose)) return -EBUSY; /* Process args and tell the world that the torturer is on the job. */ diff --git a/kernel/torture.c b/kernel/torture.c index 3bcbd4fbfe18..572576ad9f58 100644 --- a/kernel/torture.c +++ b/kernel/torture.c @@ -61,7 +61,6 @@ static bool verbose; #define FULLSTOP_RMMOD 2 /* Normal rmmod of torture. */ static int fullstop = FULLSTOP_RMMOD; static DEFINE_MUTEX(fullstop_mutex); -static int *torture_runnable; #ifdef CONFIG_HOTPLUG_CPU @@ -577,7 +576,7 @@ void stutter_wait(const char *title) cond_resched_rcu_qs(); spt = READ_ONCE(stutter_pause_test); - while (spt || (torture_runnable && !READ_ONCE(*torture_runnable))) { + while (spt) { if (spt == 1) { schedule_timeout_interruptible(1); } else if (spt == 2) { @@ -649,7 +648,7 @@ static void torture_stutter_cleanup(void) * The runnable parameter points to a flag that controls whether or not * the test is currently runnable. If there is no such flag, pass in NULL. */ -bool torture_init_begin(char *ttype, bool v, int *runnable) +bool torture_init_begin(char *ttype, bool v) { mutex_lock(&fullstop_mutex); if (torture_type != NULL) { @@ -661,7 +660,6 @@ bool torture_init_begin(char *ttype, bool v, int *runnable) } torture_type = ttype; verbose = v; - torture_runnable = runnable; fullstop = FULLSTOP_DONTSTOP; return true; } diff --git a/tools/testing/selftests/rcutorture/configs/lock/ver_functions.sh b/tools/testing/selftests/rcutorture/configs/lock/ver_functions.sh index 252aae618984..80eb646e1319 100644 --- a/tools/testing/selftests/rcutorture/configs/lock/ver_functions.sh +++ b/tools/testing/selftests/rcutorture/configs/lock/ver_functions.sh @@ -38,6 +38,5 @@ per_version_boot_params () { echo $1 `locktorture_param_onoff "$1" "$2"` \ locktorture.stat_interval=15 \ locktorture.shutdown_secs=$3 \ - locktorture.torture_runnable=1 \ locktorture.verbose=1 } diff --git a/tools/testing/selftests/rcutorture/configs/rcu/ver_functions.sh b/tools/testing/selftests/rcutorture/configs/rcu/ver_functions.sh index ffb85ed786fa..24ec91041957 100644 --- a/tools/testing/selftests/rcutorture/configs/rcu/ver_functions.sh +++ b/tools/testing/selftests/rcutorture/configs/rcu/ver_functions.sh @@ -51,7 +51,6 @@ per_version_boot_params () { `rcutorture_param_n_barrier_cbs "$1"` \ rcutorture.stat_interval=15 \ rcutorture.shutdown_secs=$3 \ - rcutorture.torture_runnable=1 \ rcutorture.test_no_idle_hz=1 \ rcutorture.verbose=1 } diff --git a/tools/testing/selftests/rcutorture/configs/rcuperf/ver_functions.sh b/tools/testing/selftests/rcutorture/configs/rcuperf/ver_functions.sh index 34f2a1b35ee5..b9603115d7c7 100644 --- a/tools/testing/selftests/rcutorture/configs/rcuperf/ver_functions.sh +++ b/tools/testing/selftests/rcutorture/configs/rcuperf/ver_functions.sh @@ -46,7 +46,6 @@ rcuperf_param_nwriters () { per_version_boot_params () { echo $1 `rcuperf_param_nreaders "$1"` \ `rcuperf_param_nwriters "$1"` \ - rcuperf.perf_runnable=1 \ rcuperf.shutdown=1 \ rcuperf.verbose=1 } -- 2.5.2
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> --- kernel/torture.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/kernel/torture.c b/kernel/torture.c index 572576ad9f58..37b94012a3f8 100644 --- a/kernel/torture.c +++ b/kernel/torture.c @@ -576,7 +576,7 @@ void stutter_wait(const char *title) cond_resched_rcu_qs(); spt = READ_ONCE(stutter_pause_test); - while (spt) { + for (; spt; spt = READ_ONCE(stutter_pause_test)) { if (spt == 1) { schedule_timeout_interruptible(1); } else if (spt == 2) { @@ -586,7 +586,6 @@ void stutter_wait(const char *title) schedule_timeout_interruptible(round_jiffies_relative(HZ)); } torture_shutdown_absorb(title); - spt = READ_ONCE(stutter_pause_test); } } EXPORT_SYMBOL_GPL(stutter_wait); -- 2.5.2
From: Paul E. McKenney
> Sent: 01 December 2017 20:09
>
> Because %p prints "(null)" and %pK prints "0000000000000000" or (on
> 32-bit systems) "00000000", this commit adjusts torture-test scripting
> accordingly.
Surely NULL v not-NULL is one bit of info that the message needs to contain?
David
On Mon, Dec 04, 2017 at 12:32:30PM +0000, David Laight wrote:
> From: Paul E. McKenney
> > Sent: 01 December 2017 20:09
> >
> > Because %p prints "(null)" and %pK prints "0000000000000000" or (on
> > 32-bit systems) "00000000", this commit adjusts torture-test scripting
> > accordingly.
>
> Surely NULL v not-NULL is one bit of info that the message needs to contain?
Indeed. So the script needs to check for the strings "00000000",
"0000000000000000", and "(null) in the console output". The "(null)"
is what "%p" prints for a NULL pointer, and the other two strings are
what "%pK" prints for a NULL pointer.
Or am I missing your point?
Thanx, Paul
From: Paul E. McKenney
> Sent: 04 December 2017 13:42
> On Mon, Dec 04, 2017 at 12:32:30PM +0000, David Laight wrote:
> > From: Paul E. McKenney
> > > Sent: 01 December 2017 20:09
> > >
> > > Because %p prints "(null)" and %pK prints "0000000000000000" or (on
> > > 32-bit systems) "00000000", this commit adjusts torture-test scripting
> > > accordingly.
> >
> > Surely NULL v not-NULL is one bit of info that the message needs to contain?
>
> Indeed. So the script needs to check for the strings "00000000",
> "0000000000000000", and "(null) in the console output". The "(null)"
> is what "%p" prints for a NULL pointer, and the other two strings are
> what "%pK" prints for a NULL pointer.
>
> Or am I missing your point?
I was thinking that even %pK should print "(null)".
Perhaps it should have printed a fixed, non-zero value for non-zero
pointers.
David
On Mon, Dec 04, 2017 at 02:13:38PM +0000, David Laight wrote: > From: Paul E. McKenney > > Sent: 04 December 2017 13:42 > > On Mon, Dec 04, 2017 at 12:32:30PM +0000, David Laight wrote: > > > From: Paul E. McKenney > > > > Sent: 01 December 2017 20:09 > > > > > > > > Because %p prints "(null)" and %pK prints "0000000000000000" or (on > > > > 32-bit systems) "00000000", this commit adjusts torture-test scripting > > > > accordingly. > > > > > > Surely NULL v not-NULL is one bit of info that the message needs to contain? > > > > Indeed. So the script needs to check for the strings "00000000", > > "0000000000000000", and "(null) in the console output". The "(null)" > > is what "%p" prints for a NULL pointer, and the other two strings are > > what "%pK" prints for a NULL pointer. > > > > Or am I missing your point? > > I was thinking that even %pK should print "(null)". That was my expectation, as in the need for this patch came as a surprise to me. > Perhaps it should have printed a fixed, non-zero value for non-zero > pointers. I must leave this to the people who have a dog in that contest. ;-) Thanx, Paul
On Mon, Dec 4, 2017 at 6:11 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> On Mon, Dec 04, 2017 at 02:13:38PM +0000, David Laight wrote:
>> From: Paul E. McKenney
>> > Sent: 04 December 2017 13:42
>> > On Mon, Dec 04, 2017 at 12:32:30PM +0000, David Laight wrote:
>> > > From: Paul E. McKenney
>> > > > Sent: 01 December 2017 20:09
>> > > >
>> > > > Because %p prints "(null)" and %pK prints "0000000000000000" or (on
>> > > > 32-bit systems) "00000000", this commit adjusts torture-test scripting
>> > > > accordingly.
>> > >
>> > > Surely NULL v not-NULL is one bit of info that the message needs to contain?
>> >
>> > Indeed. So the script needs to check for the strings "00000000",
>> > "0000000000000000", and "(null) in the console output". The "(null)"
>> > is what "%p" prints for a NULL pointer, and the other two strings are
>> > what "%pK" prints for a NULL pointer.
>> >
>> > Or am I missing your point?
>>
>> I was thinking that even %pK should print "(null)".
>
> That was my expectation, as in the need for this patch came as a
> surprise to me.
>
>> Perhaps it should have printed a fixed, non-zero value for non-zero
>> pointers.
>
> I must leave this to the people who have a dog in that contest. ;-)
Since there is an ongoing discussion with security people near to %pK
and alike, I added Kees and Linus to Cc list.
The proposed change can be done easily, though I have no knowledge
about possible implications.
--
With Best Regards,
Andy Shevchenko
On Sun, Dec 10, 2017 at 02:52:49PM +0200, Andy Shevchenko wrote: > On Mon, Dec 4, 2017 at 6:11 PM, Paul E. McKenney > <paulmck@linux.vnet.ibm.com> wrote: > > On Mon, Dec 04, 2017 at 02:13:38PM +0000, David Laight wrote: > >> From: Paul E. McKenney > >> > Sent: 04 December 2017 13:42 > >> > On Mon, Dec 04, 2017 at 12:32:30PM +0000, David Laight wrote: > >> > > From: Paul E. McKenney > >> > > > Sent: 01 December 2017 20:09 > >> > > > > >> > > > Because %p prints "(null)" and %pK prints "0000000000000000" or (on > >> > > > 32-bit systems) "00000000", this commit adjusts torture-test scripting > >> > > > accordingly. > >> > > > >> > > Surely NULL v not-NULL is one bit of info that the message needs to contain? > >> > > >> > Indeed. So the script needs to check for the strings "00000000", > >> > "0000000000000000", and "(null) in the console output". The "(null)" > >> > is what "%p" prints for a NULL pointer, and the other two strings are > >> > what "%pK" prints for a NULL pointer. > >> > > >> > Or am I missing your point? > >> > >> I was thinking that even %pK should print "(null)". > > > > That was my expectation, as in the need for this patch came as a > > surprise to me. > > > >> Perhaps it should have printed a fixed, non-zero value for non-zero > >> pointers. > > > > I must leave this to the people who have a dog in that contest. ;-) > > Since there is an ongoing discussion with security people near to %pK > and alike, I added Kees and Linus to Cc list. > > The proposed change can be done easily, though I have no knowledge > about possible implications. One question I have is whether the patches to convert RCU to %pK are even desirable at this point: https://lwn.net/Articles/737451/ If something like the patches in that article get to mainline, then shouldn't I just drop RCU's %pK patches? Thanx, Paul
On Sun, Dec 10, 2017 at 4:52 AM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>>
>>> Perhaps it should have printed a fixed, non-zero value for non-zero
>>> pointers.
>>
>> I must leave this to the people who have a dog in that contest. ;-)
>
> Since there is an ongoing discussion with security people near to %pK
> and alike, I added Kees and Linus to Cc list.
>
> The proposed change can be done easily, though I have no knowledge
> about possible implications.
I'd rather make %pK act more like %p than have gratuitous differences.
I also think %pK is kind of pointless in general. It has not been a
big success, and the whole "root or not" is kind of nasty anyway. Root
in a container? Things like that.
So I think that if people worry about leaking pointers, they should
primarily go for:
- just use %p and now get the hashed value
- if the hashed value is pointless, ask yourself whether the pointer
itself is important. Maybe it should be removed?
- as a last option, if you really think the true pointer value is
important, why is root so special, and maybe you should use %px and
make sure you have proper sensible permissions.
..and %pK just isn't really the answer in any of those cases.
Linus
On Sun, Dec 10, 2017 at 12:39:11PM -0800, Linus Torvalds wrote:
> On Sun, Dec 10, 2017 at 4:52 AM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> >>
> >>> Perhaps it should have printed a fixed, non-zero value for non-zero
> >>> pointers.
> >>
> >> I must leave this to the people who have a dog in that contest. ;-)
> >
> > Since there is an ongoing discussion with security people near to %pK
> > and alike, I added Kees and Linus to Cc list.
> >
> > The proposed change can be done easily, though I have no knowledge
> > about possible implications.
>
> I'd rather make %pK act more like %p than have gratuitous differences.
>
> I also think %pK is kind of pointless in general. It has not been a
> big success, and the whole "root or not" is kind of nasty anyway. Root
> in a container? Things like that.
>
> So I think that if people worry about leaking pointers, they should
> primarily go for:
>
> - just use %p and now get the hashed value
>
> - if the hashed value is pointless, ask yourself whether the pointer
> itself is important. Maybe it should be removed?
>
> - as a last option, if you really think the true pointer value is
> important, why is root so special, and maybe you should use %px and
> make sure you have proper sensible permissions.
>
> ..and %pK just isn't really the answer in any of those cases.
My main use case is comparing pointer values directly, for example,
in the console log against those in event-trace output. So if those
are hashed the same way, I wouldn't even notice.
I very rarely need to compute offsets, but I thought that the low-order
bits were excluded from the hash to make this work straightforwardly in
the common case. Of course, computing offsets could be a problem for
larger structures, by my reaction to that would be to make the kernel
do the offset computation for me as needed.
So it looks like I should drop the three patches in my tree that convert
%p to %pK.
Any objections?
Thanx, Paul
On Sun, Dec 10, 2017 at 1:47 PM, Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote: > On Sun, Dec 10, 2017 at 12:39:11PM -0800, Linus Torvalds wrote: >> I'd rather make %pK act more like %p than have gratuitous differences. The feature that paranoid folks currently depend on is getting a value entirely zeroed out with %pK (which is the least possible info leak risk). The hashed %p is almost just as good except that identical hashes are still usable to confirm matching values (but the cases where this would be useful to an attacker are hopefully approaching zero). > So it looks like I should drop the three patches in my tree that convert > %p to %pK. > > Any objections? Sounds good. If they're still useful when hashed, keep the %p. If you want to remove them because they're sensitive, just remove them instead of adding new %pK users. -Kees -- Kees Cook Pixel Security
On Mon, Dec 11, 2017 at 11:59:30AM -0800, Kees Cook wrote:
> On Sun, Dec 10, 2017 at 1:47 PM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> > On Sun, Dec 10, 2017 at 12:39:11PM -0800, Linus Torvalds wrote:
> >> I'd rather make %pK act more like %p than have gratuitous differences.
>
> The feature that paranoid folks currently depend on is getting a value
> entirely zeroed out with %pK (which is the least possible info leak
> risk). The hashed %p is almost just as good except that identical
> hashes are still usable to confirm matching values (but the cases
> where this would be useful to an attacker are hopefully approaching
> zero).
>
> > So it looks like I should drop the three patches in my tree that convert
> > %p to %pK.
> >
> > Any objections?
>
> Sounds good. If they're still useful when hashed, keep the %p. If you
> want to remove them because they're sensitive, just remove them
> instead of adding new %pK users.
OK, I have dropped those three patches.
Thanx, Paul
Linus Torvalds <torvalds@linux-foundation.org> writes: > This is a perfect example of just %pK being complete shit. > > %pK doesn't actually do any file permissions right. It looks like it does > it, but it's just a hot mess of garbage. > > And %pK doesn't even work the way you claim it does. Not in the general > case, and only with a particular value. Right. My email was only about the kptr_restrict = 1 case, but I didn't actually make that clear. But that's also sort of my point, it has multiple modes of operation, which is useful. > On Dec 11, 2017 21:26, "Michael Ellerman" <mpe@ellerman.id.au> wrote: I > > > > > > I understand that the CAP_SYSLOG checking that %pK does is kind of > > gross, but it does work in at least some useful cases like this. > > > > What am I missing? > > > Just do the damn thing right, like /proc/kallsyms does these days. > > With the proper open time cred check, not the wrong one at io time. OK, that's the piece I was missing - ie. what to do in the case where %px for all users is too permissive but %p is not useful. > Which has the added advantage that it actually does the right thing even > when you don't have kptr_restrict set, or when you have patches to make it > print zero even for people with capabilities. > > Don't depend on some random flag that has nothing to do with your actual > example and that has random values for security. > Just say no to kptr_restrict "logic". Your example basically depends > entirely on one particular setting, when (a) real distributions have a > different value and expose those pointers that your claim shouldn't be > exposed and (b) other people are pushing for values that will hide the > values that you claim area needed. I'm still a bit confused by the above. Because kallsyms which is your example of how to do it right, still uses kptr_restrict. I get that it also checks kallsyms_for_perf(), but that's only in the kptr_restrict = 0 case. Anyway, I'll do a patch for vmallocinfo to do the CAP_SYSLOG check at open time, and use that to decide if it should print 0 or the address. I can't think of any other flag or setting to sensibly determine if vmallocinfo should be visible, so I've just done: if (kptr_restrict < 2 && has_capability_noaudit(current, CAP_SYSLOG)) priv->show_addrs = true; else priv->show_addrs = false; So basically visible to root, unless kptr_restrict == 2, otherwise not visible. cheers
On Wed, Dec 13, 2017 at 4:59 AM, Michael Ellerman <mpe@ellerman.id.au> wrote: > > Right. My email was only about the kptr_restrict = 1 case, but I didn't > actually make that clear. > > But that's also sort of my point, it has multiple modes of operation, > which is useful. No it isn't. It's completely useless. Let me count the ways: - it ties a lot of completely unrelated things together in illogical ways. What if you want vmallocinfo for debuggability, but not something else? - we've had it for most of a decade, and few people use it, and it's not actually fixed any real security holes. It has only helped on particular setups, not in general. > OK, that's the piece I was missing - ie. what to do in the case where > %px for all users is too permissive but %p is not useful. Right. THAT IS THE WHOLE POINT OF THE NEW %p BEHAVIOR. Make people actually _think_ about the things they see, and hopefully just remove it entirely. Not the total idiocy that was %pK that never resulted in any actual improvement anywhere. %pK was the "sprinkle some crack on him, let's get out of here" approach to security. It's bogus shit. Don't do it. Seriously. It's been sprinkled around randomly just to make random people feel like they did something. IT DID NOTHING. The 'K' literally stands for "krack". Because spelling wasn't the strong part of the thing either. > I'm still a bit confused by the above. Because kallsyms which is your > example of how to do it right, still uses kptr_restrict. I get that it > also checks kallsyms_for_perf(), but that's only in the > kptr_restrict = 0 case. Yeah, it was probably a mistake, but I didn't want to change the old behavior. > Anyway, I'll do a patch for vmallocinfo to do the CAP_SYSLOG check at > open time, and use that to decide if it should print 0 or the address. .. don't use CAP_SYSLOG, at least without thihnking about it. That was just another mistake of mine in thinking that "let's keep the old behavior" is a good idea. Seriously, what does CAP_SYSLOG have to do with kernel address debugging? Nothing, really. I suspect CAP_SYS_ADMIN is a much saner thing to use. Ask yourself: who really should get access to vmalloc addresses? Linus