[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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>


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.