linux-kernel - Re: [PATCH v3 2/5] mm: fix missing cache flush for all tail pages of compound 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]

Date:   Wed, 2 Feb 2022 21:32:20 +0800
From:   Muchun Song <songmuchun@...edance.com>
To:     Mike Kravetz <mike.kravetz@...cle.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>, zi.yan@...rutgers.edu,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        David Rientjes <rientjes@...gle.com>,
        Lars Persson <lars.persson@...s.com>,
        Linux Memory Management List <linux-mm@...ck.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Xiongchun duan <duanxiongchun@...edance.com>,
        Zi Yan <ziy@...dia.com>
Subject: Re: [PATCH v3 2/5] mm: fix missing cache flush for all tail pages of
 compound page

On Wed, Feb 2, 2022 at 7:33 AM Mike Kravetz <mike.kravetz@...cle.com> wrote:
>
> On 1/31/22 08:02, Muchun Song wrote:
> > The D-cache maintenance inside move_to_new_page() only consider one page,
> > there is still D-cache maintenance issue for tail pages of compound page
> > (e.g. THP or HugeTLB).  THP migration is only enabled on x86_64, ARM64
> > and powerpc, while powerpc and arm64 need to maintain the consistency
> > between I-Cache and D-Cache, which depends on flush_dcache_page() to
> > maintain the consistency between I-Cache and D-Cache.  In theory, the
> > issue can be found on arm64 and powerpc.  HugeTLB migration is enabled
> > on arm, arm64, mips, parisc, powerpc, riscv, s390 and sh, while arm
> > has handled the compound page cache flush in flush_dcache_page(), but
> > most others do not.  In theory, the issue exists on many architectures.
> > Fix this by not using flush_dcache_folio() since it is not backportable.
> >
> > Fixes: 616b8371539a ("mm: thp: enable thp migration in generic path")
> > Fixes: 290408d4a250 ("hugetlb: hugepage migration core")
> > Signed-off-by: Muchun Song <songmuchun@...edance.com>
> > Reviewed-by: Zi Yan <ziy@...dia.com>
> > ---
> >  mm/migrate.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/migrate.c b/mm/migrate.c
> > index c9296d63878d..c418e8d92b9c 100644
> > --- a/mm/migrate.c
> > +++ b/mm/migrate.c
> > @@ -933,9 +933,12 @@ static int move_to_new_page(struct page *newpage, struct page *page,
> >               if (!PageMappingFlags(page))
> >                       page->mapping = NULL;
> >
> > -             if (likely(!is_zone_device_page(newpage)))
> > -                     flush_dcache_page(newpage);
> > +             if (likely(!is_zone_device_page(newpage))) {
> > +                     int i, nr = compound_nr(newpage);
> >
> > +                     for (i = 0; i < nr; i++)
> > +                             flush_dcache_page(newpage + i);
> > +             }
>
> As you have already noted elsewhere, the arm64 version of flush_dcache_page
> seems to handle this and flush the entire compound_page.  Is this going to
> flush the entire compound page compound_nr times?
>

Not flush cache compound_nr times but set PG_dcache_clean
on page->flags compound_nr times.  The definition of
flush_dcache_page() on arm64 is as follows.

void flush_dcache_page(struct page *page)
{
        if (test_bit(PG_dcache_clean, &page->flags))
                clear_bit(PG_dcache_clean, &page->flags);
}

So I have a plan to improve this, e.g. implement flush_dcache_folio()
on arm64 and replace those multiple dcache flush with
flush_dcache_folio().

void flush_dcache_folio(struct folio *folio)
{
       if (test_bit(PG_dcache_clean, &folio->flags))
               clear_bit(PG_dcache_clean, &folio->flags);
}

Thanks.