Re: [PATCH] alloc_tag: introduce Kconfig option for default compressed profiling

[Usama Arif <usamaarif642@gmail.com>]

On 17/04/2025 01:15, Suren Baghdasaryan wrote:
> On Wed, Apr 16, 2025 at 2:52 PM Usama Arif <usamaarif642@gmail.com> wrote:
>>
>>
>>
>> On 16/04/2025 22:08, Suren Baghdasaryan wrote:
>>> On Wed, Apr 16, 2025 at 11:06 AM Usama Arif <usamaarif642@gmail.com> wrote:
>>>>
>>>> With this Kconfig option enabled, the kernel stores allocation tag references
>>>> in the page flags by default.
>>>>
>>>> There are 2 reasons to introduce this:
>>>> - As mentioned in [1], compressed tags dont have system memory overhead
>>>> and much lower performance overhead. It would be preferrable to have this as
>>>> the default option, and to be able to switch it at compile time. Another
>>>> option is to just declare the static key as true by default?
>>>> - As compressed option is the best one, it doesn't make sense to have to
>>>> change both defconfig and command line options to enable memory
>>>> allocation profiling. Changing commandline across a large number of services
>>>> can result in signifcant work, which shouldn't be needed if the kernel
>>>> defconfig needs to be changed anyways.
>>>
>>> The reason tag compression is not the default option is because it
>>> works only if there are enough free bits in the page flags to store a
>>> tag index. If you configure it to use page flags and your build does
>>> not have enough free bits, the profiling will be disabled (see
>>> alloc_tag_sec_init()). IOW there is no graceful fallback to use page
>>> extensions. Therefore, the current default option is not the most
>>> performant but the one which works on all builds. Instead of this just
>>> set sysctl.vm.mem_profiling boot parameter in your config file.
>>
>> Hi Suren,
> 
> Hi Usama,
> 
>>
>> Thanks for the review! The main reason is to not have to make a change in
>> both defconfig and kernel command line while deploying it. We can ofcourse
>> set the commandline as well, but just makes deployment more tedious, and
>> adds an extra commandline parameter. In our case, we only want to deploy
>> compressed tags, and if there aren't enough free bits, we would prefer to
>> disable memory allocation profiling than to take the memory and performance
>> hit.
>>
>> Would keeping the default value of this config disabled be an acceptable option?
>> i.e. the below diff on top of this patch?
> 
> Well, in that case I fail to see why
> CONFIG_MEM_ALLOC_PROFILING_COMPRESSED_ENABLED_BY_DEFAULT=y is better
> than CONFIG_CMDLINE="sysctl.vm.mem_profiling=1,compressed" ? Either
> way you need to change the config file, no?
> 

Ah ok, I always thought CONFIG_CMDLINE overrides what the kernel gets from the bootloader,
but I just tried it and it appends, as long as CONFIG_CMDLINE_OVERRIDE is not set.
Your above suggestion works as well. Thanks!