Re: [Xen-devel] [PATCH 1/2] xen/rwlocks: call preempt_disable() when taking a rwlock

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.03.2020 11:15, Jürgen Groß wrote:
> On 13.03.20 11:02, Julien Grall wrote:
>> Hi Juergen,
>>
>> On 13/03/2020 08:05, Juergen Gross wrote:
>>> Similar to spinlocks preemption should be disabled while holding a
>>> rwlock.
>>>
>>> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
>>> ---
>>>   xen/include/xen/rwlock.h | 18 +++++++++++++++++-
>>>   1 file changed, 17 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/xen/include/xen/rwlock.h b/xen/include/xen/rwlock.h
>>> index 1c221dd0d9..4ee341a182 100644
>>> --- a/xen/include/xen/rwlock.h
>>> +++ b/xen/include/xen/rwlock.h
>>> @@ -2,6 +2,7 @@
>>>   #define __RWLOCK_H__
>>>   #include <xen/percpu.h>
>>> +#include <xen/preempt.h>
>>>   #include <xen/smp.h>
>>>   #include <xen/spinlock.h>
>>> @@ -57,10 +58,12 @@ static inline int _read_trylock(rwlock_t *lock)
>>>       cnts = atomic_read(&lock->cnts);
>>>       if ( likely(_can_read_lock(cnts)) )
>>>       {
>>
>> If you get preempted here, then it means the check below is likely going 
>> to fail. So I think it would be best to disable preemption before, to 
>> give more chance to succeed.
> 
> As preemption probability at this very point should be much lower than
> that of held locks I think that is optimizing the wrong path. I'm not
> opposed doing the modification you are requesting, but would like to
> hear a second opinion on that topic, especially as I'd need to add
> another preempt_enable() call when following your advice.

I can see arguments for both placements, and hence I'm fine either
way.

Jan

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