linux-kernel - Re: [PATCH v2] lib/alloc_tag: Add accumulative call counter for memory allocation profiling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <395cab19-e3b4-4581-9fe3-902e9b5d4ea7@quicinc.com>
Date: Thu, 19 Dec 2024 12:06:07 +0800
From: Zhenhua Huang <quic_zhenhuah@...cinc.com>
To: David Wang <00107082@....com>, Suren Baghdasaryan <surenb@...gle.com>
CC: <kent.overstreet@...ux.dev>, <akpm@...ux-foundation.org>,
        <linux-mm@...ck.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] lib/alloc_tag: Add accumulative call counter for
 memory allocation profiling



On 2024/12/19 10:31, David Wang wrote:
> HI,
> At 2024-12-19 02:22:53, "Suren Baghdasaryan" <surenb@...gle.com> wrote:
>> On Wed, Dec 18, 2024 at 4:49 AM David Wang <00107082@....com> wrote:
>>>
>>> Hi,
>>>
>>> I found another usage/benefit for accumulative counters:
>>>
>>> On my system, /proc/allocinfo yields about 5065 lines,  of which 2/3 lines have accumulative counter *0*.
>>> meaning no memory activities. (right?)
>>> It is quite a waste to keep those items which are *not alive yet*.
>>> With additional changes, only 1684 lines in /proc/allocinfo on my system:
>>>
>>> --- a/lib/alloc_tag.c
>>> +++ b/lib/alloc_tag.c
>>> @@ -95,8 +95,11 @@ static void alloc_tag_to_text(struct seq_buf *out, struct codetag *ct)
>>>          struct alloc_tag_counters counter = alloc_tag_read(tag);
>>>          s64 bytes = counter.bytes;
>>>
>>> +       if (counter.accu_calls == 0)
>>> +               return;
>>>          seq_buf_printf(out, "%12lli %8llu ", bytes, counter.calls);
>>>
>>>
>>> I think this is quite an improvement worth pursuing.
>>> (counter.calls could also be used to filter out "inactive" items, but
>>> lines keep disappearing/reappearing can confuse monitoring systems.)
>>
>> Please see discussion at
>> https://lore.kernel.org/all/20241211085616.2471901-1-quic_zhenhuah@quicinc.com/
> 
> Thanks for the information.
> 
>> My point is that with this change we lose information which can be
>> useful. For example if I want to analyze all the places in the kernel
>> where memory can be potentially allocated, your change would prevent
>> me from doing that
> 
> Maybe the filter can be disabled when DEBUG is on?
> 
>   > No, I disagree. Allocation that was never invoked is not the same as
>> no allocation at all. How would we know the difference if we filter
>> out the empty ones?
> 
> Totally agree with this,  I think (bytes || counter.calls)  does not make good filter. Accumulative counter is the answer. :)

hmm... it really depends on the use case IMHO. If memory consumption is 
a concern, using counter.calls should suffice. However, for 
performance-related scenarios as you stated, it's definitely better to 
use an accumulative counter."
Both of these can't address Suren's comment: "if I want to analyze all 
the places in the kernel where memory can be potentially allocated, your 
change would prevent me from doing that", but.

> 
>> If you don't want to see all the unused sites, you can filter them in
>> the userspace. I also suspect that for practical purposes you would
>> want to filter small ones (below some threshold) as well.
> 
> I have setup monitoring tool polling /proc/allocinfo every 5 seconds on my system,
> and it takes totally   ~11ms and ~100 read syscalls just read out all the content in one round,
> and with (counter.accu_calls == 0) filter, it takes totally ~4.4ms and 34 read syscalls.
>   it would be nice to have ~60% performance improvement....
> 
> 
> 
> Thanks
> David