On Tue, Aug 04, 2026, Ackerley Tng wrote: > Xiaoyao Li <[email protected]> writes: > > > > > [...snip...] > > > >> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c > >> index ea2752989f8bd..df67a6188aa99 100644 > >> --- a/virt/kvm/guest_memfd.c > >> +++ b/virt/kvm/guest_memfd.c > >> @@ -1137,10 +1137,12 @@ static bool kvm_range_is_private(struct file > >> *file, pgoff_t index, > >> { > >> struct inode *inode = file_inode(file); > >> > >> +#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES > >> if (!gmem_in_place_conversion) > >> return kvm_range_has_vm_memory_attributes(kvm, gfn, gfn + > >> nr_pages, > >> > >> KVM_MEMORY_ATTRIBUTE_PRIVATE, > >> > >> KVM_MEMORY_ATTRIBUTE_PRIVATE); > >> +#endif > >> > >> return kvm_gmem_range_has_attributes(inode, index, nr_pages, > >> KVM_MEMORY_ATTRIBUTE_PRIVATE); > > > > With my suggestion[1] on patch 7, this diff block can be dropped. > > > > [1] > > https://lore.kernel.org/all/[email protected]/ > > > > The code looks good to me, > > > > Reviewed-by: Xiaoyao Li <[email protected]> > > > > I think your earlier suggestion on only defining the module_param when > CONFIG_KVM_VM_MEMORY_ATTRIBUTES is selected still applies, since it > removes an avenue for a user to turn off gmem_in_place_conversion if > the other option (VM memory attributes) isn't available. > > +#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES > +module_param(gmem_in_place_conversion, bool, 0444); > +#endif > > Given that we're now dropping the diff block as you suggested, do you > agree that we should keep this #ifdef?
Yes, the param needs to remain hidden if CONFIG_KVM_VM_MEMORY_ATTRIBUTES=n, so that userspace attempts to disable gmem_in_place_conversion are ignored (and logged as such). The obvious downside is that the param won't exist when in-place conversion is forced on, which is kinda weird, but I think it's the lesser evil?
