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

[Paul E. McKenney]

On Mon, Oct 19, 2026 at 06:20:03PM +0200, 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 before the writer
> and 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.  Stopped readers hold no
> hazard pointer, so the writer waits on one of the 25 deferred hazard
> pointers above (reldefer - relundefer) that nothing releases.
> 
> Keep do_pending running until hazptr_torture_cleanup() stops it, after
> the writer and the readers, releasing the deferred hazard pointers of
> all CPUs on each pass once torture_must_stop() and once more on exit.
> It now also sleeps between those passes, and kthread_stop() wakes it.
> 
> 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.
> 
> 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: 960658 relundefer
> 960658).
> 
> Fixes: f23d7b47ced9 ("hazptrtorture: Add kthread to release deferred hazard pointers")
> Assisted-by: LLM
> Signed-off-by: Andrea Parri <parri.andrea@xxxxxxxxx>

Thank you for digging into this!  Some questions and comments interspersed
below.

							Thanx, Paul

> ---
>  kernel/rcu/hazptrtorture.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/rcu/hazptrtorture.c b/kernel/rcu/hazptrtorture.c
> index 7c8b5899fb011..5153720074644 100644
> --- a/kernel/rcu/hazptrtorture.c
> +++ b/kernel/rcu/hazptrtorture.c
> @@ -547,18 +547,25 @@ static int hazptr_torture_do_pending(void *arg)
>  	DEFINE_TORTURE_RANDOM(rand);
>  
>  	VERBOSE_TOROUT_STRING("hazptr_torture_do_pending task started");
> -	do {
> -		if (stutter_will_wait()) {
> +	/*
> +	 * Run until hazptr_torture_cleanup() stops this kthread, not just
> +	 * until torture_must_stop(): the writer can be waiting in ->sync()
> +	 * for a deferred hazard pointer while it is being stopped.
> +	 */
> +	while (!kthread_should_stop()) {
> +		if (torture_must_stop() || stutter_will_wait()) {
>  			for_each_possible_cpu(cpu)
>  				hazptr_torture_do_one_pending(cpu, &rand);
>  		} else {
>  			cpu = cpumask_next_wrap(cpu, cpu_possible_mask);
>  			hazptr_torture_do_one_pending(cpu, &rand);
>  		}
> -		if (!torture_must_stop())
> -			torture_hrtimeout_ms(kthread_do_pending_ms, USEC_PER_MSEC, &rand);
> +		torture_hrtimeout_ms(kthread_do_pending_ms, USEC_PER_MSEC, &rand);

Is the theory here that a kthread_stop() will wake up this sleep anyway?
And if so, would that work if the kthread_stop() already happened?

(I do recall adding the torture_must_stop() checks due to delays at
kthread_stop() time, but perhaps things have changed since then.)

>  		// Omit stutter_wait() because this function needs to do cleanup.
> -	} while (!torture_must_stop());
> +	}
> +	/* The readers have stopped, so nothing new can be deferred. */
> +	for_each_possible_cpu(cpu)
> +		hazptr_torture_do_one_pending(cpu, &rand);

Very good!

But given these two lines, do we need the changes to the loop?

>  	torture_kthread_stopping("hazptr_torture_do_pending");
>  	return 0;
>  }
> @@ -773,7 +780,6 @@ 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);
>  
> @@ -785,6 +791,12 @@ hazptr_torture_cleanup(void)
>  		reader_tasks = NULL;
>  	}
>  
> +	/*
> +	 * Stop do_pending only after the writer and the readers: the writer
> +	 * may be waiting on hazard pointers that readers deferred.
> +	 */
> +	torture_stop_kthread(hazptr_torture_do_pending, do_pending_task);
> +

And this change looks good.

>  	torture_stop_kthread(hazptr_torture_stats, stats_task);
>  
>  	/* Do torture-type-specific cleanup operations.  */
> 
> -- 
> 2.53.0
>