Messages in this thread | | | Date | Fri, 13 Jan 2023 10:20:23 +0100 | Subject | Re: [PATCH] Documentation: kvm: fix SRCU locking order docs | From | Paolo Bonzini <> |
| |
On 1/13/23 08:18, Boqun Feng wrote: > On Thu, Jan 12, 2023 at 07:20:48AM -0800, Paul E. McKenney wrote: >> On Thu, Jan 12, 2023 at 08:24:16AM +0000, David Woodhouse wrote: >>> On Wed, 2023-01-11 at 13:30 -0500, Paolo Bonzini wrote: >>>> >>>> +- ``synchronize_srcu(&kvm->srcu)`` is called inside critical sections >>>> + for kvm->lock, vcpu->mutex and kvm->slots_lock. These locks _cannot_ >>>> + be taken inside a kvm->srcu read-side critical section; that is, the >>>> + following is broken:: >>>> + >>>> + srcu_read_lock(&kvm->srcu); >>>> + mutex_lock(&kvm->slots_lock); >>>> + >>> >>> "Don't tell me. Tell lockdep!" >>> >>> Did we conclude in >>> https://lore.kernel.org/kvm/122f38e724aae9ae8ab474233da1ba19760c20d2.camel@infradead.org/ >>> that lockdep *could* be clever enough to catch a violation of this rule >>> by itself? >>> >>> The general case of the rule would be that 'if mutex A is taken in a >>> read-section for SCRU B, then any synchronize_srcu(B) while mutex A is >>> held shall be verboten'. And vice versa. >>> >>> If we can make lockdep catch it automatically, yay! >> >> Unfortunately, lockdep needs to see a writer to complain, and that patch >> just adds a reader. And adding that writer would make lockdep complain >> about things that are perfectly fine. It should be possible to make >> lockdep catch this sort of thing, but from what I can see, doing so >> requires modifications to lockdep itself. >> > > Please see if the follow patchset works: > > https://lore.kernel.org/lkml/20230113065955.815667-1-boqun.feng@gmail.com > > "I have been called. I must answer. Always." ;-)
It's missing an important testcase; if it passes (does not warn), then it should work:
CPU 1 CPU 2 ---------------------------- ------------------------------ mutex_lock(&m1); srcu_read_lock(&srcu1); srcu_read_lock(&srcu1); mutex_lock(&m1); srcu_read_unlock(&srcu1); mutex_unlock(&m1); mutex_unlock(&m1); srcu_read_unlock(&srcu1);
This is the main difference, lockdep-wise, between SRCU and an rwlock.
Paolo
| |