Re: [Xen-devel] [PATCH for-4.12 V4] x86/altp2m: fix HVMOP_altp2m_set_domain_state race

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.02.19 at 14:51, <George.Dunlap@xxxxxxxxxx> wrote:

> 
>> On Feb 15, 2019, at 1:47 PM, Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx> wrote:
>> 
>> On 15/02/2019 13:37, George Dunlap wrote:
>>> 
>>>>> The one issue is that domain_pause_except_self() currently is actually a 
>>>>> deadlock risk if two different vcpus start it at the same time.  I think 
>>>>> the 
> 
>>>>> attached patch (compile-tested only) should fix this issue; after this 
>>>>> patch 
> 
>>>>> you should be able to use domain_pause_except_self() in 
>>>>> altp2m_set_domain_state instead.
>>>> There's one thing I don't really like here, which is a result of the
>>>> (necessary) re-use of the hypercall deadlock mutex: This
>>>> certainly poses the risk of getting called from a context where
>>>> the lock was already acquired. Therefore I'd like to suggest to
>>>> use this lock in a recursive way (here and elsewhere).
>> 
>> I can't think of a usecase were we would want to tolerate recursion on
>> the hypercall deadlock spinlock.
> 
> It sounds like Jan is specifically thinking that someone may (say) call 
> domctl_lock(), then afterwards call domain_pause_except_self().
> 
> Of course, that would deadlock immediately, so would probably get caught 
> before the patch even got to `git send-email`.

Indeed. The situation I'm worried about is if this was put on some
error path, which might never be hit in testing.

>  But it seems like a 
> reasonable thing someone might want to do.
> 
> OTOH, I’m fine leaving making it recursive until someone discovers that it 
> needs to be.

Considering Andrew's remark towards this "just" being a live lock
of a domain, I won't insist on the recursiveness, but I still think
it would be better as less error prone down the road. If left as is,
it would perhaps be a good idea to at least leave a remark in the
description, so that someone running into the problem and finding
this commit won't have to guess why it is the way it is.

Jan


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