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 org.kvack.linux-mm,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
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!