On Mon, May 08, 2023 at 08:59:39PM +0200, Petr Tesařík wrote: > On Mon, 8 May 2023 12:28:52 -0400 > Kent Overstreet <kent.overstreet@xxxxxxxxx> wrote: > > > On Mon, May 08, 2023 at 06:09:13PM +0200, Petr Tesařík wrote: > > > Sure, although AFAIK the index does not cover all possible config > > > options (so non-x86 arch code is often forgotten). However, that's the > > > less important part. > > > > > > What do you do if you need to hook something that does conflict with an > > > existing identifier? > > > > As already happens in this patchset, rename the other identifier. > > > > But this is C, we avoid these kinds of conflicts already because the > > language has no namespacing > > This statement is not accurate, but I agree there's not much. Refer to > section 6.2.3 of ISO/IEC9899:2018 (Name spaces of identifiers). > > More importantly, macros also interfere with identifier scoping, e.g. > you cannot even have a local variable with the same name as a macro. > That's why I dislike macros so much. Shadowing a global identifier like that would at best be considered poor style, so I don't see this as a major downside. > But since there's no clear policy regarding macros in the kernel, I'm > merely showing a downside; it's perfectly fine to write kernel code > like this as long as the maintainers agree that the limitation is > acceptable and outweighed by the benefits. Macros do have lots of tricky downsides, but in general we're not shy about using them for things that can't be done otherwise; see wait_event(), all of tracing... I think we could in general do a job of making the macros _themselves_ more managable, when writing things that need to be macros I'll often have just the wrapper as a macro and write the bulk as inline functions. See the generic radix tree code for example. Reflection is a major use case for macros, and the underlying mechanism here - code tagging - is something worth talking about more, since it's codifying something that's been done ad-hoc in the kernel for a long time and something we hope to refactor other existing code to use, including tracing - I've got a patch already written to convert the dynamic debug code to code tagging; it's a nice -200 loc cleanup. Regarding the alloc_hooks() macro itself specifically, I've got more plans for it. I have another patch series after this one that implements code tagging based fault injection, which is _far_ more ergonomic to use than our existing fault injection capabilities (and this matters! Fault injection is a really important tool for getting good test coverage, but tools that are a pain in the ass to use don't get used) - and with the alloc_hooks() macro already in place, we'll be able to turn _every individual memory allocation callsite_ into a distinct, individually selectable fault injection point - which is something our existing fault injection framework attempts at but doesn't really manage. If we can get this in, it'll make it really easy to write unit tests that iterate over every memory allocation site in a given module, individually telling them to fail, run a workload until they hit, and verify that the code path being tested was executed. It'll nicely complement the fuzz testing capabilities that we've been working on, particularly in filesystem land.
