On 05.03.24 21:35, Matthew Wilcox wrote:
On Tue, Mar 05, 2024 at 10:10:08AM +0100, David Hildenbrand wrote:
The cost of this reliability is that we now consume the word I recently
freed in folio->page[1]. I think this is acceptable; we've still gained
a completely reliable folio_test_hugetlb() (which we didn't have before
I started messing around with the folio dtors). Non-hugetlb users
can use large_id as a pointer to something else entirely, or even as a
non-pointer, as long as they can guarantee it can't conflict (ie don't
use it as a bitfield).
That probably means that we have to always set the lowest bit to use it for
something else, or use another bit.
Yes, that would work.
I was wondering if
a) We could move that to another subpage. In hugetlb folios we have plenty
of space for such things. I guess we'd have be able to detect the folio size
without holding a reference, to make sure we can touch another subpage.
Yes, that was my concern. I wanted to put it in page[2] with all the
other hugetlb goop, but I got to thinking about an order-1 compound
page allocated at the end of memmap and got scared. We could make
folio_test_hugetlb() look at ->flags for the head bit, then look at
->flags_1 for the order and finally at ->hugetlb_id, but now we've looked
at three cachelines to answer a fairly frequent question. And then what
if the folio got split between looking at ->flags and ->flags_1 and we
get a bogus folio order that makes it look OK? We can't even look at
->flags, ->flags_1 and recheck ->flags because it might have got split,
freed and reallocated in the meantime.
b) We could overload _nr_pages_mapped. We'd effectively have to steal one
bit from _nr_pages_mapped to make this work.
Maybe what works is using the existing mechanism (hugetlb flag), and then
storing the pointer in __nr_pages_mapped.
So depending on the hugetlb flag, we can interpret __nr_pages_mapped either
as the pointer or as the old variant.
Mostly only folio_large_is_mapped() would need care for now, to ignore
_nr_pages_mapped if the hugetlb flag is set.
I don't mind that at all. We wouldn't even need to steal a bit or use the
existing flag; we could just say that -2 means this is a hugetlb folio.
As long as it ends up at the same offset as page->mapping (because that's
always NULL or a pointer possibly with a low bit set so can't ever be a
number between -4095 and -1).
Would hugetlb_id below be 32bit or 64bit on 64-bit?
IOW:
word page0 page1
0 flags flags
1 lru.next head
2 lru.prev entire_mapcount + gap
3 mapping nr_pages_mapped + gap / hugetlb_id
4 index pincount + nr_pages
5 private unused
6 mapcount+refcount mapcount+refcount(0)
7 memcg_data -
or on 32-bit
word page0 page1
0 flags flags
1 lru.next head
2 lru.prev entire_mapcount
3 mapping nr_pages_mapped / hugetlb_id
^ In the worst case, I think, nr_pages_mapped with a lot of entire
mappings could end up matching hugetlb_id. We add a large value to
nr_pages_mapped every time we add an entire mapping ... (not sure if
that could currently be a problem with many entire mappings of a large
folio)
4 index pincount
5 private unused
6 mapcount mapcount
7 refcount refcount
8 memcg_data -
9+ virtual? last_cpupid? whatever
Does this fit with your plans?
For the total mapcount this would do (and it would be better), but the
layout gets a bit "sparse" on 64bit that way, which will end up being
problematic for some other stuff I might want to put in there.
Not that we have to resolve that now, just bringing it up, that maybe we
can do better right away :)
IIUC, we would not be able to reuse the "gap" in "nr_pages_mapped + gap
/ hugetlb_id", essentially consuming an additional 32bit compared to
what we do now, correct?
I was thinking of the following, assuming your example above indicated
on64bit a hugetlb_id that is 64bit:
hugetlb folios set
* compound page
* flag in subpage 1 (like we do)
* nr_pages_mapped in subpage 1 to e.g., -2.
So to check lockless if we have a hugetlb folio
* Check if compund
* Check if the flag in subpage 1 is set
* Check if nr_pages_mapped matches
Would that still be too unreliable?
--
Cheers,
David / dhildenb
