Date: Wed, 14 Feb 2024 11:24:23 -0800
From: Suren Baghdasaryan <surenb@...gle.com>
To: Kent Overstreet <kent.overstreet@...ux.dev>
Cc: Andrew Morton <akpm@...ux-foundation.org>, David Hildenbrand <david@...hat.com>, 
	Michal Hocko <mhocko@...e.com>, vbabka@...e.cz, hannes@...xchg.org, 
	roman.gushchin@...ux.dev, mgorman@...e.de, dave@...olabs.net, 
	willy@...radead.org, liam.howlett@...cle.com, corbet@....net, 
	void@...ifault.com, peterz@...radead.org, juri.lelli@...hat.com, 
	catalin.marinas@....com, will@...nel.org, arnd@...db.de, tglx@...utronix.de, 
	mingo@...hat.com, dave.hansen@...ux.intel.com, x86@...nel.org, 
	peterx@...hat.com, axboe@...nel.dk, mcgrof@...nel.org, masahiroy@...nel.org, 
	nathan@...nel.org, dennis@...nel.org, tj@...nel.org, muchun.song@...ux.dev, 
	rppt@...nel.org, paulmck@...nel.org, pasha.tatashin@...een.com, 
	yosryahmed@...gle.com, yuzhao@...gle.com, dhowells@...hat.com, 
	hughd@...gle.com, andreyknvl@...il.com, keescook@...omium.org, 
	ndesaulniers@...gle.com, vvvvvv@...gle.com, gregkh@...uxfoundation.org, 
	ebiggers@...gle.com, ytcoode@...il.com, vincent.guittot@...aro.org, 
	dietmar.eggemann@....com, rostedt@...dmis.org, bsegall@...gle.com, 
	bristot@...hat.com, vschneid@...hat.com, cl@...ux.com, penberg@...nel.org, 
	iamjoonsoo.kim@....com, 42.hyeyoo@...il.com, glider@...gle.com, 
	elver@...gle.com, dvyukov@...gle.com, shakeelb@...gle.com, 
	songmuchun@...edance.com, jbaron@...mai.com, rientjes@...gle.com, 
	minchan@...gle.com, kaleshsingh@...gle.com, kernel-team@...roid.com, 
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org, 
	iommu@...ts.linux.dev, linux-arch@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org, linux-mm@...ck.org, 
	linux-modules@...r.kernel.org, kasan-dev@...glegroups.com, 
	cgroups@...r.kernel.org
Subject: Re: [PATCH v3 00/35] Memory allocation profiling

On Wed, Feb 14, 2024 at 9:52 AM Kent Overstreet
<kent.overstreet@...ux.dev> wrote:
>
> On Wed, Feb 14, 2024 at 08:55:48AM -0800, Andrew Morton wrote:
> > On Tue, 13 Feb 2024 14:59:11 -0800 Suren Baghdasaryan <surenb@...gle.com> wrote:
> >
> > > > > If you think you can easily achieve what Michal requested without all that,
> > > > > good.
> > > >
> > > > He requested something?
> > >
> > > Yes, a cleaner instrumentation. Unfortunately the cleanest one is not
> > > possible until the compiler feature is developed and deployed. And it
> > > still would require changes to the headers, so don't think it's worth
> > > delaying the feature for years.
> >
> > Can we please be told much more about this compiler feature?
> > Description of what it is, what it does, how it will affect this kernel
> > feature, etc.
> >
> > Who is developing it and when can we expect it to become available?
> >
> > Will we be able to migrate to it without back-compatibility concerns?
> > (I think "you need quite recent gcc for memory profiling" is
> > reasonable).
> >
> >
> >
> > Because: if the maintainability issues which Michel describes will be
> > significantly addressed with the gcc support then we're kinda reviewing
> > the wrong patchset.  Yes, it may be a maintenance burden initially, but
> > at some (yet to be revealed) time in the future, this will be addressed
> > with the gcc support?
>
> Even if we had compiler magic, after considering it more I don't think
> the patchset would be improved by it - I would still prefer to stick
> with the macro approach.
>
> There's also a lot of unresolved questions about whether the compiler
> approach would even end being what we need; we need macro expansion to
> happen in the caller of the allocation function

For the record, that's what this attribute will be doing. So it should
cover our usecase.

> , and that's another
> level of hooking that I don't think the compiler people are even
> considering yet, since cpp runs before the main part of the compiler; if
> C macros worked and were implemented more like Rust macros I'm sure it
> could be done - in fact, I think this could all be done in Rust
> _without_ any new compiler support - but in C, this is a lot to ask.
>
> Let's look at the instrumentation again. There's two steps:
>
> - Renaming the original function to _noprof
> - Adding a hooked version of the original function.
>
> We need to do the renaming regardless of what approach we take in order
> to correctly handle allocations that happen inside the context of an
> existing alloc tag hook but should not be accounted to the outer
> context; we do that by selecting the alloc_foo() or alloc_foo_noprof()
> version as appropriate.
>
> It's important to get this right; consider slab object extension
> vectors or the slab allocator allocating pages from the page allocator.
>
> Second step, adding a hooked version of the original function. We do
> that with
>
> #define alloc_foo(...) alloc_hooks(alloc_foo_noprof(__VA_ARGS__))
>
> That's pretty clean, if you ask me. The only way to make it more succint
> be if it were possible for a C macro to define a new macro, then it
> could be just
>
> alloc_fn(alloc_foo);
>
> But honestly, the former is probably preferable anyways from a ctags/cscope POV.

Powered by blists - more mailing lists