All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Puranjay Mohan" <puranjay@kernel.org>
Cc: linux-rt-devel@lists.linux.dev, bpf@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 1/6] rcu: Make call_rcu() safe to call from any context
Date: Wed, 05 Aug 2026 12:36:49 +0000	[thread overview]
Message-ID: <20260805123650.E48571F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260805122346.269445-2-puranjay@kernel.org>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] A lockless check in rcu_defer_flush() causes rcu_barrier() to miss in-flight deferred callbacks, violating the core rcu_barrier() guarantee.
- [High] __rcu_defer_drain() creates an unbounded non-interruptible critical section, breaking PREEMPT_RT constraints and risking hard lockups.
- [Medium] KASAN stack trace recording for deferred callbacks records the asynchronous irq_work context instead of the actual call_rcu() caller.
--

--- Patch [1]: [PATCH v3 1/6] rcu: Make call_rcu() safe to call from any context ---
commit 2b52f34260090add38843ff0920806c5e56bf18c
Author: Puranjay Mohan <puranjay@kernel.org>
Subject: rcu: Make call_rcu() safe to call from any context

This commit makes call_rcu() safe to call from any context by deferring
callbacks to a per-CPU llist when interrupts are disabled. An irq_work
then re-issues these callbacks once interrupts are re-enabled, preventing
deadlocks or list corruption.

> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 96848fc1f02b8..6fd7331aebc58 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
[ ... ]
> @@ -3206,6 +3211,104 @@ __call_rcu_common(struct rcu_head *head, rcu_callback_t func, bool lazy_in)
>  	local_irq_restore(flags);
>  }
>  
> +/*
> + * Re-issue deferred callbacks straight to the enqueue so they cannot defer
> + * again.  ->defer_lock serializes the drainers: this CPU's irq_work,
> + * rcu_defer_flush() and rcutree_migrate_callbacks().
> + */
> +static void __rcu_defer_drain(struct rcu_data *rdp, bool guard)
> +{
> +	struct llist_node *node, *next;
> +	unsigned long flags;
> +
> +	if (!IS_ENABLED(CONFIG_RCU_DEFER))
> +		return;
> +
> +	raw_spin_lock_irqsave(&rdp->defer_lock, flags);
> +	if (guard)
> +		WRITE_ONCE(rdp->defer_draining, true);
> +	llist_for_each_safe(node, next, llist_del_all(&rdp->defer_head)) {
> +		struct rcu_head *head = (struct rcu_head *)node;
> +
> +		head->next = NULL;
> +		rcu_do_enqueue(head, head->func, false);
> +	}

[Severity: High]
Could holding this raw spinlock across an unbounded number of enqueues cause
latency spikes or hard lockups, especially on PREEMPT_RT?

If a bulk teardown path calls call_rcu() in a loop with interrupts disabled,
thousands of callbacks could be staged. When __rcu_defer_drain() runs, it
loops over all of them in a single uninterrupted pass. If another CPU
executes rcu_barrier() and calls __rcu_defer_drain(), it would also spin on
this raw lock with interrupts disabled for the entire duration of the drain.

> +	if (guard)
> +		WRITE_ONCE(rdp->defer_draining, false);
> +	raw_spin_unlock_irqrestore(&rdp->defer_lock, flags);
> +}
[ ... ]
> +/* Stage @head for this CPU's irq_work when call_rcu() cannot enqueue now. */
> +static void call_rcu_defer(struct rcu_head *head, rcu_callback_t func)
> +{
> +	struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
[ ... ]
> +	head->func = func;
> +	if (llist_add((struct llist_node *)head, &rdp->defer_head))
> +		irq_work_queue(&rdp->defer_work);
> +}

[Severity: Medium]
Will this delay KASAN stack trace recording to the irq_work context?

Because call_rcu_defer() stages the callback without recording the stack trace,
the trace gets recorded later by kasan_record_aux_stack(head) inside
rcu_do_enqueue(). This means any use-after-free reports might point to
the irq_work instead of the original call_rcu() caller, making debugging
more difficult.

> +/*
> + * Register pending deferred callbacks into the callback lists so a following
> + * rcu_barrier() waits for them.  This runs before rcu_barrier() scans the
> + * lists.
> + */
> +static void rcu_defer_flush(void)
> +{
> +	int cpu;
> +
> +	if (!IS_ENABLED(CONFIG_RCU_DEFER))
> +		return;
> +
> +	for_each_possible_cpu(cpu) {
> +		struct rcu_data *rdp = per_cpu_ptr(&rcu_data, cpu);
> +
> +		if (!llist_empty(&rdp->defer_head))
> +			__rcu_defer_drain(rdp, false);
> +	}
> +}

[Severity: Critical]
Can this lockless check cause rcu_barrier() to miss in-flight callbacks?

If CPU A's irq_work detaches the list via llist_del_all() in
__rcu_defer_drain(), defer_head becomes empty immediately. If CPU B
executes rcu_defer_flush() concurrently, the lockless llist_empty()
check will return true and bypass acquiring defer_lock.

Since CPU A has detached the callbacks but hasn't yet enqueued them
into cblist, CPU B might skip __rcu_defer_drain(), check a cblist that
does not yet contain the detached callbacks, and allow rcu_barrier()
to return prematurely while objects are still in use.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805122346.269445-1-puranjay@kernel.org?part=1

  reply	other threads:[~2026-08-05 12:36 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-05 12:23 [PATCH v3 0/6] rcu,srcu: Make call_rcu()/call_srcu() safe from any context Puranjay Mohan
2026-08-05 12:23 ` [PATCH v3 1/6] rcu: Make call_rcu() safe to call " Puranjay Mohan
2026-08-05 12:36   ` sashiko-bot [this message]
2026-08-05 16:49   ` Paul E. McKenney
2026-08-05 12:23 ` [PATCH v3 2/6] rcu: Make Tiny " Puranjay Mohan
2026-08-05 12:37   ` sashiko-bot
2026-08-05 12:23 ` [PATCH v3 3/6] srcu: Make call_srcu() " Puranjay Mohan
2026-08-05 12:35   ` sashiko-bot
2026-08-06 14:04   ` Zqiang
2026-08-06 14:08     ` Puranjay Mohan
2026-08-05 12:23 ` [PATCH v3 4/6] srcu: Make Tiny " Puranjay Mohan
2026-08-05 12:23 ` [PATCH v3 5/6] rcutorture: Exercise ->call() from NMI context Puranjay Mohan
2026-08-05 12:23 ` [PATCH v3 6/6] selftests/bpf: Add a call_srcu() re-entry reproducer Puranjay Mohan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260805123650.E48571F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-devel@lists.linux.dev \
    --cc=puranjay@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.