lkml.org 
[lkml]   [2022]   [Feb]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [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@oracle.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@bytedance.com>
> > Reviewed-by: Zi Yan <ziy@nvidia.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.

\
 
 \ /
  Last update: 2022-02-02 14:33    [W:0.100 / U:1.604 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site