Hi, Sean, On Wed, Feb 15, 2023 at 09:23:55AM -0800, Sean Christopherson wrote: > > @@ -4230,9 +4231,25 @@ static int __kvm_faultin_pfn(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault > > } > > > > async = false; > > - fault->pfn = __gfn_to_pfn_memslot(slot, fault->gfn, false, false, &async, > > - fault->write, &fault->map_writable, > > - &fault->hva); > > + mem_fault_nowait = memory_faults_enabled(vcpu->kvm); > > + > > + fault->pfn = __gfn_to_pfn_memslot( > > + slot, fault->gfn, > > + mem_fault_nowait, > > Hrm, as prep work for this series, I think we should first clean up the pile o' > bools. This came up before when the "interruptible" flag was added[*]. We punted > then, but I think it's time to bite the bullet, especially since "nowait" and > "async" need to be mutually exclusive. > > [*] https://lore.kernel.org/all/YrR9i3yHzh5ftOxB@xxxxxxxxxx IIUC at that time we didn't reach a consensus on how to rework it, and the suggestion was using a bool and separate the cleanup (while the flag cleanup got NACKed..), which makes sense to me. Personally I like your other patch a lot: https://lore.kernel.org/all/YrTNGVpT8Cw2yrnr@xxxxxxxxxx/ The question is, after we merge the bools into a flag, do we still need a struct* anyway? If we can reach a consensus, I'll be happy to continue working on the cleanup (by picking up your patch first). Or if Anish would like to take it over in any form that'll be also perfect. Thanks, -- Peter Xu
