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. FWIW I can see some occurences of "alloc_pages" under arch/ which are not renamed by patch 19/40 of this series. For instance, does the kernel build for s390x after applying the patch series? New code may also work initially, but explode after adding an #include later... HOWEVER, if the rest of the community agrees that the added value of code tagging is worth all these potential risks, I can live with it. Petr T
