2022-05-23 06:08:54

by Vasily Averin

[permalink] [raw]
Subject: Re: [PATCH v4] tracing: add 'accounted' entry into output of allocation tracepoints

On 5/22/22 06:51, Hyeonggon Yoo wrote:
> On Sat, May 21, 2022 at 09:36:54PM +0300, Vasily Averin wrote:
>> Slab caches marked with SLAB_ACCOUNT force accounting for every
>> allocation from this cache even if __GFP_ACCOUNT flag is not passed.
>> Unfortunately, at the moment this flag is not visible in ftrace output,
>> and this makes it difficult to analyze the accounted allocations.
>>
>> This patch adds boolean "accounted" entry into trace output,
>> and set it to 'true' for calls used __GFP_ACCOUNT flag and
>> for allocations from caches marked with SLAB_ACCOUNT.
>>
>> Signed-off-by: Vasily Averin <[email protected]>
>> Acked-by: Shakeel Butt <[email protected]>
>
> May I ask what information do you want to collect
> using this patch?

I analyze ftrace output to understand which allocations are accounted.
When some userspace operation consume memory, it's important to account
most part of memory (>2/3 of all) to avoid misuse inside memcg-limited
contianers. Otherwise memcg-limited container can consume significant
portion of host memory, trigger global OOM, wake up OOM-killer and kill
random processes on host.
If memory consumers are accounted, it leads to memcg-OOM only.

Now kmem tracing output looks like this:

kmem_cache_alloc: (getname_flags.part.0+0x2c) call_site=getname_flags.part.0+0x2c ptr=0xffff8fff022e9000 bytes_req=4096 bytes_alloc=4096 gfp_flags=GFP_KERNEL accounted=false
kmalloc: (alloc_bprm+0x32) call_site=alloc_bprm+0x32 ptr=0xffff8fff2b408a00 bytes_req=416 bytes_alloc=512 gfp_flags=GFP_KERNEL|__GFP_ZERO accounted=false
kmem_cache_alloc: (mm_alloc+0x16) call_site=mm_alloc+0x16 ptr=0xffff8fff0894d500 bytes_req=1048 bytes_alloc=1088 gfp_flags=GFP_KERNEL accounted=true
mm_page_alloc: page=0xffffffffa4ab8d42 pfn=0x12ad72 order=1 migratetype=0 gfp_flags=GFP_USER|__GFP_ZERO|__GFP_ACCOUNT
kmem_cache_alloc: (vm_area_alloc+0x1a) call_site=vm_area_alloc+0x1a ptr=0xffff8fff2af27000 bytes_req=200 bytes_alloc=200 gfp_flags=GFP_KERNEL accounted=true

As you can see, without new field it is quite hard to understand,
is last allocation accounted.

This analyze helps me to identify most important allocations for given scenario
and enable accounting for selected allocations.

An example of this analyze you can found here:
https://lore.kernel.org/all/[email protected]/

> I pointed it in another thread but I'm not sure
> printing SLAB_* flags in these tracepoint is good :(

I'm not sure I understand your arguments correctly. Could you please
explain your position in more details?

> If we decide to do that, it would be better to print
> something like:
> slab_flags=SLAB_RECLAIM_ACCOUNT|SLAB_ACCOUNT|SLAB_STORE_USER
> instead of just printing 'accounted=true/false'. This patch is too
> specific to SLAB_ACCOUNT.

Any extra output degrades performance.
For my task it's not important to know SLAB flags, I just need to understand,
is current allocation accounted or not.

> And if what you want to know is just total slab memory that is accounted,
> what about adding something like SlabAccounted in /proc/meminfo?

It is not enough for me. I need to have per-process allocation information.

Thank you,
Vasily Averin


2022-05-23 06:37:10

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH v4] tracing: add 'accounted' entry into output of allocation tracepoints

On Sun, 22 May 2022 07:33:08 +0300
Vasily Averin <[email protected]> wrote:

> > slab_flags=SLAB_RECLAIM_ACCOUNT|SLAB_ACCOUNT|SLAB_STORE_USER
> > instead of just printing 'accounted=true/false'. This patch is too
> > specific to SLAB_ACCOUNT.
>
> Any extra output degrades performance.
> For my task it's not important to know SLAB flags, I just need to understand,
> is current allocation accounted or not.

If you do save the flags in the event, you can report that on output with
the __print_flags() macro:

TP_fast_assign(
[..]
__entry->sflags = s ? s->flags;
[..]
)
TP_printk("... slab_flags=%s ..",
[..]
__print_flags(sflags, "|",
{ SLAB_CONSISTENCY_CHECKS, "CONSISTENCY_CHECKS" },
{ SLAB_RED_ZONE, "RED_ZONE" },
{ SLAB_POISON, "POISON" },
{ SLAB_HWCACHE_ALIGN, "HWCACHE_ALIGN" },
{ SLAB_CACHE_DMA, "CACHE_DMA" },
{ SLAB_CACHE_DMA32, "CACHE_DMA32" },
{ SLAB_STORE_USER, "STORE_USER" },
{ SLAB_PANIC, "PANIC" }), ... )


And you get the flag output looking nicely, and all the processing is done
on the reader path.

That's if you find it useful at all.

-- Steve

2022-05-23 07:53:34

by Hyeonggon Yoo

[permalink] [raw]
Subject: Re: [PATCH v4] tracing: add 'accounted' entry into output of allocation tracepoints

On Sun, May 22, 2022 at 07:33:08AM +0300, Vasily Averin wrote:
> On 5/22/22 06:51, Hyeonggon Yoo wrote:
> > On Sat, May 21, 2022 at 09:36:54PM +0300, Vasily Averin wrote:
> >> Slab caches marked with SLAB_ACCOUNT force accounting for every
> >> allocation from this cache even if __GFP_ACCOUNT flag is not passed.
> >> Unfortunately, at the moment this flag is not visible in ftrace output,
> >> and this makes it difficult to analyze the accounted allocations.
> >>
> >> This patch adds boolean "accounted" entry into trace output,
> >> and set it to 'true' for calls used __GFP_ACCOUNT flag and
> >> for allocations from caches marked with SLAB_ACCOUNT.
> >>
> >> Signed-off-by: Vasily Averin <[email protected]>
> >> Acked-by: Shakeel Butt <[email protected]>
> >
> > May I ask what information do you want to collect
> > using this patch?
>
> I analyze ftrace output to understand which allocations are accounted.
> When some userspace operation consume memory, it's important to account
> most part of memory (>2/3 of all) to avoid misuse inside memcg-limited
> contianers. Otherwise memcg-limited container can consume significant
> portion of host memory, trigger global OOM, wake up OOM-killer and kill
> random processes on host.
> If memory consumers are accounted, it leads to memcg-OOM only.
>
> Now kmem tracing output looks like this:
>
> kmem_cache_alloc: (getname_flags.part.0+0x2c) call_site=getname_flags.part.0+0x2c ptr=0xffff8fff022e9000 bytes_req=4096 bytes_alloc=4096 gfp_flags=GFP_KERNEL accounted=false
> kmalloc: (alloc_bprm+0x32) call_site=alloc_bprm+0x32 ptr=0xffff8fff2b408a00 bytes_req=416 bytes_alloc=512 gfp_flags=GFP_KERNEL|__GFP_ZERO accounted=false
> kmem_cache_alloc: (mm_alloc+0x16) call_site=mm_alloc+0x16 ptr=0xffff8fff0894d500 bytes_req=1048 bytes_alloc=1088 gfp_flags=GFP_KERNEL accounted=true
> mm_page_alloc: page=0xffffffffa4ab8d42 pfn=0x12ad72 order=1 migratetype=0 gfp_flags=GFP_USER|__GFP_ZERO|__GFP_ACCOUNT
> kmem_cache_alloc: (vm_area_alloc+0x1a) call_site=vm_area_alloc+0x1a ptr=0xffff8fff2af27000 bytes_req=200 bytes_alloc=200 gfp_flags=GFP_KERNEL accounted=true
>
> As you can see, without new field it is quite hard to understand,
> is last allocation accounted.
>
> This analyze helps me to identify most important allocations for given scenario
> and enable accounting for selected allocations.
>
> An example of this analyze you can found here:
> https://lore.kernel.org/all/[email protected]/
>

Thank you for detailed explanation. Makes sense to me.

> > If we decide to do that, it would be better to print
> > something like:
> > slab_flags=SLAB_RECLAIM_ACCOUNT|SLAB_ACCOUNT|SLAB_STORE_USER
> > instead of just printing 'accounted=true/false'. This patch is too
> > specific to SLAB_ACCOUNT.
>
> Any extra output degrades performance.

No strong opinion but just a concern that maybe later someone want add
something similar like 'reclaimable=true/false', 'dma=true/false', ...
and I would prefer more general solution. (especially if we'll not
change tracepoints after release because of backward compatibility)

> For my task it's not important to know SLAB flags, I just need to understand,
> is current allocation accounted or not.

SLAB_ACCOUNT, SLAB_RECLAIM_ACCOUNT, SLAB_DMA, ... etc are SLAB flags.

'if current allocation is accounted or not' depends on SLAB_ACCOUNT
flag is set or not.

Thanks,
Hyeonggon

> > And if what you want to know is just total slab memory that is accounted,
> > what about adding something like SlabAccounted in /proc/meminfo?
>
> It is not enough for me. I need to have per-process allocation information.
>
> Thank you,
> Vasily Averin

2022-05-23 08:14:18

by Vasily Averin

[permalink] [raw]
Subject: Re: [PATCH v4] tracing: add 'accounted' entry into output of allocation tracepoints

On 5/22/22 23:09, Steven Rostedt wrote:
> On Sun, 22 May 2022 07:33:08 +0300
> Vasily Averin <[email protected]> wrote:
>
>>> slab_flags=SLAB_RECLAIM_ACCOUNT|SLAB_ACCOUNT|SLAB_STORE_USER
>>> instead of just printing 'accounted=true/false'. This patch is too
>>> specific to SLAB_ACCOUNT.
>>
>> Any extra output degrades performance.
>> For my task it's not important to know SLAB flags, I just need to understand,
>> is current allocation accounted or not.
>
> If you do save the flags in the event, you can report that on output with
> the __print_flags() macro:
>
> TP_fast_assign(
> [..]
> __entry->sflags = s ? s->flags;
> [..]
> )
> TP_printk("... slab_flags=%s ..",
> [..]
> __print_flags(sflags, "|",
> { SLAB_CONSISTENCY_CHECKS, "CONSISTENCY_CHECKS" },
> { SLAB_RED_ZONE, "RED_ZONE" },
> { SLAB_POISON, "POISON" },
> { SLAB_HWCACHE_ALIGN, "HWCACHE_ALIGN" },
> { SLAB_CACHE_DMA, "CACHE_DMA" },
> { SLAB_CACHE_DMA32, "CACHE_DMA32" },
> { SLAB_STORE_USER, "STORE_USER" },
> { SLAB_PANIC, "PANIC" }), ... )
>
>
> And you get the flag output looking nicely, and all the processing is done
> on the reader path.
>
> That's if you find it useful at all.

Thank you for explanation!
Yes, we can do it however I really doubt that any other slab flags are of interest to anyone.

Btw. in this form slab flags array causes sparse warnings,
because SLAB_* are defined as bitwise slab_flags_t.
This should be translated to unsigned long similarly to gfp_t flags.

Thank you,
Vasily Averin