Messages in this thread | | | Date | Sat, 18 Jun 2022 11:27:59 +0800 | Subject | Re: [PATCH 1/4] hugetlb: skip to end of PT page mapping when pte not present | From | Baolin Wang <> |
| |
On 6/18/2022 1:17 AM, Mike Kravetz wrote: > On 06/17/22 10:15, Peter Xu wrote: >> Hi, Mike, >> >> On Thu, Jun 16, 2022 at 02:05:15PM -0700, Mike Kravetz wrote: >>> @@ -6877,6 +6896,39 @@ pte_t *huge_pte_offset(struct mm_struct *mm, >>> return (pte_t *)pmd; >>> } >>> >>> +/* >>> + * Return a mask that can be used to update an address to the last huge >>> + * page in a page table page mapping size. Used to skip non-present >>> + * page table entries when linearly scanning address ranges. Architectures >>> + * with unique huge page to page table relationships can define their own >>> + * version of this routine. >>> + */ >>> +unsigned long hugetlb_mask_last_page(struct hstate *h) >>> +{ >>> + unsigned long hp_size = huge_page_size(h); >>> + >>> + switch (hp_size) { >>> + case P4D_SIZE: >>> + return PGDIR_SIZE - P4D_SIZE; >>> + case PUD_SIZE: >>> + return P4D_SIZE - PUD_SIZE; >>> + case PMD_SIZE: >>> + return PUD_SIZE - PMD_SIZE; >>> + default: >> >> Should we add a WARN_ON_ONCE() if it should never trigger? >> > > Sure. I will add this. > >>> + break; /* Should never happen */ >>> + } >>> + >>> + return ~(0UL); >>> +} >>> + >>> +#else >>> + >>> +/* See description above. Architectures can provide their own version. */ >>> +__weak unsigned long hugetlb_mask_last_page(struct hstate *h) >>> +{ >>> + return ~(0UL); >> >> I'm wondering whether it's better to return 0 rather than ~0 by default. >> Could an arch with !CONFIG_ARCH_WANT_GENERAL_HUGETLB wrongly skip some >> valid address ranges with ~0, or perhaps I misread? > > Thank you, thank you, thank you Peter! > > Yes, the 'default' return for hugetlb_mask_last_page() should be 0. If > there is no 'optimization', we do not want to modify the address so we > want to OR with 0 not ~0. My bad, I must have been thinking AND instead > of OR. > > I will change here as well as in Baolin's patch.
Ah, I also overlooked this. Thanks Peter, and thanks Mike for updating.
| |