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

[Paul E. McKenney]

On Wed, Oct 21, 2026 at 11:06:51AM +0200, Andrea Parri wrote:
> On Tue, Oct 20, 2026 at 11:49:08AM -0700, Paul E. McKenney wrote:
> > On Mon, Oct 19, 2026 at 06:20:03PM +0200, Andrea Parri wrote:
> 
> [...]
> 
> > > -		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.)
> 
> Yes, that is the theory.  torture_hrtimeout_ns() does
> set_current_state(TASK_IDLE) and then schedule_hrtimeout(), and
> kthread_stop() sets KTHREAD_SHOULD_STOP before wake_up_process(),
> which wakes TASK_IDLE sleepers.  So:
> 
>   - If kthread_stop() comes while do_pending sleeps, or between its
>     set_current_state() and schedule(), the sleep ends right away.
> 
>   - 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?

> 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?

> The next version says this in the changelog, which in the RFC only
> says "kthread_stop() wakes it".

Adding the details would be good, thank you!

> > >  		// 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?
> 
> I believe so, at least for the loop condition.  The readers are
> stopped after the writer, so during the writer's kthread_stop(), the
> writer can be in ->sync() on a hazard pointer that a still-running
> reader deferred, possibly from the iteration it was in when
> torture_must_stop() became true.  If the loop still ended at
> torture_must_stop(), do_pending would make that final pass right away
> and then wait in torture_kthread_stopping(), so a hazard pointer
> deferred after that pass would leave the writer in ->sync(), as in the
> hang above.
> 
> The other two loop changes follow from that one: the unconditional
> sleep keeps do_pending from spinning while the writer and the readers
> are being stopped, and the all-CPU pass once torture_must_stop() only
> bounds the writer's wait to one period, instead of up to nr_cpu_ids
> periods.  I'm happy to drop the latter if you prefer the simpler loop.

Should we stop the writer before stopping the readers?  Or would that
open up some other issue?

> > > +	/*
> > > +	 * 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.
> 
> Thanks.  In the next version, I'll keep the original (longer) comments
> as you suggested in the other subthread, and post this patch in one
> series with two patches that make hazptrtorture catch the slot-owner
> race, as you asked in the slot-owner thread.

The comments sound good, and the single-patch/series decision is yours,
thank you!

							Thanx, Paul