lkml.org 
[lkml]   [2021]   [Apr]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] mm: append __GFP_COMP flag for trace_malloc
On Tue, Apr 27, 2021 at 01:30:48PM +0800, Xiongwei Song wrote:
> Hi Mattew,
>
> One more thing I should explain, the kmalloc_order() appends the
> __GFP_COMP flags,
> not by the caller.
>
> void *kmalloc_order(size_t size, gfp_t flags, unsigned int order)
> {
> ...........................................................
>
> flags |= __GFP_COMP;
> page = alloc_pages(flags, order);
> ...........................................................
> return ret;
> }
> EXPORT_SYMBOL(kmalloc_order);
>
> #ifdef CONFIG_TRACING
> void *kmalloc_order_trace(size_t size, gfp_t flags, unsigned int order)
> {
> void *ret = kmalloc_order(size, flags, order);
> trace_kmalloc(_RET_IP_, ret, size, PAGE_SIZE << order, flags);
> return ret;
> }
> EXPORT_SYMBOL(kmalloc_order_trace);
> #endif

Yes, I understood that. What I don't understand is why appending the
__GFP_COMP to the trace would have been less confusing for you.

Suppose I have some code which calls:

kmalloc(10 * 1024, GFP_ATOMIC|__GFP_NOWARN|__GFP_NOMEMALLOC);

and I see in my logs

0.08% call_site=ffffffff851d0cb0 ptr=0xffff8c04a4ca0000 bytes_req=10176 bytes_alloc=16384 gfp_flags=GFP_ATOMIC|__GFP_NOWARN|__GFP_NOMEMALLOC|__GFP_COMP

That seems to me _more_ confusing because I would wonder "Where did that
__GFP_COMP come from?"

>
> Regards,
> Xiongwei
>
> On Tue, Apr 27, 2021 at 12:11 PM Xiongwei Song <sxwjean@gmail.com> wrote:
> >
> > On Tue, Apr 27, 2021 at 11:36 AM Matthew Wilcox <willy@infradead.org> wrote:
> > >
> > > On Tue, Apr 27, 2021 at 11:29:32AM +0800, Xiongwei Song wrote:
> > > > On Tue, Apr 27, 2021 at 10:54 AM Matthew Wilcox <willy@infradead.org> wrote:
> > > > >
> > > > > On Tue, Apr 27, 2021 at 10:43:20AM +0800, Xiongwei Song wrote:
> > > > > > From: Xiongwei Song <sxwjean@gmail.com>
> > > > > >
> > > > > > When calling kmalloc_order, the flags should include __GFP_COMP here,
> > > > > > so that trace_malloc can trace the precise flags.
> > > > >
> > > > > I suppose that depends on your point of view.
> > > > Correct.
> > > >
> > > > Should we report the
> > > > > flags used by the caller, or the flags that we used to allocate memory?
> > > > > And why does it matter?
> > > > When I capture kmem:kmalloc events on my env with perf:
> > > > (perf record -p my_pid -e kmem:kmalloc)
> > > > I got the result below:
> > > > 0.08% call_site=ffffffff851d0cb0 ptr=0xffff8c04a4ca0000
> > > > bytes_req=10176 bytes_alloc=16384
> > > > gfp_flags=GFP_ATOMIC|__GFP_NOWARN|__GFP_NOMEMALLOC
> > >
> > > Hmm ... if you have a lot of allocations about this size, that would
> > > argue in favour of adding a kmem_cache of 10880 [*] bytes. That way,
> > > we'd get 3 allocations per 32kB instead of 2.
> > I understand you. But I don't think our process needs this size. This size
> > may be a bug in our code or somewhere, I don't know the RC for now.
> >
> > > [*] 32768 / 3, rounded down to a 64 byte cacheline
> > >
> > > But I don't understand why this confused you. Your caller at
> > > ffffffff851d0cb0 didn't specify __GFP_COMP. I'd be more confused if
> > > this did report __GFP_COMP.
> > >
> > I just wanted to save some time when debugging.
> >
> > Regards

\
 
 \ /
  Last update: 2021-04-27 13:27    [W:0.312 / U:2.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site