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
