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

[Harry Yoo]

On 7/21/26 6:20 PM, Vlastimil Babka (SUSE) wrote:
> 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.

...for kfence objects!

> 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.

Haha, right. Thanks for clarifying.

>> 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.

Agreed.

> Should perhaps
> allow moving some of the checks added by this patch to more generic shared
> places.

Which places did you mean by 'more generic shared places'?

-- 
Cheers,
Harry / Hyeonggon

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature