From: Muchun Song <songmuchun@bytedance.com>
To: Mike Kravetz <mike.kravetz@oracle.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
zi.yan@cs.rutgers.edu,
"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
David Rientjes <rientjes@google.com>,
Lars Persson <lars.persson@axis.com>,
Linux Memory Management List <linux-mm@kvack.org>,
LKML <linux-kernel@vger.kernel.org>,
Xiongchun duan <duanxiongchun@bytedance.com>,
Zi Yan <ziy@nvidia.com>
Subject: Re: [PATCH v3 2/5] mm: fix missing cache flush for all tail pages of compound page
Date: Wed, 2 Feb 2022 21:32:20 +0800 [thread overview]
Message-ID: <CAMZfGtULga8fC=i2n2jPHNjr5-45DTQjJqJ6FeZmy9qi91bXtw@mail.gmail.com> (raw)
In-Reply-To: <9fa0211a-aee2-22fb-d076-0464a4d8524c@oracle.com>
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.
next prev parent reply other threads:[~2022-02-02 13:33 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-31 16:02 [PATCH v3 0/5] Fix some cache flush bugs Muchun Song
2022-01-31 16:02 ` [PATCH v3 1/5] mm: thp: fix wrong cache flush in remove_migration_pmd() Muchun Song
2022-01-31 16:02 ` [PATCH v3 2/5] mm: fix missing cache flush for all tail pages of compound page Muchun Song
2022-02-01 13:24 ` Muchun Song
2022-02-01 23:32 ` Mike Kravetz
2022-02-02 13:32 ` Muchun Song [this message]
2022-01-31 16:02 ` [PATCH v3 3/5] mm: hugetlb: fix missing cache flush in copy_huge_page_from_user() Muchun Song
2022-01-31 16:02 ` [PATCH v3 4/5] mm: hugetlb: fix missing cache flush in hugetlb_mcopy_atomic_pte() Muchun Song
2022-02-01 22:23 ` Mike Kravetz
2022-02-02 13:58 ` Muchun Song
2022-01-31 16:02 ` [PATCH v3 5/5] mm: replace multiple dcache flush with flush_dcache_folio() Muchun Song
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAMZfGtULga8fC=i2n2jPHNjr5-45DTQjJqJ6FeZmy9qi91bXtw@mail.gmail.com' \
--to=songmuchun@bytedance.com \
--cc=akpm@linux-foundation.org \
--cc=duanxiongchun@bytedance.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=lars.persson@axis.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mike.kravetz@oracle.com \
--cc=rientjes@google.com \
--cc=zi.yan@cs.rutgers.edu \
--cc=ziy@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).