From 7fafaac5b9ce22cc57777865390520476ad2262d Mon Sep 17 00:00:00 2001 From: "Paul E. McKenney" Date: Fri, 31 Jan 2014 17:37:28 -0800 Subject: [PATCH] rcutorture: Fix rcutorture shutdown races Not all of the rcutorture kthreads waited for kthread_should_stop() before returning from their top-level functions, and none of them used torture_shutdown_absorb() properly. These problems can result in segfaults and hangs at shutdown time, and some recent changes perturbed timing sufficiently to make them much more probable. This commit therefore creates a torture_kthread_stopping() function that does the proper kthread shutdown dance in one centralized location. Accommodate this grouping by making VERBOSE_TOROUT_STRING() capable of taking a non-const string as its argument, which allows the new torture_kthread_stopping() to pass its "title" argument directly to the updated version of VERBOSE_TOROUT_STRING(). Signed-off-by: Paul E. McKenney --- include/linux/torture.h | 3 ++- kernel/rcu/rcutorture.c | 39 +++++++++++---------------------------- kernel/torture.c | 26 ++++++++++++++++++++++---- 3 files changed, 35 insertions(+), 33 deletions(-) diff --git a/include/linux/torture.h b/include/linux/torture.h index c79c41d543ef..2ea11094daf6 100644 --- a/include/linux/torture.h +++ b/include/linux/torture.h @@ -45,7 +45,7 @@ #define TOROUT_STRING(s) \ pr_alert("%s" TORTURE_FLAG s "\n", torture_type) #define VERBOSE_TOROUT_STRING(s) \ - do { if (verbose) pr_alert("%s" TORTURE_FLAG s "\n", torture_type); } while (0) + do { if (verbose) pr_alert("%s" TORTURE_FLAG " %s\n", torture_type, s); } while (0) #define VERBOSE_TOROUT_ERRSTRING(s) \ do { if (verbose) pr_alert("%s" TORTURE_FLAG "!!! " s "\n", torture_type); } while (0) @@ -88,5 +88,6 @@ void torture_init_end(void); bool torture_cleanup(void); bool torture_must_stop(void); bool torture_must_stop_irq(void); +void torture_kthread_stopping(char *title); #endif /* __LINUX_TORTURE_H */ diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c index 6e9ba51b23b9..bcaafd6cf633 100644 --- a/kernel/rcu/rcutorture.c +++ b/kernel/rcu/rcutorture.c @@ -602,12 +602,13 @@ checkwait: stutter_wait("rcu_torture_boost"); } while (!torture_must_stop()); /* Clean up and exit. */ - VERBOSE_TOROUT_STRING("rcu_torture_boost task stopping"); - torture_shutdown_absorb("rcu_torture_boost"); - while (!kthread_should_stop() || rbi.inflight) + while (!kthread_should_stop() || rbi.inflight) { + torture_shutdown_absorb("rcu_torture_boost"); schedule_timeout_uninterruptible(1); + } smp_mb(); /* order accesses to ->inflight before stack-frame death. */ destroy_rcu_head_on_stack(&rbi.rcu); + torture_kthread_stopping("rcu_torture_boost"); return 0; } @@ -638,10 +639,7 @@ rcu_torture_fqs(void *arg) } stutter_wait("rcu_torture_fqs"); } while (!torture_must_stop()); - VERBOSE_TOROUT_STRING("rcu_torture_fqs task stopping"); - torture_shutdown_absorb("rcu_torture_fqs"); - while (!kthread_should_stop()) - schedule_timeout_uninterruptible(1); + torture_kthread_stopping("rcu_torture_fqs"); return 0; } @@ -710,10 +708,7 @@ rcu_torture_writer(void *arg) rcutorture_record_progress(++rcu_torture_current_version); stutter_wait("rcu_torture_writer"); } while (!torture_must_stop()); - VERBOSE_TOROUT_STRING("rcu_torture_writer task stopping"); - torture_shutdown_absorb("rcu_torture_writer"); - while (!kthread_should_stop()) - schedule_timeout_uninterruptible(1); + torture_kthread_stopping("rcu_torture_writer"); return 0; } @@ -748,10 +743,7 @@ rcu_torture_fakewriter(void *arg) stutter_wait("rcu_torture_fakewriter"); } while (!torture_must_stop()); - VERBOSE_TOROUT_STRING("rcu_torture_fakewriter task stopping"); - torture_shutdown_absorb("rcu_torture_fakewriter"); - while (!kthread_should_stop()) - schedule_timeout_uninterruptible(1); + torture_kthread_stopping("rcu_torture_fakewriter"); return 0; } @@ -892,12 +884,9 @@ rcu_torture_reader(void *arg) schedule(); stutter_wait("rcu_torture_reader"); } while (!torture_must_stop()); - VERBOSE_TOROUT_STRING("rcu_torture_reader task stopping"); - torture_shutdown_absorb("rcu_torture_reader"); if (irqreader && cur_ops->irq_capable) del_timer_sync(&t); - while (!kthread_should_stop()) - schedule_timeout_uninterruptible(1); + torture_kthread_stopping("rcu_torture_reader"); return 0; } @@ -1010,7 +999,7 @@ rcu_torture_stats(void *arg) rcu_torture_stats_print(); torture_shutdown_absorb("rcu_torture_stats"); } while (!torture_must_stop()); - VERBOSE_TOROUT_STRING("rcu_torture_stats task stopping"); + torture_kthread_stopping("rcu_torture_stats"); return 0; } @@ -1171,12 +1160,9 @@ static int rcu_torture_barrier_cbs(void *arg) if (atomic_dec_and_test(&barrier_cbs_count)) wake_up(&barrier_wq); } while (!torture_must_stop()); - VERBOSE_TOROUT_STRING("rcu_torture_barrier_cbs task stopping"); - torture_shutdown_absorb("rcu_torture_barrier_cbs"); - while (!kthread_should_stop()) - schedule_timeout_interruptible(1); cur_ops->cb_barrier(); destroy_rcu_head_on_stack(&rcu); + torture_kthread_stopping("rcu_torture_barrier_cbs"); return 0; } @@ -1207,10 +1193,7 @@ static int rcu_torture_barrier(void *arg) n_barrier_successes++; schedule_timeout_interruptible(HZ / 10); } while (!torture_must_stop()); - VERBOSE_TOROUT_STRING("rcu_torture_barrier task stopping"); - torture_shutdown_absorb("rcu_torture_barrier"); - while (!kthread_should_stop()) - schedule_timeout_interruptible(1); + torture_kthread_stopping("rcu_torture_barrier"); return 0; } diff --git a/kernel/torture.c b/kernel/torture.c index 5e2838f902f9..330576660cf4 100644 --- a/kernel/torture.c +++ b/kernel/torture.c @@ -169,7 +169,7 @@ torture_onoff(void *arg) } schedule_timeout_interruptible(onoff_interval); } - VERBOSE_TOROUT_STRING("torture_onoff task stopping"); + torture_kthread_stopping("torture_onoff"); return 0; } @@ -370,7 +370,7 @@ static int torture_shuffle(void *arg) torture_shuffle_tasks(); torture_shutdown_absorb("torture_shuffle"); } while (!torture_must_stop()); - VERBOSE_TOROUT_STRING("torture_shuffle task stopping"); + torture_kthread_stopping("torture_shuffle"); return 0; } @@ -465,7 +465,7 @@ static int torture_shutdown(void *arg) jiffies_snap = jiffies; } if (torture_must_stop()) { - VERBOSE_TOROUT_STRING("torture_shutdown task stopping"); + torture_kthread_stopping("torture_shutdown"); return 0; } @@ -583,7 +583,7 @@ static int torture_stutter(void *arg) ACCESS_ONCE(stutter_pause_test) = 0; torture_shutdown_absorb("torture_stutter"); } while (!torture_must_stop()); - VERBOSE_TOROUT_STRING("torture_stutter task stopping"); + torture_kthread_stopping("torture_stutter"); return 0; } @@ -696,3 +696,21 @@ bool torture_must_stop_irq(void) return ACCESS_ONCE(fullstop) != FULLSTOP_DONTSTOP; } EXPORT_SYMBOL_GPL(torture_must_stop_irq); + +/* + * Each kthread must wait for kthread_should_stop() before returning from + * its top-level function, otherwise segfaults ensue. This function + * prints a "stopping" message and waits for kthread_should_stop(), and + * should be called from all torture kthreads immediately prior to + * returning. + */ +void torture_kthread_stopping(char *title) +{ + if (verbose) + VERBOSE_TOROUT_STRING(title); + while (!kthread_should_stop()) { + torture_shutdown_absorb(title); + schedule_timeout_uninterruptible(1); + } +} +EXPORT_SYMBOL_GPL(torture_kthread_stopping);