On Wed, Feb 24, 2021 at 10:22:38PM -0700, Yu Zhao wrote:
> On Thu, Feb 25, 2021 at 03:55:53AM +0000, Matthew Wilcox wrote:
> > On Wed, Feb 24, 2021 at 04:50:39PM -0700, Yu Zhao wrote:
> > > Let me work out something *conceptually* smaller first, and if you
> > > think folio is absolutely more suitable even for this specific issue,
> > > I'll go review and test the four patches you listed. Sounds good?
> >
> > Umm. It seems to me that no matter what you do, it'll be equivalent to
> > this, only without the type-safety?
>
> I'm thinking about something trivial but still very effective. So far
> I've only tested it with PG_{active,unevictable}, and I'm already
> seeing a 4KB gain less the 2KB loss from page_lru().
>
> I didn't go with this at the beginning because it's also time-
> consuming. I need to go over every single use of
> PG_{active,unevictable,swapbacked,lru}.
Well, yes. If you went with the folio, it'd also be typesafe.
What you've done here makes it a runtime error, and it's only detected
if you enable CONFIG_DEBUG_VM_PGFLAGS, which people don't do, in general.
> +++ b/fs/proc/task_mmu.c
> @@ -1712,6 +1712,7 @@ static void gather_stats(struct page *page, struct numa_maps *md, int pte_dirty,
> unsigned long nr_pages)
> {
> int count = page_mapcount(page);
> + struct page *head = compound_head(page);
>
> md->pages += nr_pages;
> if (pte_dirty || PageDirty(page))
... if you went full-on folio in this function, you could also make this
FolioDirty, saving another call to compound_head.
> @@ -1720,7 +1721,7 @@ static void gather_stats(struct page *page, struct numa_maps *md, int pte_dirty,
> if (PageSwapCache(page))
... ditto ...
> md->swapcache += nr_pages;
>
> - if (PageActive(page) || PageUnevictable(page))
> + if (PageActive(head) || PageUnevictable(head))
> md->active += nr_pages;
>
> if (PageWriteback(page))
... ditto...
