On Thu, Apr 30, 2026 at 12:38:16PM +0530, Shrikanth Hegde wrote:
> Hi Paul.
>
> On 4/29/26 11:31 PM, Paul E. McKenney wrote:
>
> > > That mask = ~0 is really looks uncomfortable to me. What does it mean?
> > > It might end up even sending to non possible CPUs without proper checks.
> > >
> > > It should use either cpumask_setall? or use cpu_online_mask?
> > >
> > > Your current patch rcu_cpu_beenfullyonline indicates that code around
> > > srcu_schedule_cbs_sdp handles hotplug already right?
> > > in that case, just setting mask = cpu_online_mask would work?
> >
> > Agreed. Which is why I have this commit queued:
> >
> > f8d5aaaf90f8 ("srcu: Don't queue workqueue handlers to never-online CPUs")
> >
> > This is currently slated for the upcoming merge window, but if you
> > need it sooner, please let us know. Please see the end of this email
> > for the full commit.
> >
> >
> > Thanx, Paul
> >
> > > > /**
> > > > * queue_work_on - queue work on specific cpu
> > > > * @cpu: CPU number to execute work on
> > > > * @wq: workqueue to use
> > > > * @work: work to queue
> > > > *
> > > > * We queue the work to a specific CPU, the caller must ensure it
> > > > * can't go away. Callers that fail to ensure that the specified
> > > > * CPU cannot go away will execute on a randomly chosen CPU.
> > > > * But note well that callers specifying a CPU that never has been
> > > > * online will get a splat.
> > > > *
> > > > * Return: %false if @work was already on a queue, %true otherwise.
> > > > */
> > >
> > >
> > > In that case, making offline CPUs have a unbound workqueue is wrong. no?
> > >
> > > It might encourage more users to abuse queue_work_on interface to
> > > send to offline CPUs without any checks and onus now falls onto
> > > workqueue to disaptch to unbound wqs.
> > >
> > > So I think it is better to put the guardrails in SRCU instead of any
> > > change in
> > > workqueue.
> >
> > ------------------------------------------------------------------------
> >
> > commit f8d5aaaf90f8294890802ce8dccbafd9850ac5f9
> > Author: Paul E. McKenney <[email protected]>
> > Date: Thu Apr 9 11:16:02 2026 -0700
> >
> > srcu: Don't queue workqueue handlers to never-online CPUs
> > While an srcu_struct structure is in the midst of switching from CPU-0
> > to all-CPUs state, it can attempt to invoke callbacks for CPUs that
> > have never been online. Worse yet, it can attempt in invoke callbacks
> > for CPUs that never will be online due to not being present in the
>
> for CPUs that never will be online due to being present in the
> cpu_possible_mask?
>
> > cpu_possible_mask. This can cause hangs on s390, which is not set up
> > to
> > deal with workqueue handlers being scheduled on such CPUs. This commit
> > therefore causes Tree SRCU to refrain from queueing workqueue handlers
> > on CPUs that have not yet (and might never) come online.
> > Because callbacks are not invoked on CPUs that have not been
> > online, it is an error to invoke call_srcu(), synchronize_srcu(), or
> > synchronize_srcu_expedited() on a CPU that is not yet fully online.
> > However, it turns out to be less code to redirect the callbacks
> > from too-early invocations of call_srcu() than to warn about such
> > invocations. This commit therefore also redirects callbacks queued on
> > not-yet-fully-online CPUs to the boot CPU.
> > Reported-by: Vasily Gorbik <[email protected]>
> > Signed-off-by: Paul E. McKenney <[email protected]>
> > Tested-by: Vasily Gorbik <[email protected]>
> > Cc: Tejun Heo <[email protected]>
> >
> > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> > index 0d01cd8c4b4a7b..7c2f7cc131f7ae 100644
> > --- a/kernel/rcu/srcutree.c
> > +++ b/kernel/rcu/srcutree.c
> > @@ -897,11 +897,9 @@ static void srcu_schedule_cbs_snp(struct srcu_struct
> > *ssp, struct srcu_node *snp
> > {
> > int cpu;
> > - for (cpu = snp->grplo; cpu <= snp->grphi; cpu++) {
> > - if (!(mask & (1UL << (cpu - snp->grplo))))
> > - continue;
> > - srcu_schedule_cbs_sdp(per_cpu_ptr(ssp->sda, cpu), delay);
> > - }
> > + for (cpu = snp->grplo; cpu <= snp->grphi; cpu++)
> > + if ((mask & (1UL << (cpu - snp->grplo))) &&
> > rcu_cpu_beenfullyonline(cpu))
> > + srcu_schedule_cbs_sdp(per_cpu_ptr(ssp->sda, cpu),
> > delay);
> > }
> > /*
> > @@ -1322,7 +1320,9 @@ static unsigned long srcu_gp_start_if_needed(struct
> > srcu_struct *ssp,
> > */
> > idx = __srcu_read_lock_nmisafe(ssp);
> > ss_state = smp_load_acquire(&ssp->srcu_sup->srcu_size_state);
> > - if (ss_state < SRCU_SIZE_WAIT_CALL)
> > + // If !rcu_cpu_beenfullyonline(), interrupts are still disabled,
> > + // so no migration is possible in either direction from this CPU.
> > + if (ss_state < SRCU_SIZE_WAIT_CALL ||
> > !rcu_cpu_beenfullyonline(raw_smp_processor_id()))
>
> How can this happen? To get a CPU offline in raw_smp_processor_id() you need
> to run on the offline
> CPU.
CPUs run for a surprisingly long time before they get around to marking
themselves online. If a CPU invokes call_srcu() during this time,
this code really will be running on a CPU that is marked as offline.
Now, my initial thought was to instead splat if this happened, but it
turned out to require more code to reliably splat than to just handle
the situation correctly. So here we are! ;-)
Thanx, Paul
> > sdp = per_cpu_ptr(ssp->sda, get_boot_cpu_id());
> > else
> > sdp = raw_cpu_ptr(ssp->sda);
>
