linux-kernel - Re: [PATCH 3/5] mm: hugetlb: free the 2nd vmemmap page associated with each HugeTLB 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:   Mon, 26 Jul 2021 16:56:17 -0700
From:   Mike Kravetz <mike.kravetz@...cle.com>
To:     Matthew Wilcox <willy@...radead.org>,
        Muchun Song <songmuchun@...edance.com>
Cc:     akpm@...ux-foundation.org, osalvador@...e.de, mhocko@...e.com,
        song.bao.hua@...ilicon.com, david@...hat.com,
        chenhuang5@...wei.com, bodeddub@...zon.com, corbet@....net,
        duanxiongchun@...edance.com, fam.zheng@...edance.com,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org, zhengqi.arch@...edance.com
Subject: Re: [PATCH 3/5] mm: hugetlb: free the 2nd vmemmap page associated
 with each HugeTLB page

On 7/26/21 2:16 PM, Matthew Wilcox wrote:
> On Wed, Jul 14, 2021 at 05:17:58PM +0800, Muchun Song wrote:
>> +static __always_inline struct page *page_head_if_fake(const struct page *page)
>> +{
>> +	if (!hugetlb_free_vmemmap_enabled)
>> +		return NULL;
>> +
>> +	/*
>> +	 * Only addresses aligned with PAGE_SIZE of struct page may be fake head
>> +	 * struct page. The alignment check aims to avoid access the fields (
>> +	 * e.g. compound_head) of the @page[1]. It can avoid touch a (possibly)
>> +	 * cold cacheline in some cases.
>> +	 */
>> +	if (IS_ALIGNED((unsigned long)page, PAGE_SIZE) &&
>> +	    test_bit(PG_head, &page->flags)) {
>> +		unsigned long head = READ_ONCE(page[1].compound_head);
>> +
>> +		if (likely(head & 1))
>> +			return (struct page *)(head - 1);
>> +	}
>> +
>> +	return NULL;
>> +}
> 
> Why return 'NULL' instead of 'page'?
> 
> This is going to significantly increase the cost of calling
> compound_page() (by whichever spelling it has).  That will make
> the folio patchset more compelling ;-)

Matthew, any suggestions for benchmarks/workloads to measure the
increased overhead?  Suspect you have some ideas based on folio work.

My concern is that we are introducing overhead for code paths not
associated with this feature.  The next patch even tries to minimize
this overhead a bit if hugetlb_free_vmemmap_enabled is not set.
-- 
Mike Kravetz