Re: [PATCH 2/2] x86: package up context switch hook pointers

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 15/02/17 08:42, Jan Beulich wrote:
>>>> On 14.02.17 at 16:26, <andrew.cooper3@citrix.com> wrote:
>> On 14/02/17 10:29, Jan Beulich wrote:
>>> @@ -2066,6 +2073,15 @@ static void __context_switch(void)
>>>      per_cpu(curr_vcpu, cpu) = n;
>>>  }
>>>  
>>> +/*
>>> + * Schedule tail *should* be a terminal function pointer, but leave a bugframe
>>> + * around just incase it returns, to save going back into the context
>>> + * switching code and leaving a far more subtle crash to diagnose.
>>> + */
>>> +#define schedule_tail(vcpu) do {                          \
>>> +        (((vcpu)->domain->arch.ctxt_switch->tail)(vcpu)); \
>>> +        BUG();                                            \
>>> +    } while (0)
>> schedule_tail() is used only twice.  I'd suggest dropping it entirely
>> and calling the ->tail() function pointer normally, rather than hiding
>> it this.
> I had considered this too, and now that you ask for it I'll happily
> do so.

Thinking more, it would be a good idea to annotate the respective
functions noreturn, so the compiler will catch anyone who accidently
puts a return statement in.

>
>> Upon reviewing, this patch, don't we also need a ->same() call in the
>> continue_same() path in the previous patch?
> No, I did specifically check this already: The call to continue_same()
> sits (far) ahead of the clearing of ->is_running, and as long as that
> flag is set, vcpu_pause() (or vcpu_sleep_sync(), to be precise) will
> spin as necessary.

Ok.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel