Re: [PATCH -rcu/dev 1/4] hazptrtorture: Release all deferred hazard pointers before stopping

[Bradley Morgan]

On 3 November 2026 04:50:51 GMT, "Paul E. McKenney" <paulmck@xxxxxxxxxx>
wrote:
>On Wed, Oct 28, 2026 at 04:51:12PM -0700, Paul E. McKenney wrote:
>> On Wed, Oct 28, 2026 at 10:26:18AM -0700, Paul E. McKenney wrote:
>> > On Tue, Oct 27, 2026 at 05:38:03PM +0100, Andrea Parri wrote:
>> > > hazptrtorture can hang at shutdown when deferral is enabled, as in
>the
>> > > PREEMPT scenario: the writer stays in ->sync() and kthread_stop() on
>it
>> > > never returns, until kvm.sh kills QEMU:
>> > > 
>> > >     [ 1800.899278] hazptr-torture: Stopping
>hazptr_torture_do_pending task
>> > >     [ 1800.899447] hazptr-torture: Stopping hazptr_torture_writer
>task
>> > >     [ 1812.713870] hazptr-torture: rtc: 00000000225df70a ver: 12436
>HTWS_SYNC tfle: 0 rta: 12437 rtaf: 0 rtf: 12427 onoff:
>48783/48783:48786/48786 3,254:3,280 611671:731494 (HZ=1000) acq:
>5720353149 rel: 5720210186 acqirq: 571927 relirq: 714778 reldefer: 714139
>relundefer 714114
>> > >     [ 1812.713971] hazptr-torture: hazptr_torture_stats is stopping
>> > > 
>> > >     Summary: Potential hang (QEMU killed)
>> > >     !!! PID 1393523 hung at 1980 vs. 1800 seconds
>> > > 
>> > > hazptr_torture_cleanup() stops the do_pending kthread first, then
>the
>> > > writer, then the readers.  Once torture_must_stop(), do_pending
>makes
>> > > one last pass and leaves its loop, but unless stutter_will_wait(),
>that
>> > > pass releases the deferred hazard pointers of a single CPU.
>> > > 
>> > > Readers can also still defer after that pass, from the iteration
>they
>> > > are in when torture_must_stop() becomes true.  Readers that have
>left
>> > > their loop hold no hazard pointer, so the writer waits on one of the
>25
>> > > deferred hazard pointers above (reldefer - relundefer) that nothing
>> > > releases.
>> > > 
>> > > Stop the readers before do_pending, and do_pending before the
>writer.
>> > > do_pending still leaves its loop at torture_must_stop(), and now
>> > > releases the deferred hazard pointers of all CPUs once
>> > > torture_kthread_stopping() returns, that is, once
>> > > hazptr_torture_cleanup() stops it, after the readers.  The writer
>can
>> > > still wait in ->sync(), but only until then.
>> > > 
>> > > This should be safe because readers defer only from their own loop,
>> > > which they have left once torture_stop_kthread() returns for them,
>and
>> > > the hazptr_torture_acquire() IPI runs synchronously within that
>loop.
>> > > The readers do not need the writer to stop: they leave their loop at
>> > > torture_must_stop(), after at most one stutter_wait().
>> > > 
>> > > do_pending keeps the torture_must_stop() check before its sleep.  A
>> > > kthread_stop() that comes after that check and before the sleep
>starts
>> > > still delays do_pending's exit by a full kthread_do_pending_ms
>sleep,
>> > > however long that is.  With the readers stopped first, this happens
>> > > only if do_pending passed the check before the test began stopping
>and
>> > > had not yet gone to sleep when kthread_stop() came, after all the
>> > > readers had stopped.
>> > > 
>> > > With this change, a 30-minute PREEMPT run on x86_64 with 8 CPUs ends
>> > > with "End of test: SUCCESS" and with as many hazard pointers
>released
>> > > from the pending lists as were deferred (reldefer: 983727 relundefer
>> > > 983727).
>> > > 
>> > > The hang was found with the help of an LLM.
>> > > 
>> > > Fixes: f23d7b47ced9 ("hazptrtorture: Add kthread to release deferred
>hazard pointers")
>> > > Signed-off-by: Andrea Parri <parri.andrea@xxxxxxxxx>
>> > 
>> > Again, thank you!
>> > 
>> > Please see the question below.
>> > 
>> > 							Thanx, Paul
>> > 
>> > > ---
>> > >  kernel/rcu/hazptrtorture.c | 13 +++++++++++--
>> > >  1 file changed, 11 insertions(+), 2 deletions(-)
>> > > 
>> > > diff --git a/kernel/rcu/hazptrtorture.c b/kernel/rcu/hazptrtorture.c
>> > > index 7c8b5899fb011..c46083a8af093 100644
>> > > --- a/kernel/rcu/hazptrtorture.c
>> > > +++ b/kernel/rcu/hazptrtorture.c
>> > > @@ -560,6 +560,9 @@ static int hazptr_torture_do_pending(void *arg)
>> > >  		// Omit stutter_wait() because this function needs to do cleanup.
>> > >  	} while (!torture_must_stop());
>> > 
>> > The torture_must_stop() function expands to this condition:
>> > 
>> > 	torture_must_stop_irq() || kthread_should_stop()
>> > 
>> > The torture_must_stop_irq() function expands to this condition:
>> > 
>> > 	READ_ONCE(fullstop) != FULLSTOP_DONTSTOP
>> > 
>> > And the fullstop variable is set to FULLSTOP_RMMOD by
>> > torture_cleanup_begin() (unless it has already been set to
>> > FULLSTOP_SHUTDOWN).
>> > 
>> > And torture_cleanup_begin() is invoked by hazptr_torture_cleanup()
>> > right at the beginning, before reaching any of the calls to
>> > torture_stop_kthread().
>> > 
>> > So doesn't the above torture_must_stop() instead need to be
>> > kthread_should_stop()?
>> > 
>> > As it is, hazptr_torture_do_pending() will still stop before the
>> > hazptr_torture_reader() instances do, correct?
>> > 
>> > Or am I missing something subtle here?
>> 
>> Well, one thing that I am missing is that changing from
>> torture_must_stop() to kthread_should_stop() might cause trouble
>> if rcutorture execution is terminated by system shutdown.
>> 
>> So my next suggestion is that this be added just after
>> hazptr_torture_reader() exits its do-while loop:
>> 
>> 	for_each_possible_cpu(cpu)
>> 		hazptr_torture_do_one_pending(cpu, &rand);
>> 
>> Thoughts?
>
>How about like this?
>
>						Thanx, Paul
>
>------------------------------------------------------------------------
>
>commit 863f3c68c5dce601456368a9152de0239421cef9
>Author: Andrea Parri <parri.andrea@xxxxxxxxx>
>Date:   Tue Oct 27 17:38:03 2026 +0100
>
>    hazptrtorture: Release all deferred hazard pointers before stopping
>    
>    hazptrtorture can hang at shutdown when deferral is enabled, as in the
>    PREEMPT scenario: the writer stays in ->sync() and kthread_stop() on it
>    never returns, until kvm.sh kills QEMU:
>    
>        [ 1800.899278] hazptr-torture: Stopping hazptr_torture_do_pending task
>        [ 1800.899447] hazptr-torture: Stopping hazptr_torture_writer task
>        [ 1812.713870] hazptr-torture: rtc: 00000000225df70a ver: 12436 HTWS_SYNC tfle: 0 rta: 12437 rtaf: 0 rtf: 12427 onoff: 48783/48783:48786/48786 3,254:3,280 611671:731494 (HZ=1000) acq: 5720353149 rel: 5720210186 acqirq: 571927 relirq: 714778 reldefer: 714139 relundefer 714114
>        [ 1812.713971] hazptr-torture: hazptr_torture_stats is stopping
>    
>        Summary: Potential hang (QEMU killed)
>        !!! PID 1393523 hung at 1980 vs. 1800 seconds
>    
>    hazptr_torture_cleanup() stops the kthreads in random order, which
>    means that there could be a reader thread still running after
>    the do_pending kthread does its last cleanup pass.  If the writer
>    is still running at that point, any remaining deferred hazard
>    pointers will never be cleaned up and the writer can wait forever
>    if it is unlucky enough to be waiting on one of the deferred
>    hazard poiners.
>    
>    Therefore, make each reader clean up all deferred hazard pointers,
>    which means that after the last reader finishes, there can no
>    longer be any deferred hazard pointers, which means that the
>    writer can terminate promptly.
>    
>    With this change, a 30-minute PREEMPT run on x86_64 with 8 CPUs ends
>    with "End of test: SUCCESS" and with as many hazard pointers released
>    from the pending lists as were deferred (reldefer: 983727 relundefer
>    983727).
>    
>    The hang was found with the help of an LLM.
>    
>    [ paulmck: Make readers release deferred hazard pointers. ]
>    
>    Fixes: f23d7b47ced9 ("hazptrtorture: Add kthread to release deferred hazard pointers")
>    Signed-off-by: Andrea Parri <parri.andrea@xxxxxxxxx>
>    Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxx>
>
>diff --git a/kernel/rcu/hazptrtorture.c b/kernel/rcu/hazptrtorture.c
>index 7c8b5899fb011..3059ccb01979d 100644
>--- a/kernel/rcu/hazptrtorture.c
>+++ b/kernel/rcu/hazptrtorture.c
>@@ -449,6 +449,28 @@ static void hazptr_torture_defer(struct hazptr_pending *hppp, struct torture_ran
> 	atomic_long_inc(per_cpu_ptr(&hazptr_torture_releases_defer, raw_smp_processor_id()));
> }
> 
>+/*
>+ * Release the specified CPU's set of deferred/pending hazard pointers.
>+ */
>+static void hazptr_torture_do_one_pending(int cpu, struct torture_random_state *trsp)
>+{
>+	struct hazptr_pending *hppp;
>+	struct hazptr_pending *hppp1;
>+	struct llist_head *llhp;
>+	struct llist_node *llnp;
>+
>+	llhp = per_cpu_ptr(&hazptr_pending, cpu);
>+	llnp = llist_del_all(llhp);
>+	if (!llnp)
>+		return;
>+	llist_for_each_entry_safe(hppp, hppp1, llnp, hpp_node) {
>+		hazptr_torture_reader_tail(hppp, trsp);
>+		atomic_long_inc(per_cpu_ptr(&hazptr_torture_releases_undefer,
>+					    raw_smp_processor_id()));
>+		kfree(hppp);
>+	}
>+}
>+
> /*
>  * Hazard-pointer torture reader kthread.  Repeatedly dereferences
>  * hazptr_torture_current, incrementing the corresponding element of the
>@@ -513,31 +535,13 @@ static int hazptr_torture_reader(void *arg)
> 		stutter_wait("hazptr_torture_reader");
> 	} while (!torture_must_stop());
> 	torture_kthread_stopping("hazptr_torture_reader");
>+	// Each reader clears all CPUs, so nothing is left after all
>+	// readers exit their for_each_possible_cpu() loops.
>+	for_each_possible_cpu(cpu)
>+		hazptr_torture_do_one_pending(cpu, &rand);
> 	return 0;
> }
> 
>-/*
>- * Release the specified CPU's set of deferred/pending hazard pointers.
>- */
>-static void hazptr_torture_do_one_pending(int cpu, struct torture_random_state *trsp)
>-{
>-	struct hazptr_pending *hppp;
>-	struct hazptr_pending *hppp1;
>-	struct llist_head *llhp;
>-	struct llist_node *llnp;
>-
>-	llhp = per_cpu_ptr(&hazptr_pending, cpu);
>-	llnp = llist_del_all(llhp);
>-	if (!llnp)
>-		return;
>-	llist_for_each_entry_safe(hppp, hppp1, llnp, hpp_node) {
>-		hazptr_torture_reader_tail(hppp, trsp);
>-		atomic_long_inc(per_cpu_ptr(&hazptr_torture_releases_undefer,
>-					    raw_smp_processor_id()));
>-		kfree(hppp);
>-	}
>-}
>-
> /*
>  * Hazard-pointer release of deferred/pending hazard pointers.
>  */
>@@ -560,6 +564,7 @@ static int hazptr_torture_do_pending(void *arg)
> 		// Omit stutter_wait() because this function needs to do cleanup.
> 	} while (!torture_must_stop());
> 	torture_kthread_stopping("hazptr_torture_do_pending");
>+	// Readers clean up after themselves before exit.
> 	return 0;
> }
> 
>@@ -773,9 +778,7 @@ hazptr_torture_cleanup(void)
> 		return;
> 	}
> 
>-	torture_stop_kthread(hazptr_torture_do_pending, do_pending_task);
> 	torture_stop_kthread(hazptr_torture_preempt, preempt_task);
>-	torture_stop_kthread(hazptr_torture_writer, writer_task);
> 
> 	if (reader_tasks) {
> 		for (i = 0; i < nrealreaders; i++)
>@@ -785,6 +788,14 @@ hazptr_torture_cleanup(void)
> 		reader_tasks = NULL;
> 	}
> 
>+	/*
>+	 * Stop the writer only after do_pending, and do_pending only after
>+	 * the readers: the writer may be waiting on hazard pointers that
>+	 * readers deferred, which do_pending releases once they stopped.
>+	 */
>+	torture_stop_kthread(hazptr_torture_do_pending, do_pending_task);
>+	torture_stop_kthread(hazptr_torture_writer, writer_task);
>+
> 	torture_stop_kthread(hazptr_torture_stats, stats_task);
> 
> 	/* Do torture-type-specific cleanup operations.  */


This looks okay to me.

--- Thanks!