linux-kernel - Re: [RFC PATCH 1/5] mm/mlock: rework mlock_count to use

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] [day] [month] [year] [list]

Message-ID: <CAJD7tkYV-ffQChd3dAtf+4BFg+oywYTO-n826+f8rHXofYx33w@mail.gmail.com>
Date:   Sun, 18 Jun 2023 11:32:14 -0700
From:   Yosry Ahmed <yosryahmed@...gle.com>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Yu Zhao <yuzhao@...gle.com>,
        "Jan Alexander Steffens (heftig)" <heftig@...hlinux.org>,
        Steven Barrett <steven@...uorix.net>,
        Brian Geffon <bgeffon@...gle.com>,
        "T.J. Alumbaugh" <talumbau@...gle.com>,
        Gaosheng Cui <cuigaosheng1@...wei.com>,
        Suren Baghdasaryan <surenb@...gle.com>,
        "Liam R. Howlett" <Liam.Howlett@...cle.com>,
        David Hildenbrand <david@...hat.com>,
        Jason Gunthorpe <jgg@...pe.ca>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        David Howells <dhowells@...hat.com>,
        Hugh Dickins <hughd@...gle.com>,
        Greg Thelen <gthelen@...gle.com>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 1/5] mm/mlock: rework mlock_count to use _mapcount for
 order-0 folios

On Sun, Jun 18, 2023 at 4:20 AM Matthew Wilcox <willy@...radead.org> wrote:
>
> On Sun, Jun 18, 2023 at 06:57:44AM +0000, Yosry Ahmed wrote:
> > @@ -337,6 +318,7 @@ struct folio {
> >                       atomic_t _entire_mapcount;
> >                       atomic_t _nr_pages_mapped;
> >                       atomic_t _pincount;
> > +                     atomic_t _mlock_count;
> >  #ifdef CONFIG_64BIT
> >                       unsigned int _folio_nr_pages;
> >  #endif
>
> You can't quite do this.  On 32-bt systems (I know, I know ...),
> we have:
>
> offset  page 0          page 1
> 0       flags           flags
> 4       lru             head
> 8       lru             dtor+order
> 12      mapping         entire_mapcount
> 16      index           nr_pages_napped
> 20      private         pincount
> 24      mapcount
> 28      refcount
>
> so it actually ends up overlapping page->mapcount on the second page,
> which is of course used for counting the number of PTEs which map
> that specific page.

Ah yeah, of course. In retrospect, it was very silly of me not to
notice given that I was modifying the mapcount handling code.

Thanks for pointing this out.

>
> I don't have a scenario where this would matter, but we are quite
> careful to only allocate order-2+ large folios, so I'd suggest putting
> it in page 2 instead of page 1.

Can you point me to the code that does so?

>
> I should probably add a comment to struct folio warning of this dragon.
> I thought the #ifdef CONFIG_64BIT would be enough to warn unwary
> passers-by of its presence, but a more explicit sign must be in order.
>

I saw the CONFIG_64BIT and spent some time trying to figure out why we
need it. I added the size of all explicit fields on 32-bit and they
were less than 32 bytes, so I was confused. I looked at the commit log
with no luck. I should have realized we should not overlay mapcount or
refcount. The presence of _mapcount_1/_refcount_1 fields could have
helped.

Anyway, we can also put it in page 1 inside #ifdef CONFIG_64BIT. We
will need a few extra #ifdef's in mm/mlock.c, and I think that's
pretty much it. For 32-bit, we can just keep piggybacking on mapcount
for all folios, I doubt a huge number of mappings is a concern for
32-bit anyway. I don't have a strong preference.