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

[Paul E. McKenney]

On Thu, Oct 22, 2026 at 02:55:55PM +0200, Andrea Parri wrote:
> On Wed, Oct 21, 2026 at 09:50:11AM -0700, Paul E. McKenney wrote:
> > On Wed, Oct 21, 2026 at 11:06:51AM +0200, Andrea Parri wrote:
> 
> [...]
> 
> > >   - If kthread_stop() has already happened, i.e., it comes between
> > >     the kthread_should_stop() check and set_current_state(), the
> > >     wakeup is lost, but KTHREAD_SHOULD_STOP stays set: do_pending
> > >     sleeps for one full period and the loop then exits.  That is
> > >     kthread_do_pending_ms plus up to 1ms of fuzz, so 3-4ms by
> > >     default.
> >
> > Very good.  Except that if someone sets a very long sleep time, that
> > could result in excessive delay for kthread exit, right?  Or am I missing
> > a turn in there somewhere?
> 
> You are not missing anything: in that case, do_pending sleeps for one
> full kthread_do_pending_ms period, however long it is.

Then let's please keep the torture_must_stop() check.  Yes, there is
still a window between the return from torture_must_stop() and the
call to torture_hrtimeout_ms(), but maybe that is something to take
care of later on.

> > > For comparison, without this patch do_pending leaves its loop at
> > > torture_must_stop() and waits in torture_kthread_stopping(), which
> > > sleeps in TASK_UNINTERRUPTIBLE for HZ/20 at a time, so it has the
> > > same lost-wakeup window with a 50ms bound instead.
> >
> > But that is a fixed wait, without the possibility of a user extending
> > it, correct?
> 
> Correct, torture_kthread_stopping() always sleeps for HZ/20 at a time.
> 
> [...]
> 
> > Should we stop the writer before stopping the readers?  Or would that
> > open up some other issue?
> 
> hazptr_torture_cleanup() already stops the writer before the readers,
> and that order is what lets the writer wait on hazard pointers that
> readers defer while it is being stopped.

OK, that is the other issue that would be opened up.  ;-)

>                                           If you mean the other way
> around, stopping the readers first, I believe that works:
> 
>   - Stop the readers, then do_pending, then the writer.
> 
>   - Keep the do_pending loop as it is in -rcu/dev, including the
>     torture_must_stop() check before the sleep, and move this patch's
>     final all-CPU pass after torture_kthread_stopping(), so that it
>     runs once the readers have stopped.
> 
> The writer can then still wait in ->sync() on a deferred hazard
> pointer, but only until that final pass, and nothing waits on the
> writer.  The readers do not need the writer to stop: they leave their
> loop at torture_must_stop(), after at most one stutter_wait().
> 
> With that order, the long sleep above can still happen, but only if
> do_pending passed its torture_must_stop() check before the test began
> stopping, and had not yet gone to sleep when kthread_stop() came,
> after all the readers had stopped.
> 
> Would you prefer that?  If so, I will rework this patch along those
> lines for v1, and rerun the tests.

Taking a more careful look, and thank you for bearing with me...

Yes, agreed, almost:

o	Keep the hazptr_torture_do_pending() do-while loop as it
	is, but add the for_each_possible_cpu() after loop exit.

o	Stop the readers first, stop hazptr_torture_do_pending()
	to flush out anything left from the readers, then stop
	the writer.

However, for the "almost", note that the "while (!torture_must_stop())"
needs to become "while (!kthread_should_stop())".  If this change is
not made, then hazptr_torture_do_pending() will exit its do-while loop
as soon as hazptr_torture_cleanup() calls torture_cleanup_begin().
This will of course defeat the reordering.

Does that work, or am I still missing something?

Alternatively, you could keep the current do-while condition and place
a loop around your post-do-while code like this:

	while (!kthread_should_stop())
		for_each_possible_cpu(cpu)
			hazptr_torture_do_one_pending(cpu, &rand);

But changing the do-while condition looks cleaner to me.

This code will also need adjustment when something like a call_hazptr()
is added, presumably along with a hazptr_barrier(), but we can worry
about that when it happens.

							Thanx, Paul