Re: [PATCH] xen/smp: Support NULL IPI function pointers

[Andrew Cooper <amc96@xxxxxxxx>]

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.cooper3@xxxxxxxxxx>
>> ---
>> CC: Jan Beulich <JBeulich@xxxxxxxx>
>> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>> CC: Wei Liu <wl@xxxxxxx>
>>
>> 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.

~Andrew