linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* PageTransCompoundMap confusion
@ 2021-06-04 14:49 Matthew Wilcox
  2021-06-04 15:50 ` Sean Christopherson
  0 siblings, 1 reply; 3+ messages in thread
From: Matthew Wilcox @ 2021-06-04 14:49 UTC (permalink / raw)
  To: Sean Christopherson, Yang Shi, Andrea Arcangeli
  Cc: linux-mm, Paolo Bonzini, kvm

I'm a bit confused about what PageTransCompoundMap() is supposed to do.
What it actually does is check that the specific page (which may or
may not be a head page) is not mapped by a PTE.  I don't understand why
you'd care how some (other?) process does or does not have it mapped.
What I _think_ you want to know is "Can I map this page with a PMD entry
in the guest".  And the answer to that is simply:

bool kvm_is_transparent_hugepage(kvm_pfn_t pfn)
{
	struct page *head = compound_head(pfn_to_page(pfn));
	return compound_order(head) >= HPAGE_PMD_ORDER;
}

but maybe there's some reason you don't want to map hugetlbfs or other
sufficiently large compound pages with PMDs?

Looking at the one caller of kvm_is_transparent_hugepage(), I'd be
tempted to inline the above into transparent_hugepage_adjust()
and call get_page() directly instead of indirecting through
kvm_get_pfn().


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: PageTransCompoundMap confusion
  2021-06-04 14:49 PageTransCompoundMap confusion Matthew Wilcox
@ 2021-06-04 15:50 ` Sean Christopherson
  2021-06-04 16:18   ` Marc Zyngier
  0 siblings, 1 reply; 3+ messages in thread
From: Sean Christopherson @ 2021-06-04 15:50 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Yang Shi, Andrea Arcangeli, linux-mm, Paolo Bonzini, kvm,
	Will Deacon, Marc Zyngier

+Will and Marc

On Fri, Jun 04, 2021, Matthew Wilcox wrote:
> I'm a bit confused about what PageTransCompoundMap() is supposed to do.
> What it actually does is check that the specific page (which may or
> may not be a head page) is not mapped by a PTE.  I don't understand why
> you'd care how some (other?) process does or does not have it mapped.
> What I _think_ you want to know is "Can I map this page with a PMD entry
> in the guest".  And the answer to that is simply:
> 
> bool kvm_is_transparent_hugepage(kvm_pfn_t pfn)
> {
> 	struct page *head = compound_head(pfn_to_page(pfn));
> 	return compound_order(head) >= HPAGE_PMD_ORDER;
> }
> 
> but maybe there's some reason you don't want to map hugetlbfs or other
> sufficiently large compound pages with PMDs?
> 
> Looking at the one caller of kvm_is_transparent_hugepage(), I'd be
> tempted to inline the above into transparent_hugepage_adjust()
> and call get_page() directly instead of indirecting through
> kvm_get_pfn().

arm64 is the only remaining user of kvm_is_transparent_hugepage().

x86 purged its usage a while back, and instead looks at the host PTEs via
lookup_address_in_mm() to get the current mapping level.  The motivation was to
consolidate the hugepage logic for THP, HugeTLBFS, and DAX, and to naturally
support both 2mb and 1gb for all flavors of hugepages.

Could arm64 do something similar and kill off kvm_is_transparent_hugepage()
entirely?


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: PageTransCompoundMap confusion
  2021-06-04 15:50 ` Sean Christopherson
@ 2021-06-04 16:18   ` Marc Zyngier
  0 siblings, 0 replies; 3+ messages in thread
From: Marc Zyngier @ 2021-06-04 16:18 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Matthew Wilcox, Yang Shi, Andrea Arcangeli, linux-mm,
	Paolo Bonzini, kvm, Will Deacon

On Fri, 04 Jun 2021 16:50:20 +0100,
Sean Christopherson <seanjc@google.com> wrote:
> 
> +Will and Marc
> 
> On Fri, Jun 04, 2021, Matthew Wilcox wrote:
> > I'm a bit confused about what PageTransCompoundMap() is supposed to do.
> > What it actually does is check that the specific page (which may or
> > may not be a head page) is not mapped by a PTE.  I don't understand why
> > you'd care how some (other?) process does or does not have it mapped.
> > What I _think_ you want to know is "Can I map this page with a PMD entry
> > in the guest".  And the answer to that is simply:
> > 
> > bool kvm_is_transparent_hugepage(kvm_pfn_t pfn)
> > {
> > 	struct page *head = compound_head(pfn_to_page(pfn));
> > 	return compound_order(head) >= HPAGE_PMD_ORDER;
> > }
> > 
> > but maybe there's some reason you don't want to map hugetlbfs or other
> > sufficiently large compound pages with PMDs?
> > 
> > Looking at the one caller of kvm_is_transparent_hugepage(), I'd be
> > tempted to inline the above into transparent_hugepage_adjust()
> > and call get_page() directly instead of indirecting through
> > kvm_get_pfn().
> 
> arm64 is the only remaining user of kvm_is_transparent_hugepage().
> 
> x86 purged its usage a while back, and instead looks at the host
> PTEs via lookup_address_in_mm() to get the current mapping level.
> The motivation was to consolidate the hugepage logic for THP,
> HugeTLBFS, and DAX, and to naturally support both 2mb and 1gb for
> all flavors of hugepages.
> 
> Could arm64 do something similar and kill off
> kvm_is_transparent_hugepage() entirely?

Yup, we should be able to do that, although we get the additional fun
of caused by having 3 different page sizes. I'll have a go at it next
week.

Thanks for the heads up,

	M.

-- 
Without deviation from the norm, progress is not possible.


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-06-04 16:19 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-04 14:49 PageTransCompoundMap confusion Matthew Wilcox
2021-06-04 15:50 ` Sean Christopherson
2021-06-04 16:18   ` Marc Zyngier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).