On Thu, Mar 07, 2024 at 10:20:15AM +0100, David Hildenbrand wrote:
> > > > 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
> > > > 4 index pincount
> > > > 5 private unused
> > > > 6 mapcount mapcount
> > > > 7 refcount refcount
> > > > 8 memcg_data -
> > > > 9+ virtual? last_cpupid? whatever
> >
> > How about this layout?
> >
> > @@ -350,8 +350,13 @@ struct folio {
> > unsigned long _head_1;
> > unsigned long _folio_avail;
> > /* public: */
> > - atomic_t _entire_mapcount;
> > - atomic_t _nr_pages_mapped;
> > + union {
> > + unsigned long _hugetlb_id;
> > + struct {
> > + atomic_t _entire_mapcount;
> > + atomic_t _nr_pages_mapped;
> > + };
> > + };
> > atomic_t _pincount;
> > #ifdef CONFIG_64BIT
> > unsigned int _folio_nr_pages;
> >
> > That keeps _folio_avail as, well, available. It puts _hugetlb_id in
> > the same bits as ->mapping. It continues to leave ->private unused
> > on 64-bit. I think this does everything we want?
>
> _entire_mapcount is (still) used for hugetlb folios.
Oh, duh, of course it is. I thought we used page[0].mapcount for them,
but we don't and shouldn't. I suppose we could use a magic value for
page[0].mapcount to indicate hugetlb, but that'd make page_mapcount()
more complex.
> With the total mapcount in place, I was thinking about renaming it to
> "_pmd_mapcount" and stop using it for hugetlb folios, just like we'd not be
> using _nr_pages_mapped for hugetlb folios.
>
> [I also thought about moving the _pmd_mapcount to another subpage, where
> we'd also have a _pud_mapcount in the future; but again, stuff for the
> future]
>
> Until then, wouldn't _hugetlb_id be problematic here? [I could move
> _entire_mapcount/_pmd_mapcount later I guess]
New idea then, how about simply:
unsigned long _flags_1;
unsigned long _head_1;
- unsigned long _folio_avail;
+ unsigned long _hugetlb_id;
We have to check the various other users of struct page to see what we
might conflict with.
slab:
struct list_head slab_list;
void *freelist; /* first free object */
struct rcu_head rcu_head;
ptdesc:
struct rcu_head pt_rcu_head;
struct list_head pt_list;
pgtable_t pmd_huge_pte;
So it's always used as a pointer (or a pointer in disguise). That means
that either using a pointer to something hugetlb-related or a value like
(void *)-2 will be unambiguous. It'll just be something to document in
each of the types split from struct page.
