Re: [PATCH v2 01/13] mm/slab: skip kfence objects in allocation profiling

"Vlastimil Babka (SUSE)" <[email protected]>
Newsgroups org.kernel.vger.cgroups,org.kernel.vger.linux-kernel,org.kvack.linux-mm
Message-ID <[email protected]>
On 7/21/26 07:43, Harry Yoo wrote:
> On 7/20/26 11:16 PM, Vlastimil Babka (SUSE) wrote:
>> struct kfence_metadata only contains struct slabobj_ext with
>> CONFIG_MEMCG, which is then used for the "fake" slab's obj_exts field.
>> If CONFIG_MEMCG is enabled, the struct can also end up used for memory
>> allocation profiling. If CONFIG_MEMCG is disabled but profiling is
>> enabled, it will end up allocating its obj_exts via
>> prepare_slab_obj_exts_hook() and assigning them to the fake struct slab.
>> These will probably then never be freed.
>> 
>> So things sorta work, but not always in the intended and optimal way.
>> The upcoming changes to slabobj_ext layout would additionally need a
>> proper refactoring to keep working.
>> 
>> However, there's little benefit in accounting KFENCE objects. KFENCE
>> allocations are rare and there can be only CONFIG_KFENCE_NUM_OBJECTS
>> (default to 255) outstanding ones at any time. For any callsite
>> prominent enough in the memory allocation profiling stats, allocations
>> served from KFENCE will be lost in the noise.
> 
> For a similar reason I wonder if we can drop obj_exts completely.

To make it clear, you mean kfence objects wouldn't have objcg accounting and
would essentially behave as if obj_exts for them could not be allocated.

> The idea that kfence objects might have different slabobj_ext layout
> isn't really worth the extra complexity if we have very limited amount
> of kfence objects. The amount of objects that escape memcg accounting
> will be quite limited.

Indeed. I think the main point wasn't to have everything accounted, but to
avoid specially cased kfence paths.

But I think this would probably just require adding is_kfence checks to
objext allocation paths to fail them, so that's manageable. Should perhaps
allow moving some of the checks added by this patch to more generic shared
places.

> Any thoughts from KFENCE & MEMCG folks?
> 
>> Thus let's not complicate things and simply stop accounting KFENCE
>> objects in allocation profiling and skip them in the related slab hooks.
>> 
>> We also need to skip kfence objects in mark_obj_codetag_empty() in case
>> a sheaf is allocated from kfence, per earlier sashiko review.
>> 
>> Signed-off-by: Vlastimil Babka (SUSE) <[email protected]>
>> ---
>>  Documentation/mm/allocation-profiling.rst |  7 +++++++
>>  mm/slub.c                                 | 11 +++++++++++
>>  2 files changed, 18 insertions(+)
>> 
>> diff --git a/Documentation/mm/allocation-profiling.rst b/Documentation/mm/allocation-profiling.rst
>> index 5389d241176a..d02eb54ee8f2 100644
>> --- a/Documentation/mm/allocation-profiling.rst
>> +++ b/Documentation/mm/allocation-profiling.rst
>> @@ -112,3 +112,10 @@ break it out by rhashtable type.
>>  
>>  - Then, use the following form for your allocations:
>>    alloc_hooks_tag(ht->your_saved_tag, kmalloc_noprof(...))
>> +
>> +Notes
>> +=====
>> +
>> +- When a slab object is allocated from KFENCE, its accounting is skipped.
>> +  KFENCE allocations are rare and limited to a small number, so this omission
>> +  is negligible.
>> diff --git a/mm/slub.c b/mm/slub.c
>> index 0337e60db5ac..76acb78f2655 100644
>> --- a/mm/slub.c
>> +++ b/mm/slub.c
>> @@ -2076,6 +2076,11 @@ static inline void mark_obj_codetag_empty(const void *obj)
>>  		struct slabobj_ext *ext = slab_obj_ext(obj_slab,
>>  						       slab_exts, offs);
>>  
>> +		if (is_kfence_address(obj)) {
>> +			put_slab_obj_exts(slab_exts);
>> +			return;
>> +		}
> 
> I think we should check this before get_slab_obj_exts()?
> 
> Checking if the object is from kfence after slab_obj_ext_codetag_ref()
> looks bit weird.

Hm indeed.

> Otherwise LGTM.

Thanks!

>> +
>>  		if (unlikely(is_codetag_empty(&ext->ref))) {
>>  			put_slab_obj_exts(slab_exts);
>>  			return;
>> @@ -2352,6 +2357,9 @@ __alloc_tagging_slab_alloc_hook(struct kmem_cache *s, void *object, gfp_t flags,
>>  	if (alloc_flags & SLAB_ALLOC_NO_RECURSE)
>>  		return;
>>  
>> +	if (is_kfence_address(object))
>> +		return;
>> +
>>  	slab = virt_to_slab(object);
>>  	obj_exts = prepare_slab_obj_exts_hook(s, slab, flags, alloc_flags, object);
>>  	/*
>> @@ -2399,6 +2407,9 @@ __alloc_tagging_slab_free_hook(struct kmem_cache *s, struct slab *slab, void **p
>>  	for (i = 0; i < objects; i++) {
>>  		unsigned int off = obj_to_index(s, slab, p[i]);
>>  
>> +		if (is_kfence_address(p[i]))
>> +			continue;
>> +
>>  		alloc_tag_sub(&slab_obj_ext(slab, obj_exts, off)->ref, s->size);
>>  	}
>>  	put_slab_obj_exts(obj_exts);
>> 
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.