On Wed, Nov 18, 2020 at 3:22 AM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
>
> On Wed, Nov 18, 2020 at 12:29:07AM +0800, Muchun Song wrote:
> > > ideally, we should be able to free PageTail if we change struct page in some way.
> > > Then we will save much more for 2MB hugetlb. but it seems it is not easy.
> >
> > Now for the 2MB HugrTLB page, we only free 6 vmemmap pages.
> > But your words woke me up. Maybe we really can free 7 vmemmap
> > pages. In this case, we can see 8 of the 512 struct page structures
> > has beed set PG_head flag. If we can adjust compound_head()
> > slightly and make compound_head() return the real head struct
> > page when the parameter is the tail struct page but with PG_head
> > flag set. I will start an investigation and a test.
>
> What are you thinking?
>
> static inline struct page *compound_head(struct page *page)
> {
> unsigned long head = READ_ONCE(page->compound_head);
>
> if (unlikely(head & 1))
> return (struct page *) (head - 1);
> + if (unlikely(page->flags & PG_head))
> + return (struct page *)(page[1]->compound_head - 1)
Yeah, I think so too. Maybe adding an align check is better.
+ if ((test_bit(PG_head, &page->flags) &&
+ IS_ALIGNED((unsigned long)page, PAGE_SIZE))
> return page;
> }
>
> ... because if it's that, there are code paths which also just test
> PageHead, and so we'd actually need to change PageHead to be something
> like:
Yeah, I also think that rework compound_head() and PageHead() is enough.
Thanks.
>
> static inline bool PageHead(struct page *page)
> {
> return (page->flags & PG_head) &&
> (page[1]->compound_head == (unsigned long)page + 1);
> }
>
> I'm not sure if that's worth doing -- there may be other things I
> haven't thought of.
--
Yours,
Muchun
