On 7/22/26 7:05 PM, Vlastimil Babka (SUSE) wrote:
> On 7/22/26 10:15, Harry Yoo wrote:
>> On 7/21/26 10:09 PM, Vlastimil Babka (SUSE) wrote:
>>> On 7/20/26 14:44, Harry Yoo (Oracle) wrote:
>>>> Currently, k[v]free_rcu() cannot be called in unknown context since
>>>> it could lead to a deadlock when called in the middle of k[v]free_rcu().
>>>>
>>>> Make users' lives easier by introducing kfree_rcu_nolock() variant,
>>>> now that kfree_rcu_sheaf() is available on PREEMPT_RT and
>>>> __kfree_rcu_sheaf() handles unknown context.
>>>>
>>>> kfree_rcu_nolock() falls back to the kvfree_rcu batching when sheaves
>>>> path fails.
>>>
>>> From the code I would say it falls back to defer_kfree_rcu(), which is an
>>> irq_work that does kvfree_rcu batching?
>>
>> Indeed that was what I intended to say, but I messed up while
>> writing it :-) Will fix that.
>
> Thanks!
I'm the one to thank!
>>> Otherwise the sentence would contradict the following:
>>>
>>>> In most cases, the sheaves path is expected to succeed
>>>> and it's unnecessary to add complexity to the existing kvfree_rcu
>>>> batching by teaching it how to handle unknown context.
>>>
>>> This talks about trying to perform the batching immediately without
>>> defering.
>>
>> You mean the earlier sentence "kfree_rcu_nolock() falls back to the
>> kvfree_rcu batching when sheaves path fails", right?
>
> I was trying to point out how it contradicts the wording above. To be clear,
> with above fixed, this part is fine.
Ah, Ack.
>>>> Since defer_kfree_rcu() can be called on caches without sheaves, move
>>>> deferred_work_barrier() and rcu_barrier() outside the branch in
>>>> kvfree_rcu_barrier_on_cache().
>>>>
>>>> Signed-off-by: Harry Yoo (Oracle) <[email protected]>
>>>
>>> Other than that and a comment correction below, LGTM.
>>>
>>> Reviewed-by: Vlastimil Babka (SUSE) <[email protected]>
>>
>> Thanks!
>>
>>>> ---
>>>> include/linux/rcupdate.h | 19 +++++++++++++++++++
>>>> mm/slab_common.c | 28 ++++++++++++++++++++++++++--
>>>> mm/slub.c | 24 +++++++++++++++++++++++-
>>>> 3 files changed, 68 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
>>>> index ef5bb6981133..ce0fd401352e 100644
>>>> --- a/include/linux/rcupdate.h
>>>> +++ b/include/linux/rcupdate.h
>>>> @@ -1099,6 +1099,7 @@ static inline void rcu_read_unlock_migrate(void)
>>>> * In mm/slab_common.c, no suitable header to include here.
>>>> */
>>>> void kvfree_call_rcu(struct kvfree_rcu_head *head, void *ptr);
>>>> +void kfree_call_rcu_nolock(struct kvfree_rcu_head *head, void *ptr);
>>>>
>>>> /*
>>>> * The BUILD_BUG_ON() makes sure the rcu_head offset can be handled. See
>>>> the
>>>> @@ -1124,6 +1125,24 @@ do {
>>>> \
>>>> kvfree_call_rcu(NULL, (void *) (___p)); \
>>>> } while (0)
>>>>
>>>> +/**
>>>> + * kfree_rcu_nolock() - a version of kfree_rcu() that can be called in
>>>> any context.
>>>> + * @ptr: pointer to kfree for double-argument invocations.
>>>> + * @kvrhf: the name of the struct kvfree_rcu_head within the type of @ptr.
>>>> + *
>>>> + * Objects that are not allocated via kmalloc_nolock() are not supported.
>>>
>>> Is this true? If __kfree_rcu_sheaf() succeeds, rcu_sheaf is either flushed
>>> normally (call_rcu with rcu_free_sheaf()) or via the irq_work doing the
>>> same. The defer_kfree_rcu() fallback ends up doing a normal
>>> kvfree_call_rcu(). I don't see any path that ends up doing kfree_nolock().
>>> Am I missing something?
>>
>> I added this limitation because I thought it doesn't make sense
>> to support this when we don't support large kmalloc and vmalloc in
>> unknown context.
>>
>> Perhaps too limiting than it should be though.
>
> Hmm right. Maybe we could just state that it handles all small kmalloc() and
> kmalloc_nolock() allocations without deferring as much as possible, but
> large k[v]malloc() allocations that end up being large_kmalloc() or
> vmalloc(), will also work but be always deferred?
That sounds good! Will do.
>>>> + * kfree_rcu_nolock() supports 2-arg variant only.
>>>> + */
>>>> +#define kfree_rcu_nolock(ptr, kvrhf)
>>>> \
>>>> +do {
>>>> \
>>>> + typeof (ptr) ___p = (ptr);
>>>> \
>>>> +
>>>> \
>>>> + if (___p) {
>>>> \
>>>> + BUILD_BUG_ON(offsetof(typeof(*(ptr)), kvrhf) >= 4096);
>>>> \
>>>> + kfree_call_rcu_nolock(&((___p)->kvrhf), (void *) (___p));
>>>> \
>>>> + }
>>>> \
>>>> +} while (0)
>>>> +
>>>> /*
>>>> * Place this after a lock-acquisition primitive to guarantee that
>>>> * an UNLOCK+LOCK pair acts as a full barrier. This guarantee applies
--
Cheers,
Harry / Hyeonggon
