> On 18 Nov 2021, at 10:35, Andrew Cooper <am...@srcf.net> wrote:
>
> On 18/11/2021 09:58, Jan Beulich wrote:
>> On 17.11.2021 17:48, Andrew Cooper wrote:
>>> There are several cases where the act of interrupting a remote processor has
>>> the required side effect. Explicitly allow NULL function pointers so the
>>> calling code doesn't have to provide a stub implementation.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
>>> ---
>>> CC: Jan Beulich <jbeul...@suse.com>
>>> CC: Roger Pau Monné <roger....@citrix.com>
>>> CC: Wei Liu <w...@xen.org>
>>>
>>> The wait parameter is a little weird. It serves double duty and will
>>> confirm
>>> that the IPI has been taken. All it does is let you control whether you
>>> also
>>> wait for the handler to complete first. As such, it is effectively useless
>>> with a stub function.
>>>
>>> I don't particularly like folding into the .wait() path like that, but I
>>> dislike it less than an if()/else if() and adding a 3rd cpumask_clear_cpu()
>>> into the confusion which is this logic.
>> I can accept this, albeit personally I would have preferred the extra if()
>> over the goto.
>
> Just so it's been posted. This is what the if/else looks like:
>
> diff --git a/xen/common/smp.c b/xen/common/smp.c
> index 79f4ebd14502..ff569bbe9d53 100644
> --- a/xen/common/smp.c
> +++ b/xen/common/smp.c
> @@ -87,7 +87,11 @@ void smp_call_function_interrupt(void)
>
> irq_enter();
>
> - if ( call_data.wait )
> + if ( unlikely(!func) )
> + {
> + cpumask_clear_cpu(cpu, &call_data.selected);
> + }
> + else if ( call_data.wait )
> {
> (*func)(info);
> smp_mb();
>
>
> GCC does manage to fold this into the goto version presented originally.
>
> If everyone else thinks this version is clearer to read then I'll go
> with it.
Hi Andrew,
I would vote for the long version if it’s not a problem.
Cheers,
Luca
>
> ~Andrew
