Re: [Xen-devel] [PATCH v3] xen: introduce VCPUOP_register_runstate_phys_memory_area hypercall

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.06.19 at 14:32, <andrii.anisov@xxxxxxxxx> wrote:
> Jan, Julien,
> 
> On 11.06.19 12:10, Jan Beulich wrote:
>>>> At the very least such loops want a cpu_relax() in their bodies.
>>>> But this being on a hypercall path - are there theoretical guarantees
>>>> that a guest can't abuse this to lock up a CPU?
>>> Hmmm, I suggested this but it looks like a guest may call the hypercall 
> multiple
>>> time from different vCPU. So this could be a way to delay work on the CPU.
>>>
>>> I wanted to make the context switch mostly lockless and therefore avoiding 
> to
>>> introduce a spinlock.
>> 
>> Well, constructs like the above are trying to mimic a spinlock
>> without actually using a spinlock. There are extremely rare
>> situation in which this may indeed be warranted, but here it
>> falls in the common "makes things worse overall" bucket, I
>> think. To not unduly penalize the actual update paths, I think
>> using a r/w lock would be appropriate here.
> 
> So what is the conclusion here? Should we go with trylock and 
> hypercall_create_continuation() in order to avoid locking but still not fail 
> to the guest?

I'm not convinced a "trylock" approach is needed - that's
something Julien suggested. I'm pretty sure we're acquiring other
locks in hypercall context without going the trylock route. I am
convinced though that the pseudo-lock you've used needs to be
replaced by a real (and perhaps r/w) one, _if_ there is any need
for locking in the first place.

Jan



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