Re: [Xen-devel] [for-4.7] x86/emulate: synchronize LOCKed instruction emulation

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 04/14/2016 06:44 PM, Jan Beulich wrote:
>>>>> Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx> 04/14/16 7:57 AM >>>
>> On 04/14/16 07:35, Jan Beulich wrote:
>>>>>> Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx> 04/13/16 7:53 PM >>>
>>>> @@ -1589,6 +1591,8 @@ x86_emulate(
>>>      >}
>>>   >done_prefixes:
>>>  >
>>>> +    ops->smp_lock(lock_prefix);
>>>> +
>>>      >if ( rex_prefix & REX_W )
>>>          >op_bytes = 8;
>>>  
>>> Already from the context it is obvious that the lock can be taken later.
>>
>> I'll take it later.
> 
> And that alone won't suffice: Instructions not touching memory shouldn't take
> any lock at all. As shouldn't instructions that only read or only write 
> memory.
> 
>>> Overall I can see why you want this, but what is the performance impact? 
>>> After
>>> all you're basically making the emulator act like very old systems using a 
>>> bus
>>> lock (i.e. a global one) instead of utilizing cacheline exclusive 
>>> ownership. Plus
>>> you still don't (and cannot, afaict) deal with one LOCKed instruction 
>>> getting
>>> emulated and another in parallel getting executed by the CPU without 
>>> emulation
>>> (granted this shouldn't normally happen, but we also can't exclude that 
>>> case).
>>
>> I was under the impression that for reads, with the new percpu_rwlocks
>> the performance impact is close to zero, with some performance impact
>> for writes - but writes should, for one, be more infrequent, and then
>> the added safety factor should make up for that.
> 
> That's the performance effect on the hypervisor you talk about. But what about
> the guest, which all of the sudden gets another domain wide lock applied?

Well, yes, there's bound to be some performance loss - but I think the
question is: is the added safety worth it? As always, there are
trade-offs. I am quite possibly missing something, but surely a slightly
slower, running guest is better than a fast unstable one.

As for the patch, it's definitely perfectible, and I appreciate all
suggestions to make it the best it can be (or even to scrape it for a
better solution).


Thanks,
Razvan

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