On 06.06.2025 12:30, Alejandro Vallejo wrote: > On Fri Jun 6, 2025 at 12:07 PM CEST, Jan Beulich wrote: >> On 06.06.2025 12:02, Alejandro Vallejo wrote: >>> On Fri Jun 6, 2025 at 8:52 AM CEST, Jan Beulich wrote: >>>> On 05.06.2025 21:47, Alejandro Vallejo wrote: >>>>> --- a/xen/common/grant_table.c >>>>> +++ b/xen/common/grant_table.c >>>>> @@ -42,8 +42,10 @@ >>>>> #include <xen/xvmalloc.h> >>>>> #include <xen/nospec.h> >>>>> #include <xsm/xsm.h> >>>>> + >>>>> #include <asm/flushtlb.h> >>>>> #include <asm/guest_atomics.h> >>>>> +#include <asm/grant_table.h> >>>>> >>>>> #ifdef CONFIG_PV_SHIM >>>>> #include <asm/guest.h> >>>>> --- a/xen/include/xen/grant_table.h >>>>> +++ b/xen/include/xen/grant_table.h >>>>> @@ -27,10 +27,6 @@ >>>>> #include <xen/rwlock.h> >>>>> #include <public/grant_table.h> >>>>> >>>>> -#ifdef CONFIG_GRANT_TABLE >>>>> -#include <asm/grant_table.h> >>>>> -#endif >>>>> - >>>>> struct grant_table; >>>>> >>>>> /* Seed a gnttab entry for Hyperlaunch/dom0less. */ >>>> >>>> The description doesn't make clear why these two files need changing. >>> >>> What sort of description? I removed a conditional include and added it to >>> one >>> of the few places it didn't include it already along with xen/grant_table.h. >>> >>> The title does say the patch removes the crossed includes in >>> asm/grant_table.h >>> and xen/grant_table.h. >>> >>> It's, I hope, self-explanatory regular spring cleanup. >> >> Then I'm sorry, to me it isn't. "Break links" has an entirely different (file >> system) meaning to me, in the common case. Plus that says what is being done, >> but not why. And it's the "why" that I'm seeking clarification on. From your >> response to my remarks on v1 I was concluding that the issue is that in a few >> places asm/grant_table.h would need including additionally. I didn't expect >> any #include to (need to) go away. > > Let me take a step back then. How about this commit message for this same > patch? > > xen/gnttab: Remove cyclic includes in xen/grant_table.h and arm's > asm/grant_table.h > > The way they currently include each other, with one of the includes being > conditional > on CONFIG_GRANT_TABLE, makes it hard to know which contents are included > when. > > Seeing how nothing in either header depends on the other, let the include > sites > include both if both are needed. > > Signed-off-by: Alejandro Vallejo <agarc...@amd.com>
While definitely better as a description, I still don't see the point of removing the conditional #include from xen/grant_table.h. Jan
