On Wed, 3 May 2023 05:54:43 -0400 Kent Overstreet <kent.overstreet@xxxxxxxxx> wrote: > On Wed, May 03, 2023 at 11:50:51AM +0200, Petr Tesařík wrote: > > On Wed, 3 May 2023 09:51:49 +0200 > > Michal Hocko <mhocko@xxxxxxxx> wrote: > > > > > On Wed 03-05-23 03:34:21, Kent Overstreet wrote: > > >[...] > > > > We've made this as clean and simple as posssible: a single new macro > > > > invocation per allocation function, no calling convention changes (that > > > > would indeed have been a lot of churn!) > > > > > > That doesn't really make the concern any less relevant. I believe you > > > and Suren have made a great effort to reduce the churn as much as > > > possible but looking at the diffstat the code changes are clearly there > > > and you have to convince the rest of the community that this maintenance > > > overhead is really worth it. > > > > I believe this is the crucial point. > > > > I have my own concerns about the use of preprocessor macros, which goes > > against the basic idea of a code tagging framework (patch 13/40). > > AFAICS the CODE_TAG_INIT macro must be expanded on the same source code > > line as the tagged code, which makes it hard to use without further > > macros (unless you want to make the source code unreadable beyond > > imagination). That's why all allocation functions must be converted to > > macros. > > > > If anyone ever wants to use this code tagging framework for something > > else, they will also have to convert relevant functions to macros, > > slowly changing the kernel to a minefield where local identifiers, > > struct, union and enum tags, field names and labels must avoid name > > conflict with a tagged function. For now, I have to remember that > > alloc_pages is forbidden, but the list may grow. > > No, we've got other code tagging applications (that have already been > posted!) and they don't "convert functions to macros" in the way this > patchset does - they do introduce new macros, but as new identifiers, > which we do all the time. Yes, new all-lowercase macros which do not expand to a single identifier are still added under include/linux. It's unfortunate IMO, but it's a fact of life. You have a point here. > This was simply the least churny way to hook memory allocations. This is a bold statement. You certainly know what you plan to do, but other people keep coming up with ideas... Like, anyone would like to tag semaphore use: up() and down()? Don't get me wrong. I can see how the benefits of code tagging, and I agree that my concerns are not very strong. I just want that the consequences are understood and accepted, and they don't take us by surprise. Petr T
