Il 27/06/2013 04:56, Paul Gortmaker ha scritto: >> Il 26/06/2013 20:11, Paul Gortmaker ha scritto: >>> > > spin_unlock(&kvm->mmu_lock); >>> > > + kvm_put_kvm(kvm); >>> > > srcu_read_unlock(&kvm->srcu, idx); >>> > > >> > >> > kvm_put_kvm needs to go last. I can fix when applying, but I'll wait >> > for Gleb to take a look too. > I'm curious why you would say that -- since the way I sent it has the > lock tear down be symmetrical and opposite to the build up - e.g. > > idx = srcu_read_lock(&kvm->srcu); > > [...] > > + kvm_get_kvm(kvm); > > [...] > spin_lock(&kvm->mmu_lock); > > [...] > > unlock: > spin_unlock(&kvm->mmu_lock); > + kvm_put_kvm(kvm); > srcu_read_unlock(&kvm->srcu, idx); > > You'd originally said to put the kvm_get_kvm where it currently is; > perhaps instead we want the get/put to encompass the whole > srcu_read locked section?
The put really needs to be the last thing you do, as the data structure can be destroyed before it returns. Where you put kvm_get_kvm doesn't really matter, since you're protected by the kvm lock. So, moving the kvm_get_kvm before would also work---I didn't really mean that kvm_get_kvm has to be literally just before the raw_spin_unlock. However, I actually like having the get_kvm right there, because it makes it explicit that you are using reference counting as a substitute for holding the lock. I find it quite idiomatic, and in some sense the lock/unlock is still symmetric: the kvm_put_kvm goes exactly where you'd have unlocked the kvm_lock. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
