linux-kernel - Re: [PATCH mm-unstable v1] mm: add a total mapcount for large folios

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, 9 Aug 2023 21:26:10 +0200
From:   David Hildenbrand <david@...hat.com>
To:     Matthew Wilcox <willy@...radead.org>,
        Ryan Roberts <ryan.roberts@....com>
Cc:     linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        linux-doc@...r.kernel.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        Jonathan Corbet <corbet@....net>,
        Mike Kravetz <mike.kravetz@...cle.com>,
        Hugh Dickins <hughd@...gle.com>,
        Yin Fengwei <fengwei.yin@...el.com>,
        Yang Shi <shy828301@...il.com>, Zi Yan <ziy@...dia.com>
Subject: Re: [PATCH mm-unstable v1] mm: add a total mapcount for large folios

On 09.08.23 21:21, Matthew Wilcox wrote:
> On Wed, Aug 09, 2023 at 08:07:43PM +0100, Ryan Roberts wrote:
>>> +++ b/mm/hugetlb.c
>>> @@ -1479,7 +1479,7 @@ static void __destroy_compound_gigantic_folio(struct folio *folio,
>>>   	struct page *p;
>>>   
>>>   	atomic_set(&folio->_entire_mapcount, 0);
>>> -	atomic_set(&folio->_nr_pages_mapped, 0);
>>> +	atomic_set(&folio->_total_mapcount, 0);
>>
>> Just checking this is definitely what you intended? _total_mapcount is -1 when
>> it means "no pages mapped", so 0 means 1 page mapped?
> 
> We're destroying the page here, so rather than setting the meaning of
> this, we're setting the contents of this memory to 0.
> 
> 
> Other thoughts that ran through my mind ... can we wrap?  I don't think
> we can; we always increment total_mapcount by 1, no matter whether we're
> incrementing entire_mapcount or an individual page's mapcount, and we
> always call folio_get() first, so we can't increment total_mapcount
> past 2^32 because folio_get() will die first.  We might be able to
> wrap past 2^31, but I don't think so.

 From my understanding, if we wrap the total mapcount, we already 
wrapped the refcount -- as you say, grabbing a reference ahead of time 
for each mapping is mandatory. Both are 31bit values. We could treat the 
total mapcount as an unsigned int, but that's rather future work.

Also, even folio_mapcount() and total_mapcount() return an "int" as of now.

But yes, I also thought about that. In the future we might want (at 
least) for bigger folios refcount+total_mapcount to be 64bit. Or we 
manage to decouple both and only have the total_mapcount be 64bit only.

-- 
Cheers,

David / dhildenb