Re: [PATCH v9 07/41] KVM: guest_memfd: Wire up core private/shared attribute interfaces
Xiaoyao Li <[email protected]> Tue, 4 Aug 2026 10:11:05 +0800
| Newsgroups | dev.linux.lists.linux-coco,org.kernel.vger.kvm,org.kernel.vger.linux-doc,org.kernel.vger.linux-kernel,org.kernel.vger.linux-kselftest,org.kernel.vger.linux-trace-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <[email protected]> |
On 8/3/2026 11:33 PM, Sean Christopherson wrote: > On Mon, Aug 03, 2026, Xiaoyao Li wrote: >> On 8/1/2026 12:26 AM, Sean Christopherson wrote: >>> On Fri, Jul 31, 2026, Xiaoyao Li wrote: >>>> On 7/31/2026 4:42 AM, Ackerley Tng wrote: >>>> 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, >> >> I don't see it a problem. Without introducing kvm_range_is_private() in this >> patch, the core logic of range-based search on gmem can still be introduced >> as kvm_gmem_range_has_attributes() directly in patch 10 or in a separate >> patch. >> >>> 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. >> >> So my initial feedback was "we can just use kvm_mem_is_private(kvm, gfn)". >> It makes code simpler. > > My apologies, I hadn't actually applied v9 to look at the full context. I agree, > providing kvm_range_is_private() is completely unnecessary. It's not even really > a problem with this patch, it's a pre-existing issue that can and should be > addressed with a prep patch. yea, that's why I said "it seems can be a separate patch. " >> And as a bonus, the logic to choose between gmem-based attribute query and >> vm-based attribute query is hidden from the static call. > > I don't follow. What do you mean by "hidden from the static call?" The whole > point of using a static call is to select between gmem-based and vm-based private > memory. Or are you saying the populate() flow doesn't need to manually check > gmem_in_place_conversion? yea. By using kvm_mem_is_private(), we don't need the kvm_range_is_private() introduced by this patch, which has the gmem_in_place_conversion check. >> The second bonus of dropping kvm_range_is_private() is that we can eliminate >> one more chunk in patch 20. > > Ya. > > Untested, but unless I'm missing something, patches 6 and 7 of this series can > be replaced with the attached patches (6 and 7 should be squashed together no > matter what). The attached patches are just what I wanted. They look good!