linux-kernel - Re: [PATCH -mm] mm: Clear to access sub-page last when clearing huge page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 09 Aug 2017 07:06:48 +0800
From:   "Huang\, Ying" <ying.huang@...el.com>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     "Huang\, Ying" <ying.huang@...el.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        <linux-mm@...ck.org>, <linux-kernel@...r.kernel.org>,
        Andrea Arcangeli <aarcange@...hat.com>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Nadia Yvette Chambers <nyc@...omorphy.com>,
        Michal Hocko <mhocko@...e.com>, Jan Kara <jack@...e.cz>,
        Hugh Dickins <hughd@...gle.com>,
        Minchan Kim <minchan@...nel.org>, Shaohua Li <shli@...com>
Subject: Re: [PATCH -mm] mm: Clear to access sub-page last when clearing huge page

Matthew Wilcox <willy@...radead.org> 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