Re: BUG: workqueue lockup - SRCU schedules work on not-online CPUs during size transition

[Shrikanth Hegde <sshegde@linux.ibm.com>]

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 <paulmck@kernel.org>
> 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 <gor@linux.ibm.com>
>      Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
>      Tested-by: Vasily Gorbik <gor@linux.ibm.com>
>      Cc: Tejun Heo <tj@kernel.org>
> 
> 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.

>   		sdp = per_cpu_ptr(ssp->sda, get_boot_cpu_id());
>   	else
>   		sdp = raw_cpu_ptr(ssp->sda);