On Thu, 3 Nov 2022, Sidhartha Kumar wrote: > On 11/2/22 6:48 PM, Hugh Dickins wrote: ... > > Undo "mm: add private field of first tail to struct page and struct > > folio"'s recent addition of private_1 to the folio tail: instead add > > hugetlb_subpool, hugetlb_cgroup, hugetlb_cgroup_rsvd, hugetlb_hwpoison > > to a second tail page of the folio: THP has long been using several > > fields of that tail, so make better use of it for hugetlb too. > > This is not how a generic folio should be declared in future, > > but it is an effective transitional way to make use of it. ... > > @@ -260,13 +267,16 @@ struct page { > > * to find how many references there are to this folio. > > * @memcg_data: Memory Control Group data. > > * @_flags_1: For large folios, additional page flags. > > - * @__head: Points to the folio. Do not use. > > + * @_head_1: Points to the folio. Do not use. > > Changes to my original patch set look good, this seems to be a cleaner > implementation. Thanks a lot, Sidhartha, I'm glad to hear that it works for you too. I expect that it will be done differently in the future: maybe generalizing the additional fields to further "private"s as you did, letting different subsystems accessorize them differently; or removing them completely from struct folio, letting subsystems declare their own struct folio containers. I don't know how that will end up, but this for now seems good and clear. > > Should the usage of page_1 and page_2 also be documented here? You must have something interesting in mind to document about them, but I cannot guess what! They are for field alignment, not for use. (page_2 to help when/if someone needs to add another pageful.) Do you mean that I should copy the /* private: the union with struct page is transitional */ comment from above the original "struct page page;" line I copied? Or give all three of them a few underscores to imply not for use? Thanks, Hugh