Matthew Wilcox <willy@xxxxxxxxxxxxx> writes: > On Mon, Aug 07, 2017 at 03:21:31PM +0800, Huang, Ying wrote: >> @@ -2509,7 +2509,8 @@ enum mf_action_page_type { >> #if defined(CONFIG_TRANSPARENT_HUGEPAGE) || defined(CONFIG_HUGETLBFS) >> extern void clear_huge_page(struct page *page, >> unsigned long addr, >> - unsigned int pages_per_huge_page); >> + unsigned int pages_per_huge_page, >> + unsigned long addr_hint); > > I don't really like adding the extra argument to this function ... > >> +++ b/mm/huge_memory.c >> @@ -549,7 +549,8 @@ static int __do_huge_pmd_anonymous_page(struct vm_fault *vmf, struct page *page, >> struct vm_area_struct *vma = vmf->vma; >> struct mem_cgroup *memcg; >> pgtable_t pgtable; >> - unsigned long haddr = vmf->address & HPAGE_PMD_MASK; >> + unsigned long address = vmf->address; >> + unsigned long haddr = address & HPAGE_PMD_MASK; >> >> VM_BUG_ON_PAGE(!PageCompound(page), page); >> >> @@ -566,7 +567,7 @@ static int __do_huge_pmd_anonymous_page(struct vm_fault *vmf, struct page *page, >> return VM_FAULT_OOM; >> } >> >> - clear_huge_page(page, haddr, HPAGE_PMD_NR); >> + clear_huge_page(page, haddr, HPAGE_PMD_NR, address); >> /* >> * The memory barrier inside __SetPageUptodate makes sure that >> * clear_huge_page writes become visible before the set_pmd_at() > > How about calling: > > - clear_huge_page(page, haddr, HPAGE_PMD_NR); > + clear_huge_page(page, address, HPAGE_PMD_NR); > >> +++ b/mm/memory.c >> @@ -4363,10 +4363,10 @@ static void clear_gigantic_page(struct page *page, >> clear_user_highpage(p, addr + i * PAGE_SIZE); >> } >> } >> -void clear_huge_page(struct page *page, >> - unsigned long addr, unsigned int pages_per_huge_page) >> +void clear_huge_page(struct page *page, unsigned long addr, >> + unsigned int pages_per_huge_page, unsigned long addr_hint) >> { >> - int i; >> + int i, n, base, l; >> >> if (unlikely(pages_per_huge_page > MAX_ORDER_NR_PAGES)) { >> clear_gigantic_page(page, addr, pages_per_huge_page); > > ... and doing this: > > void clear_huge_page(struct page *page, > - unsigned long addr, unsigned int pages_per_huge_page) > + unsigned long addr_hint, unsigned int pages_per_huge_page) > { > - int i; > + int i, n, base, l; > + unsigned long addr = addr_hint & > + (1UL << (pages_per_huge_page + PAGE_SHIFT)); > >> @@ -4374,9 +4374,31 @@ void clear_huge_page(struct page *page, >> } >> >> might_sleep(); >> - for (i = 0; i < pages_per_huge_page; i++) { >> + VM_BUG_ON(clamp(addr_hint, addr, addr + >> + (pages_per_huge_page << PAGE_SHIFT)) != addr_hint); > > ... then you can ditch this check Yes. This looks good for me. If there is no objection, I will go this way in the next version. Best Regards, Huang, Ying -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>