[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <YC8U7IagCOfzeBpW@google.com>
Date: Thu, 18 Feb 2021 17:31:24 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Mike Kravetz <mike.kravetz@...cle.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>,
Vitaly Kuznetsov <vkuznets@...hat.com>,
Wanpeng Li <wanpengli@...cent.com>,
Jim Mattson <jmattson@...gle.com>,
Joerg Roedel <joro@...tes.org>, kvm@...r.kernel.org,
linux-kernel@...r.kernel.org, Ben Gardon <bgardon@...gle.com>,
Makarand Sonare <makarandsonare@...gle.com>
Subject: Re: [PATCH 05/14] KVM: x86/mmu: Consult max mapping level when
zapping collapsible SPTEs
On Thu, Feb 18, 2021, Mike Kravetz wrote:
> On 2/18/21 8:23 AM, Sean Christopherson wrote:
> > On Thu, Feb 18, 2021, Paolo Bonzini wrote:
> >> On 13/02/21 01:50, Sean Christopherson wrote:
> >>>
> >>> pfn = spte_to_pfn(iter.old_spte);
> >>> if (kvm_is_reserved_pfn(pfn) ||
> >>> - (!PageTransCompoundMap(pfn_to_page(pfn)) &&
> >>> - !kvm_is_zone_device_pfn(pfn)))
> >>> + iter.level >= kvm_mmu_max_mapping_level(kvm, slot, iter.gfn,
> >>> + pfn, PG_LEVEL_NUM))
> >>> continue;
> >>
> >>
> >> This changes the test to PageCompound. Is it worth moving the change to
> >> patch 1?
> >
> > Yes? I originally did that in a separate patch, then changed my mind.
> >
> > If PageTransCompoundMap() also detects HugeTLB pages, then it is the "better"
> > option as it checks that the page is actually mapped huge. I dropped the change
> > because PageTransCompound() is just a wrapper around PageCompound(), and so I
> > assumed PageTransCompoundMap() would detect HugeTLB pages, too. I'm not so sure
> > about that after rereading the code, yet again.
>
> I have not followed this thread, but HugeTLB hit my mail filter and I can
> help with this question.
>
> No, PageTransCompoundMap() will not detect HugeTLB. hugetlb pages do not
> use the compound_mapcount_ptr field. So, that final check/return in
> PageTransCompoundMap() will always be false.
Thanks Mike!
Paolo, I agree it makes sense to switch to PageCompound in the earlier patch, in
case this one needs to be reverted.
Powered by blists - more mailing lists