On Fri, Dec 15, 2023 at 11:09 AM Andy Shevchenko <andy.shevche...@gmail.com> wrote: > > On Fri, Dec 15, 2023 at 8:31 PM Tanzir Hasan <tanz...@google.com> wrote: > > On Fri, Dec 15, 2023 at 8:04 AM Andy Shevchenko <a...@kernel.org> wrote: > >> On Thu, Dec 14, 2023 at 09:06:12PM +0000, tanz...@google.com wrote: > > ... > > >> > +#include <linux/kernel.h> > >> > >> I highly discourage from doing that. Instead, split what is needed to > >> the separate (new) header and include that one. > > > > > > I think it would make the most sense to do this in a separate patch. > > What word-at-a-time.h needs from kernel.h is REPEAT_BYTE and to my > > knowledge, > > almost every other version of word-at-a-time.h includes kernel.h gets this > > by > > including kernel.h. A future change could be removing REPEAT_BYTE > > out of kernel.h > > Just create a patch that either moves that macro (along with upper_*() > and lower_*() APIs) to a more distinguishable header > (maybe bytes.h or words.h or wordpart.h, etc) and use it in your case > and fix others.
Andy, These are good suggestions and we should do them... ...and Tanzir only has 3 weeks left of his internship. I don't want him to get bogged down chasing build regressions from modifying the headers themselves. I think what's best for him from here through the remainder of his internship is to stay focused on applying suggestions from IWYU to just modify the #include list of .c files, and not start splitting .h files. Splitting the .h files will be the next step, and is made easier by having the codebase not have so many indirect includes (via IWYU), but we need time to soak header changes, and time Tanzir does not have. Please can we keep the suggestions focused on whether the modifications to the header includes (and the tangential cleanups) are correct? While REPEAT_BYTE has a manageable number of users, upper_* and lower_* have significantly more; I worry about moving those causing regressions. We can move them, but such changes would need significantly more soak time than this series IMO. Tanzir is also working on statistical analysis; I suspect if he analyzes include/linux/kernel.h, he can comment on whether the usage of REPEAT_BYTE is correlated with the usage of upper_* and lower_* in order to inform whether they should be grouped together or not. -- Thanks, ~Nick Desaulniers
