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@xxxxxxxxxx> 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, 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.
