Re: [PATCH for-next v3 0/9] mm/slab: introduce kfree_rcu_nolock() and improve slub_kunit coverage

[Harry Yoo <harry@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 4275 bytes --]



On 6/16/26 5:28 AM, Alexei Starovoitov wrote:
> On Mon Jun 15, 2026 at 4:05 AM PDT, Harry Yoo (Oracle) wrote:
>> Not the best time to post a series, but didn't want to delay posting
>> the series for too long. no pressures ;)  This is aimed to be queued
>> for review and testing after the merge window closes.
>>
>> This series is based on next-20260612, and is also available on
>> git.kernel.org [3].
>>
>> To RCU folks: It would be great if you could kindly take a quick look at
>> patch 4 and either ack or nack the patch ;)
>>
>> To BPF folks: Ulad asked to share workloads to measure performance
>> of kfree_rcu_nolock(). Unfortunately, I focused more on correctness
>> and have not spent much effort on that. It would be nice if BPF folks
>> could help evaluate it on their relevant workloads.
> 
> kfree_rcu_nolock() needs to replace bpf_mem_alloc which is backbone
> of bpf maps and bpf local storage.

> So all of the selftests/bpf/benchs/run_bench_*.sh

Didn't notice it was a thing, thanks!

> will exercise it one way or the other the replacement is complete.
> In other words performance is absolutely critical.
>
>> To PREEMPT_RT folks: The most relevant part is allowing
>> kfree_rcu_sheaf() on PREEMPT_RT (patch 6). It carefully avoids sleeping
>> by acquiring the locks via local_trylock() or spin_trylock_irqsave()
>> to avoid sleeping within a raw spinlock. When trylock or unlock is
>> unsafe, kmalloc_nolock() always fails.
>>
>> Changes since RFC v2
>> ====================
>>
>> Reduced complexity and intrusiveness (Uladzislau Rezki)
>> -------------------------------------------------------
>>
>> While discussing concerns about the complexity of adding allow_spin
>> handling with Ulad (Thanks!), I realized that adding complexity to the
>> kvfree_rcu batching is not strictly necessary: only slab objects need to
>> be batched, they are already batched by rcu sheaves, and slab already
>> supports unknown context. So it is enough to implement only a minimal
>> fallback for the sheaves path.
>>
>> I tried to avoid making intrusive changes to the existing kvfree_rcu
>> path as much as possible. struct rcu_ptr is renamed to kfree_rcu_head
>> following Vlastimil's suggestion, and it is used only in the
>> kfree_rcu_nolock() path for now.
>>
>> As a result, the complexity is significantly reduced and the series
>> became much less intrusive. This is also reflected well in the diffstat
>> below.
> 
> Overall looks good to me.

Thanks!

> btw sashiko was confused in few cases.
> Not everything that it flags needs a fix. Sometimes it's not an issue at all.
> It only sounds like one.

Right, most of the comments are false positives.
But there is one comment that looks like a real bug...

Sashiko wrote:
> This is a pre-existing issue, but can concurrent lockless calls to
> deferred_work_barrier() cause an rcuwait race on PREEMPT_RT, leading to
> permanent task hangs?
>
> The function iterates over all CPUs, invoking irq_work_sync() on each
> CPU's deferred work object. On PREEMPT_RT, irq_work_sync() relies on
> rcuwait_wait_event() to block until completion,

Yes. If CONFIG_PREEPMT_RT is enabled and irq_work doesn't have
IRQ_WORK_HARD_IRQ, irq_work_sync() calls rcuwait_wait_event().

> and the rcuwait
> synchronization primitive strictly allows only one waiter at a time.

Hmm yeah, include/linux/rcuwait.h says:
| The caller is responsible for locking around rcuwait_wait_event(),
| and [prepare_to/finish]_rcuwait() such that writes to @task are
| properly serialized.

> Because deferred_work_barrier() is called without any global
> serialization (for instance, in kmem_cache_destroy() and
> kvfree_rcu_barrier_on_cache(), and now in flush_all_rcu_sheaves()),

Yes, without slab_mutex held.

> multiple threads can enter irq_work_sync() for the same work object
> concurrently.
> 
> This overwrites the waiter task pointer, meaning only one task will be
> woken up when the work completes, leaving the other tasks hanging
> permanently in an uninterruptible sleep.

I think this is indeed a pre-existing issue that needs to be resolved.
Looks like we now have one more bug to fix ;-)

-- 
Cheers,
Harry / Hyeonggon

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]