On Fri, Jul 31, 2026, Xiaoyao Li wrote: > On 7/31/2026 4:42 AM, Ackerley Tng wrote: > > Xiaoyao Li <[email protected]> writes: > > > > > On 7/29/2026 8:35 AM, Ackerley Tng via B4 Relay wrote: > > > > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c > > > > index 33c9830190e2e..89cf922232920 100644 > > > > --- a/virt/kvm/guest_memfd.c > > > > +++ b/virt/kvm/guest_memfd.c > > > > @@ -893,6 +893,27 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct > > > > kvm_memory_slot *slot, > > > > EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gmem_get_pfn); > > > > > > > > #ifdef CONFIG_HAVE_KVM_ARCH_GMEM_POPULATE > > > > +static bool kvm_range_is_private(struct file *file, pgoff_t index, > > > > + size_t nr_pages, struct kvm *kvm, > > > > gfn_t gfn) > > > > +{ > > > > + struct inode *inode = file_inode(file); > > > > + pgoff_t last = index + nr_pages - 1; > > > > + struct maple_tree *mt; > > > > + void *entry; > > > > + > > > > + 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); > > > > + > > > > + mt = &GMEM_I(inode)->attributes; > > > > + mt_for_each(mt, entry, index, last) { > > > > + if (kvm_gmem_interpret_entry(inode, entry) != > > > > + KVM_MEMORY_ATTRIBUTE_PRIVATE) > > > > + return false; > > > > + } > > > > + return true; > > > > +} > > > > > > > > static long __kvm_gmem_populate(struct kvm *kvm, struct > > > > kvm_memory_slot *slot, > > > > struct file *file, gfn_t gfn, struct > > > > page *src_page, > > > > @@ -913,9 +934,7 @@ static long __kvm_gmem_populate(struct kvm *kvm, > > > > struct kvm_memory_slot *slot, > > > > > > > > folio_unlock(folio); > > > > > > > > - if (!kvm_range_has_vm_memory_attributes(kvm, gfn, gfn + 1, > > > > - > > > > KVM_MEMORY_ATTRIBUTE_PRIVATE, > > > > - > > > > KVM_MEMORY_ATTRIBUTE_PRIVATE)) { > > > > + if (!kvm_range_is_private(file, index, 1, kvm, gfn)) { > > > > > > It's checking if a single gfn is private. > > > > > > > But with huge page support we'd need to check a range of gfns... I guess > > there's the argument of not keeping complexity for the future, but in > > this case it's undoing functionality for this series and probably adding > > it back later. > > let's leave the work for huge page support in the future.
It's not just for hugepage support, the core functionality is used in this series by __kvm_gmem_set_attributes() in: KVM: guest_memfd: Return early if range already has requested attributes > > > We can just use kvm_mem_is_private()? And it seems can be a separate > > > patch. > > > > > > > Do you mean that this refactoring could be a separate patch? I could > > refactor out kvm_range_is_private() in a separate patch, then put in the > > mt_for_each() check in this patch together as part of all the rest of > > the "wiring". > > > > I considered that but it seemed like the refactoring was too small to > > separate out into another patch when the mt_for_each() part is going to > > be in this patch anyway. > > > > Or, I could add an earlier patch to first replace the call to > > kvm_range_has_vm_memory_attributes() with a call to check just 1 gfn, > > then this wiring patch would be simpler. > > > > This is what I meant. An earlier patch to just replace > kvm_range_has_vm_memory_attributes() with kvm_mem_is_private(). Then we can > drop this 'big' diff in this patch. > > If kvm_range_is_private() is useful/required by huge page support, then let > the huge page series to introduce it. It has nothing to do with the in-place > conversion series. If it weren't for the fact that the range-based search is used later in this series, I would 100% agree with Xiaoyao. But since the core logic is used and needed elsewhere, and because kvm_range_has_vm_memory_attributes() takes a range, my vote is to provide the plumbing now, even though a small portion of it isn't strictly necessary.
