Messages in this thread | | | Date | Mon, 14 Aug 2023 16:01:24 -0700 | Subject | Re: [PATCH RFC v2 3/3] mm: Proper document tail pages fields for folio | From | Randy Dunlap <> |
| |
Hi--
On 8/14/23 11:44, Peter Xu wrote: > Tail page struct reuse is over-comlicated. Not only because we have > implicit uses of tail page fields (mapcounts, or private for thp swap > support, etc., that we _may_ still use in the page structs, but not obvious > the relationship between that and the folio definitions), but also because > we have 32/64 bits layouts for struct page so it's unclear what we can use > and what we cannot when trying to find a new spot in folio struct. > > We also have tricks like page->mapping, where we can reuse only the tail > page 1/2 but nothing more than tail page 2. It is all mostly hidden, until > someone starts to read into a VM_BUG_ON_PAGE() of __split_huge_page_tail(). > > It's also unclear on how many fields we can reuse for a tail page. The > real answer is (after help from Matthew): we have 7 WORDs guaranteed on 64 > bits and 8 WORDs on 32 bits. Nothing more than that is guaranteed to even > exist. > > Let's document it clearly on what we can use and what we can't when > extending folio on reusing tail page fields, with 100% explanations on each > of them. Hopefully after the doc update it will make it easier when: > > (1) Any reader to know exactly what field is where and for what, the > relationships between folio tail pages and struct page definitions, > > (2) Any potential new fields to be added to a large folio, so we're clear > which field one can still reuse. > > This is assuming WORD is defined as sizeof(void *) on any archs, just like > the other comment in struct page we already have. > > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > include/linux/mm_types.h | 41 ++++++++++++++++++++++++++++++++++------ > 1 file changed, 35 insertions(+), 6 deletions(-) > > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h > index 829f5adfded1..9c744f70ae84 100644 > --- a/include/linux/mm_types.h > +++ b/include/linux/mm_types.h > @@ -322,11 +322,40 @@ struct folio { > }; > struct page page; > }; > + /* > + * Some of the tail page fields may not be reused by the folio > + * object because they're already been used by the page struct. On
they have
> + * 32bits there're at least 8 WORDs while on 64 bits there're at
preferably s/there're/there are/
> + * least 7 WORDs: > + * > + * |--------+-------------+-------------------| > + * | index | 32 bits | 64 bits | > + * |--------+-------------+-------------------| > + * | 0 | flags | flags | > + * | 1 | head | head | > + * | 2 | FREE | FREE | > + * | 3 | FREE [1] | FREE [1] | > + * | 4 | FREE | FREE | > + * | 5 | FREE | private [2] | > + * | 6 | mapcnt | mapcnt+refcnt [3] | > + * | 7 | refcnt [3] | | > + * |--------+-------------+-------------------| > + * > + * [1] "mapping" field. It is free to use but needs to be with > + * some caution due to poisoning, see TAIL_MAPPING_REUSED_MAX. > + * > + * [2] "private" field, used when THP_SWAP is on (but disabled on > + * 32 bits, so this index is FREE on 32bit or hugetlb folios). > + * May need to be fixed finally. > + * > + * [3] "refcount" field must be zero for all tail pages. See e.g. > + * has_unmovable_pages() on page_ref_count() check and comment. > + */ > union { > struct { > unsigned long _flags_1; > unsigned long _head_1; > - /* public: */ > + /* public: WORD 2 */ > unsigned char _folio_dtor; > unsigned char _folio_order; > /* private: 2 bytes can be reused later */ > @@ -335,7 +364,7 @@ struct folio { > /* 4 bytes can be reused later (64 bits only) */ > unsigned char _free_1_1[4]; > #endif > - /* public: */ > + /* public: WORD 3 */ > atomic_t _entire_mapcount; > atomic_t _nr_pages_mapped; > atomic_t _pincount; > @@ -350,20 +379,20 @@ struct folio { > struct page __page_1; > }; > union { > - struct { > + struct { /* hugetlb folios */ > unsigned long _flags_2; > unsigned long _head_2; > - /* public: */ > + /* public: WORD 2 */ > void *_hugetlb_subpool; > void *_hugetlb_cgroup; > void *_hugetlb_cgroup_rsvd; > void *_hugetlb_hwpoison; > /* private: the union with struct page is transitional */ > }; > - struct { > + struct { /* non-hugetlb folios */ > unsigned long _flags_2a; > unsigned long _head_2a; > - /* public: */ > + /* public: WORD 2-3 */ > struct list_head _deferred_list; > /* private: 8 more free bytes for either 32/64 bits */ > unsigned char _free_2_1[8];
| |