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

[Andrea Parri <parri.andrea@gmail.com>]

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.

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.

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

> >  		// 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.

> > +	/*
> > +	 * 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.

Thanks,
  Andrea