On Fri, Jul 18, 2025 at 07:54:38AM -0700, Sean Christopherson wrote: > On Thu, Jul 17, 2025, Yao Yuan wrote: > > On Wed, Jul 16, 2025 at 11:07:36AM +0800, Keir Fraser wrote: > > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > > > index 3bde4fb5c6aa..9132148fb467 100644 > > > --- a/include/linux/kvm_host.h > > > +++ b/include/linux/kvm_host.h > > > @@ -965,11 +965,15 @@ static inline bool kvm_dirty_log_manual_protect_and_init_set(struct kvm *kvm) > > > return !!(kvm->manual_dirty_log_protect & KVM_DIRTY_LOG_INITIALLY_SET); > > > } > > > > > > +/* > > > + * Get a bus reference under the update-side lock. No long-term SRCU reader > > > + * references are permitted, to avoid stale reads vs concurrent IO > > > + * registrations. > > > + */ > > > static inline struct kvm_io_bus *kvm_get_bus(struct kvm *kvm, enum kvm_bus idx) > > > { > > > - return srcu_dereference_check(kvm->buses[idx], &kvm->srcu, > > > - lockdep_is_held(&kvm->slots_lock) || > > > - !refcount_read(&kvm->users_count)); > > > + return rcu_dereference_protected(kvm->buses[idx], > > > + lockdep_is_held(&kvm->slots_lock)); > > > > I want to consult the true reason for using protected version here, > > save unnecessary READ_ONCE() ? > > Avoiding the READ_ONCE() is a happy bonus. The main goal is to help document > and enforce that kvm_get_bus() can only be used if slots_lock is held. Keeping > this as srcu_dereference_check() would result in PROVE_RCU getting a false negative > if the caller held kvm->srcu but not slots_lock. Ah, I noticed the srcu_read_lock_held(ssp) in srcu_dereference_check() this time ! > > From a documentation perspective, rcu_dereference_protected() (hopefully) helps > highlight that there's something "special" about this helper, e.g. gives the reader > a hint that they probably shouldn't be using kvm_get_bus(). Yes, I got it. Thanks for your nice explanation! >