Hi, Jan posted an lockdep splat complaining about a suspicious rcu_dereference_check: https://lists.cs.columbia.edu/pipermail/kvmarm/2018-May/031116.html The gist of that is: ... [ 1025.695517] dump_stack+0x9c/0xd4 [ 1025.695524] lockdep_rcu_suspicious+0xcc/0x118 [ 1025.695537] gfn_to_memslot+0x174/0x190 [ 1025.695546] kvm_read_guest+0x50/0xb0 [ 1025.695553] vgic_its_check_id.isra.0+0x114/0x148 ... I chased that down and wonder if kvm_read_guest{,_page} is supposed to be called inside a kvm->srcu critical section? We have a check that suggests that eventually someone needs to enter the SRCU criticial section: static inline struct kvm_memslots *__kvm_memslots(struct kvm *kvm, int as_id) { return srcu_dereference_check(kvm->memslots[as_id], &kvm->srcu, lockdep_is_held(&kvm->slots_lock) || !refcount_read(&kvm->users_count)); } If I get this correctly this mean for accessing kvm->memslots we either need to be inside an srcu critical section or hold the kvm->slots_lock (for updates only). If I am not mistaken, it is not necessary for *callers* of kvm_read_guest_page() to do this, as this could be entirely contained inside this function - since we only use the reference to the memslot entry while doing the copy_from_user(), and the data is safe afterwards from an RCU point of view because it has been *copied*. Does that sound correct? Has anyone run a lockdep enabled kernel with PROVE_RCU before and stumbled upon this message? I am a bit doubtful that this is valid because this is generic KVM code and should trigger with every kvm_read_guest() user. With my limited understanding I would say the we just need to have a srcu_read_lock() call at the beginning of kvm_read_guest_page() and the corresponding srcu_read_unlock() call at the end of that function. Is that enough? Cheers, Andre.
