linux-kernel - Re: [PATCH] mm: thp: handle page cache THP correctly in PageTransCompoundMap

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: <alpine.LSU.2.11.1910221802270.2748@eggly.anvils>
Date:   Tue, 22 Oct 2019 18:31:27 -0700 (PDT)
From:   Hugh Dickins <hughd@...gle.com>
To:     Yang Shi <yang.shi@...ux.alibaba.com>
cc:     Hugh Dickins <hughd@...gle.com>, aarcange@...hat.com,
        kirill.shutemov@...ux.intel.com, gavin.dg@...ux.alibaba.com,
        akpm@...ux-foundation.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm: thp: handle page cache THP correctly in
 PageTransCompoundMap

On Tue, 22 Oct 2019, Yang Shi wrote:
> On 10/22/19 3:27 PM, Hugh Dickins wrote:
> > 
> > I completely agree that the current PageTransCompoundMap() is wrong.
> > 
> > A fix for that is one of many patches I've not yet got to upstreaming.
> > Comparing yours and mine, I'm worried by your use of PageDoubleMap(),
> > because really that's a flag for anon THP, and not properly supported
> > on shmem (or now I suppose file) THP - I forget the details, is it
> > that it sometimes gets set, but never cleared?  Generally, we just
> > don't refer to PageDoubleMap() on shmem THPs (but there may be
> > exceptions: sorting out the THP mapcount maze, and eliminating
> > PageDoubleMap(), is one of my long-held ambitions, not yet reached).
> > 
> > Here's the patch I've been carrying, but it's from earlier, so I
> > should warn that I've done no more than build-testing it on 5.4,
> > and I'm too far away from these issues at the moment to be able to
> > make a good judgement or argue for it - I hope you and others can
> > decide which patch is the better.  I should also add that we're
> > barely using PageTransCompoundMap() at all: at best it can only
> > give a heuristic guess as to whether the page is pmd-mapped in
> > any particular case, and we preferred to take forward the KVM
> > patches we posted back in April 2016, plumbing hva down to where
> > it's needed - though of course those are somewhat different now.
> 
> Thanks for catching this. I was definitely thinking about using
> compount_mapcount instead of DoubleMap flag when I was working the patch. I
> just simply thought it would change less file by using DoubleMap flag but I
> didn't notice it was kind of unbalanced for file THP.
> 
> With the unbalanced DoubleMap flag, it sounds better to use
> compound_mapcount.

Yes: no doubt PageDoubleMap could be fixed on shmem+file, but I have no
interest in doing that, because it's just unnecessary overhead for them.
(They have their own overhead, of subpage mapcounting for pmd: which is
something to eliminate and unify with anon when I get around to it.)

> 
> Thanks for sharing your patch, I'm going to rework v2 by using
> compound_mapcount. Do you mind I might steal your patch?

Please do! One less for me to worry about, thanks.

> 
> I'm supposed we'd better fix this bug regardless of whether you would like to
> move forward your KVM patches.

Absolutely. There remain a few other uses of PageTransCompoundMap
anyway, and I really wanted this outright mm fix to go in before
re-submitting AndresLC's KVM patch (I'll ask a KVM-savvy colleague
to take that over, Cc'ing you, once the mm end is correct).

Hugh