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

Re: [Xen-devel] rwlock and per-cpu rwlock recursive?



Hi Andrew,

On 02/27/2018 07:57 PM, Andrew Cooper wrote:
On 27/02/18 19:18, Julien Grall wrote:
On Arm, we made the (wrong?) assumption that the rwlock was recursive.
We have a couple of places where the read lock can be nested (mostly
the memaccess code).

I found out today that it can actually deadlock in the following case:
     1) CPU A -> Ask for the read lock
         => Succeed
     2) CPU B -> Ask for the write lock
         => Already taken by CPU A so go to the slowpath
         => Take the internal lock (lock->lock)
         => Wait until the lock is released.
     3) CPU A -> Ask for the read lock recursively
         => A writer is waiting so go to the slowpath
         => Try to take the internal lock (lock->lock)
             => Blocking because CPU B already has it

So we end up in a deadlock case. Can someone confirm whether whether
rwlock was meant to be recursive?

rwlocks are not recursive, but probably will appear to be in the common
case.

When uncontended, an effectively-arbitrary number of read_lock()'s can
complete, but when contended, readers and writers use a regular spinlock
(which is a ticketlock under the hood) to ensure fairness.

Thank you for the explanation. I will have to rework the Arm code then.


Similarly, I was thinking to move the p2m code to the per-cpu rwlock
as x86 does. From what I gathered by reading the x86 code, this lock
could be taken recursively. Am I right?

No - what gives you the impression that it can be taken recursively?  In
the case where we detect taking a second percpu_rwlock, we fall back to
the slowpath of a real read_lock() call.

I knew the p2m lock could be taken recursively but I was not sure how this was achieved. So I assumed it was thanks to the percpu_rwlock.


Lastly, the x86 p2m code seemed to use rwlock before hand. How did the
p2m code was handling recursive locking?

Plain spinlocks (when using the recursive helpers), and x86
mm_{rw,}lock_t's can be taken recursively.  A call to spin_lock() will
deadlock if the lock is already taken and you meant to use
spin_lock_recursive().

You probably want to see about reading xen/arch/x86/mm/mm-locks.h

Looking at the implementation, I see recursion handling for mm_write_lock but not for mm_read_lock. Can you confirm that only the write lock can be taken recursively?

Cheers,

--
Julien Grall

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

 


Rackspace

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