linux-kernel - Re: [PATCH] mm: test page->flags directly in page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210225121207.GY2858050@casper.infradead.org>
Date:   Thu, 25 Feb 2021 12:12:07 +0000
From:   Matthew Wilcox <willy@...radead.org>
To:     Yu Zhao <yuzhao@...gle.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>, vbabka@...e.cz,
        alex.shi@...ux.alibaba.com, guro@...com, hannes@...xchg.org,
        hughd@...gle.com, linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        mhocko@...nel.org, vdavydov.dev@...il.com
Subject: Re: [PATCH] mm: test page->flags directly in page_lru()

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...