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]
Message-ID: <703bb7de-aba7-ee4b-c2fb-3562318072a5@redhat.com>
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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ