Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20220121081345.80320-1-songmuchun@bytedance.com>
 <20220121081345.80320-2-songmuchun@bytedance.com> <80208467-D04B-4215-9C5A-8BDBDB365853@nvidia.com>
In-Reply-To: <80208467-D04B-4215-9C5A-8BDBDB365853@nvidia.com>
From:   Muchun Song <songmuchun@bytedance.com>
Date:   Sat, 22 Jan 2022 06:25:40 +0800
Message-ID: <CAMZfGtWF37O0hHTJtwCUUYuS-bdf8DxbuXhjwOepskETcOOyaA@mail.gmail.com>
Subject: Re: [PATCH 2/2] mm: fix missing cache flush for all tail pages of THP
To:     Zi Yan <ziy@nvidia.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
        Linux Memory Management List <linux-mm@kvack.org>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Jan 21, 2022 at 10:59 PM Zi Yan <ziy@nvidia.com> wrote:
>
> On 21 Jan 2022, at 3:13, 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 THP. Fix this
> > by using flush_dcache_folio().
> >
> > Fixes: 616b8371539a ("mm: thp: enable thp migration in generic path")
> > Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> > ---
> >  mm/migrate.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/mm/migrate.c b/mm/migrate.c
> > index c9296d63878d..daf2b3508670 100644
> > --- a/mm/migrate.c
> > +++ b/mm/migrate.c
> > @@ -934,8 +934,7 @@ static int move_to_new_page(struct page *newpage, struct page *page,
> >                       page->mapping = NULL;
> >
> >               if (likely(!is_zone_device_page(newpage)))
> > -                     flush_dcache_page(newpage);
> > -
> > +                     flush_dcache_folio(page_folio(newpage));
> >       }
> >  out:
> >       return rc;
> > --
> > 2.11.0
>
> Yes, the entire THP should be flushed. But it is better
> to use a for loop instead of the folio variant, so that the patch
> can be ported easily to the stable trees. The for loop can be
> converted later when the whole function is converted to use folio.
>

Agree. Will do. Thanks for your review.