Re: [PATCH v2] rcu: Use an intermediate irq_work to start process_srcu()

[Joel Fernandes <joelagnelf@nvidia.com>]

On 3/24/2026 1:36 PM, Boqun Feng wrote:
> On Tue, Mar 24, 2026 at 07:56:44AM -0700, Alexei Starovoitov wrote:
>> On Tue, Mar 24, 2026 at 4:27 AM Frederic Weisbecker <frederic@kernel.org> wrote:
>>>
>>> Le Fri, Mar 20, 2026 at 03:29:16PM -0700, Boqun Feng a écrit :
>>>> Since commit c27cea4416a3 ("rcu: Re-implement RCU Tasks Trace in terms
>>>> of SRCU-fast") we switched to SRCU in BPF. However as BPF instrument can
>>>> happen basically everywhere (including where a scheduler lock is held),
>>>> call_srcu() now needs to avoid acquiring scheduler lock because
>>>> otherwise it could cause deadlock [1]. Fix this by following what the
>>>> previous RCU Tasks Trace did: using an irq_work to delay the queuing of
>>>> the work to start process_srcu().
>>>>
>>>> [boqun: Apply Joel's feedback]
>>>> [boqun: Apply Andrea's test feedback]
>>>>
>>>> Reported-by: Andrea Righi <arighi@nvidia.com>
>>>> Closes: https://lore.kernel.org/all/abjzvz_tL_siV17s@gpd4/
>>>> Fixes: commit c27cea4416a3 ("rcu: Re-implement RCU Tasks Trace in terms of SRCU-fast")
>>>> Link: https://lore.kernel.org/rcu/3c4c5a29-24ea-492d-aeee-e0d9605b4183@nvidia.com/ [1]
>>>> Suggested-by: Zqiang <qiang.zhang@linux.dev>
>>>> Tested-by: Andrea Righi <arighi@nvidia.com>
>>>> Signed-off-by: Boqun Feng <boqun@kernel.org>
>>>
>>> I have the feeling that this problem should be solved at the BPF
>>> level. Tracepoints can fire at any time, in that sense they are like NMIs,
>>> and NMIs shouldn't acquire locks, let alone call call_rcu_*()
>>>
>>> BPF should arrange for delaying such operations to more appropriate contexts.
>>>
>>> I understand this is a regression trigerred by an RCU change but to me it
>>> rather reveals a hidden design issue rather than an API breakage.
>>
>> You all are still missing that rcu_tasks_trace was developed
>> exclusively for bpf with bpf requirements.
> 
> I'm missing this for sure. But what would be a better design?
> 
> BPF is also calling normal RCU (via call_rcu()) as well, and I don't
> think we can say normal call_rcu() was designed exclusively for BPF.
> Plus BPF heavily uses irq_work to avoid deadlocks but irq_work_queue()
> itself has a tracepoint in it (trace_ipi_send_cpu()), so in theory you
> could hit a deadlock there too.
> 
>> Then srcu_fast was introduced and then it looked like that
>> task_trace can be replaced with srcu_fast and that's where the problems
>> discovered. So either task_trace need to be resurrected
>> or srcu_fast needs to be fixed.
>> Let's punt to bpf subsystem isn't an option.
> 
> I don't think we are suggesting that, i.e. "BPF should fix its own
> issue", at least myself was hoping that we can redesign the APIs that
> BPF relies on and make it clear that it's for BPF and we can in theory
> avoid all the deadlocks (we will probably have to make some primitives
> non-traceable) and BPF can use it combined with other general
> synchronization primitives.
> 
> The current approach seems to me that we just whack-a-mole when an issue
> happens, and it's not a systematic solution.
> 
> (It doesn't have to be a problem, it could be an opportunity ;-))

The advantage of the "whack-a-mole" approach is perhaps totally benign BPF
usecases (non-tracing related even) where not using irq_work might be just
ok seems to be a win (no self-IPIs pointless, other irq_work overhead, low
latency). Probably for the best trade-offs, we need to probably deal with
it on a case-by-case basis that irq-work'ing everything randomly (?). That
said, I'm not seeing like 20 reports or something of this so perhaps for
now "case by case" is winning.

One argument could be BPF is supposed to be SAFE so anything it does like
map operations that relies on kernel APIs that take locks etc, should be
probably be done with a lot of care and extra precautions... simply because
you can insert a BPF program at any tracepoint.

Not sure where the line is, BPF is kind of an interesting usecase as in it
wants to be powerful (run in kernel context) but also be safe/verified at
the same time.

my 2 c, thanks,

--
Joel Fernandes