Re: [PATCH slab/for-next v4 6/8] mm/slab: introduce struct kvfree_rcu_head for kvfree_rcu batching

[Harry Yoo <harry@kernel.org>]

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



On 7/21/26 8:06 PM, Vlastimil Babka (SUSE) wrote:
> On 7/20/26 14:44, Harry Yoo (Oracle) wrote:
>> rcu_head is overkill for kvfree_rcu() because the callback
>> function is always either kfree(), vfree(), or free_large_kmalloc(),
>> and thus there is no need for a function pointer.
>>
>> kvfree_rcu batching reuses the field to store the start address
>> of an object, however, this is not strictly needed because we can
>> calculate the start address in the slowpath. For the purpose of
>> kvfree_rcu batching, it is sufficient to implement a linked list using
>> a single pointer.
>>
>> Introduce a new struct called kvfree_rcu_head (the name was suggested
>> by Vlastimil Babka), which is similar to rcu_head but is only a single
>> pointer to build a linked list, without a function pointer, when
>> CONFIG_KVFREE_RCU_BATCHED=y.
>>
>> When kvfree_rcu is not batched, kvfree_rcu_head is the same size
>> as rcu_head. Note that shrinking struct kvfree_rcu_head on
>> CONFIG_KVFREE_RCU_BATCHED=n kernels would inevitably require additional
>> complexity and also some sort of batching (which defeats the purpose of
>> the config option) because it cannot fall back to call_rcu().
>>
>> For now there are no user-visible changes to the API. k[v]free_rcu()
>> simply casts rcu_head to kvfree_rcu_head. While this does not affect
>> the API, it allows kfree_rcu_nolock() to reuse kvfree_rcu batching
>> as a fallback when trylock or sheaf allocation fails.
>>
>> Stop storing the object pointer in rcu_head.func and instead calculate
>> the object's start address in kvfree_rcu_list(). Factor out the existing
>> logic to calculate the start address from kvfree_rcu_cb() to
>> kvmalloc_obj_start_addr().
>>
>> Signed-off-by: Harry Yoo (Oracle) <harry@kernel.org>
> 
> Nice.
> 
> Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>

Thanks!

> Nit:
> 
>> --- a/mm/slab.h
>> +++ b/mm/slab.h
>> @@ -351,6 +351,33 @@ static inline int objs_per_slab(const struct kmem_cache *cache,
>>  	return slab->objects;
>>  }
>>  
>> +/* kvfree_rcu_head offset can be only less than page size */
>> +static inline void *kvmalloc_obj_start_addr(void *head)
>> +{
>> +	void *obj = head;
>> +
>> +	if (unlikely(is_vmalloc_addr(obj))) {
>> +		obj = (void *) PAGE_ALIGN_DOWN((unsigned long)obj);
>> +	} else {
>> +		struct page *page = virt_to_page(obj);
>> +		struct slab *slab = page_slab(page);
> 
> Can use virt_to_slab().

Ah, right. We don't call free_large_kmalloc() here.

>> +
>> +		if (!slab) {
>> +			obj = (void *) PAGE_ALIGN_DOWN((unsigned long)obj);
>> +		} else if (is_kfence_address(obj)) {
>> +			obj = kfence_object_start(obj);
>> +		} else {
>> +			struct kmem_cache *s = slab->slab_cache;
>> +			unsigned int idx = __obj_to_index(s, slab_address(slab), obj);
>> +
>> +			obj = slab_address(slab) + s->size * idx;
>> +			obj = fixup_red_left(s, obj);
>> +		}
>> +	}
>> +
>> +	return obj;
>> +}
>> +
>>  /*
>>   * State of the slab allocator.
>>   *

-- 
Cheers,
Harry / Hyeonggon


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