Re: [PATCH v2] xen: move vcpu_kick() declaration to common header

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.12.2025 10:24, Oleksii Kurochko wrote:
> 
> On 12/1/25 9:40 AM, Jan Beulich wrote:
>> On 28.11.2025 17:23, Oleksii Kurochko wrote:
>>> The vcpu_kick() declaration is duplicated across multiple
>>> architecture-specific event.h headers (ARM, x86, PPC).
>>>
>>> Remove the redundant declarations and move vcpu_kick() into
>>> the common xen/include/xen/sched.h header.
>>>
>>> Drop the definition of vcpu_kick() from ppc/include/asm/event.h,
>>> as it is already provided in ppc/stubs.c.
>>>
>>> Add inclusion of xen/sched.h in the files where vcpu_kick() is
>>> used.
>>>
>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
>> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Thanks.
> 
> 
>> albeit preferably with at least ...
>>
>>> --- a/xen/arch/x86/cpu/mcheck/vmce.c
>>> +++ b/xen/arch/x86/cpu/mcheck/vmce.c
>>> @@ -12,6 +12,7 @@
>>>   #include <xen/event.h>
>>>   #include <xen/kernel.h>
>>>   #include <xen/delay.h>
>>> +#include <xen/sched.h>
>>>   #include <xen/smp.h>
>>>   #include <xen/mm.h>
>>>   #include <asm/hvm/save.h>
>> ... this change omitted. This file includes the private "mce.h", which in 
>> turn
>> includes xen/sched.h.
>>
>>> --- a/xen/arch/x86/pv/traps.c
>>> +++ b/xen/arch/x86/pv/traps.c
>>> @@ -10,6 +10,7 @@
>>>   #include <xen/event.h>
>>>   #include <xen/hypercall.h>
>>>   #include <xen/lib.h>
>>> +#include <xen/sched.h>
>>>   #include <xen/softirq.h>
>> Somewhat similarly here, xen/event.h includes xen/sched.h. That's less 
>> obviously
>> guaranteed, though, so making the include explicit here is likely okay.
> 
> I am generally okay with not adding what is probably an unnecessary new header
> inclusion, but it is unclear to me why we should avoid including a header just
> because it is already included by another one. In other words, if one day 
> someone
> removes "xen/sched.h" from "mce.h", is it acceptable for this to result in a
> compilation error? How do we decide when such an error is acceptable and when
> it is not?

This is precisely why I made the distinction between global vs private headers.
Relying on private headers to have certain #include-s is imo always okay. For
global headers that's less clear, ...

> Should the default behavior be that if header X is already included indirectly
> through other headers, there is no need to include header X directly?

... and hence I wouldn't want to give too much of a rule of thumb. One may say
that if a (global) header, to fulfill its purpose, absolutely has to include
some other header, then one can rely on this when using that header. But in
most cases the situation is somewhat blurred, so it's case-by-case decision.

What commonly happens is that #include-s are added to keep the build working
in all configurations. But #include-s typically wouldn't be added "just 
because".

Jan