Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752514AbdHHXG4 (ORCPT ); Tue, 8 Aug 2017 19:06:56 -0400 Received: from mga07.intel.com ([134.134.136.100]:60334 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752475AbdHHXGy (ORCPT ); Tue, 8 Aug 2017 19:06:54 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,345,1498546800"; d="scan'208";a="297562957" From: "Huang\, Ying" To: Matthew Wilcox Cc: "Huang\, Ying" , Andrew Morton , , , Andrea Arcangeli , "Kirill A. Shutemov" , Nadia Yvette Chambers , Michal Hocko , Jan Kara , Hugh Dickins , Minchan Kim , Shaohua Li Subject: Re: [PATCH -mm] mm: Clear to access sub-page last when clearing huge page References: <20170807072131.8343-1-ying.huang@intel.com> <20170808121220.GA31390@bombadil.infradead.org> Date: Wed, 09 Aug 2017 07:06:48 +0800 In-Reply-To: <20170808121220.GA31390@bombadil.infradead.org> (Matthew Wilcox's message of "Tue, 8 Aug 2017 05:12:20 -0700") Message-ID: <87y3qt64mv.fsf@yhuang-mobile.sh.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2803 Lines: 81 Matthew Wilcox 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