Re: [Xen-devel] [PATCH 4/6] xen/hypercall: Cope with -ERESTART on more hypercall paths

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.12.2019 18:43, Andrew Cooper wrote:
> On 09/12/2019 16:29, Jan Beulich wrote:
>> On 09.12.2019 17:25, Jan Beulich wrote:
>>> On 05.12.2019 23:30, Andrew Cooper wrote:
>>>> These hypercalls each use continue_hypercall_on_cpu(), whose API is about 
>>>> to
>>>> switch to use -ERESTART.  Update the soon-to-be affected paths to cope,
>>>> folding existing contination logic where applicable.
>>>>
>>>> In addition:
>>>>  * For platform op and sysctl, insert a cpu_relax() into what is otherwise 
>>>> a
>>>>    tight spinlock loop, and make the continuation logic common at the
>>>>    epilogue.
>>> Is this really needed with a hypercall_preempt_check() invocation
>>> already in the bodies of these loops?
> 
> Yes.  The reason you're supposed to pause is to stop having memory
> traffic constantly trying to pull the spinlock's cacheline into shared
> state.
> 
> Racing round a tight loop constantly reading 4 or 5 memory locations is
> almost as bad.
> 
>> And if it's really to be added, shouldn't it be at the bottom
>> of the loop bodies rather than at the top?
> 
> It doesn't matter.

Well, modern documentation indeed gives no hint whatsoever towards
its placement. Recalling the initial guidelines from Intel (from
even before the whitepaper was made available) there was a
suggestion towards placing it close ahead of the problematic
memory access. The last example in the whitepaper looks to support
this without explicitly saying so.

Anyway, preferably with it moved
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel