From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id C16D0C28CF5 for ; Wed, 26 Jan 2022 03:30:25 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 067CF6B0071; Tue, 25 Jan 2022 22:30:25 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 014096B0075; Tue, 25 Jan 2022 22:30:24 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E1DD26B0078; Tue, 25 Jan 2022 22:30:24 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0060.hostedemail.com [216.40.44.60]) by kanga.kvack.org (Postfix) with ESMTP id D34986B0071 for ; Tue, 25 Jan 2022 22:30:24 -0500 (EST) Received: from smtpin09.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 8936894FDE for ; Wed, 26 Jan 2022 03:30:24 +0000 (UTC) X-FDA: 79071010368.09.AD43900 Received: from mail-yb1-f171.google.com (mail-yb1-f171.google.com [209.85.219.171]) by imf13.hostedemail.com (Postfix) with ESMTP id 0AEA82000D for ; Wed, 26 Jan 2022 03:30:23 +0000 (UTC) Received: by mail-yb1-f171.google.com with SMTP id k17so12343269ybk.6 for ; Tue, 25 Jan 2022 19:30:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance-com.20210112.gappssmtp.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=yj8bOFT13velpoWfDU2cuSAdvEHEJWwY0FpIeFHzEoI=; b=4oYU6dNUqbgGN0ScDndA29BevVKWSKrxlRuvf5uChM8k8GYdPoF0RgDms9jihcwd0a Eu7SCZDb/TL8m65JxGr3tOA2CVViyGn5vfhCEbcalEnfoAzs56sl294vJAwvdQ26MyKQ cz0RryADg5hQtJ7FbPNTTdmgb4ClgDQkQMSYl3TCSbmauJrAr2cGoU6TxIKWgYyfdy75 WxMY4d4JNfdz/B9/ArkgQzD6dXTp9u387VgM0amOHQMF759XLyd9nHaf4I6vbP2CZ/oL 9Bx0YvSsvzlLzIGEeIb38gPQC1suayuKWmNFFUQ1rAEng739gqoSboNQXS0UmAU/xBqr HNtg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=yj8bOFT13velpoWfDU2cuSAdvEHEJWwY0FpIeFHzEoI=; b=IeV56WabIEX6hZdeZC/+b2E08Y6B0ywQ+Coe6GmOJ7V0Wlu6Y+nQLYFJiUg4uRPvjP LpYUBhoggIQg2hlGY9LO/NPViGWmwmkMDgXztyijlVP44EwxaTIxEzTbV61dewokBiGb TJi6ncOe525Es0dBxmO2YQm2mu5FowfzY6LOSlAtLR5kznb/hH+2EYqpqZdW2ycFvB74 TnIj85o3dtl0nOSsuL3JMEQlT3vAvGWyDwAjL7HlMhtUYn+hvnOkOk9GQ+49Ly2P8nMu qJ6uI7LPrzyxK4FfA+S3sLwmTwGpEGpi74NTmayFwFdUugRHf7HjJy1si20r/0spf652 f7Yg== X-Gm-Message-State: AOAM5312o0TnDQmB3xD1TURIl8/VafA9ylftnaNx9VOS14LpAhIwYA5G prCb2IKg5ok7QsYWaHxDNGV0rEeC6QAif2bPblFjWA== X-Google-Smtp-Source: ABdhPJyu2HgBCxLYWh9tJPDbWFsw0PeAfgccjCbZ1cKnBYjkmHiq8RwI4O5czsYvRSibufTvt89vUvtbytZWMZ8L99I= X-Received: by 2002:a25:341:: with SMTP id 62mr35495944ybd.132.1643167823311; Tue, 25 Jan 2022 19:30:23 -0800 (PST) MIME-Version: 1.0 References: <20220124051752.83281-1-songmuchun@bytedance.com> <20220124051752.83281-2-songmuchun@bytedance.com> <4d5044e7-cac9-b6e6-1467-59ea6010b0f5@google.com> <5D9B52E1-A74B-4964-AACF-ADB91536C4C0@nvidia.com> <7D7EB27F-DEA7-41AA-B24E-B61A2A1A5F07@nvidia.com> <039a9107-756e-bc0a-6e72-fbe08408de38@oracle.com> In-Reply-To: <039a9107-756e-bc0a-6e72-fbe08408de38@oracle.com> From: Muchun Song Date: Wed, 26 Jan 2022 11:29:47 +0800 Message-ID: Subject: Re: [PATCH v2 2/2] mm: fix missing cache flush for all tail pages of THP To: Mike Kravetz Cc: Zi Yan , David Rientjes , Andrew Morton , "Kirill A. Shutemov" , Linux Memory Management List , LKML , Xiongchun duan , Lars Persson Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: o5womp61sqngct6oqtpg1r6qmi4qx6u4 X-Rspam-User: nil Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=bytedance-com.20210112.gappssmtp.com header.s=20210112 header.b=4oYU6dNU; spf=pass (imf13.hostedemail.com: domain of songmuchun@bytedance.com designates 209.85.219.171 as permitted sender) smtp.mailfrom=songmuchun@bytedance.com; dmarc=pass (policy=none) header.from=bytedance.com X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: 0AEA82000D X-HE-Tag: 1643167823-983235 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, Jan 26, 2022 at 5:24 AM Mike Kravetz wrot= e: > > On 1/24/22 22:01, Muchun Song wrote: > > On Tue, Jan 25, 2022 at 10:42 AM Zi Yan wrote: > >> > >> On 24 Jan 2022, at 20:55, Muchun Song wrote: > >> > >>> On Tue, Jan 25, 2022 at 3:22 AM Zi Yan wrote: > >>>> > >>>> On 24 Jan 2022, at 13:11, David Rientjes wrote: > >>>> > >>>>> On Mon, 24 Jan 2022, Muchun Song wrote: > >>>>> > >>>>>> The D-cache maintenance inside move_to_new_page() only consider on= e page, > >>>>>> there is still D-cache maintenance issue for tail pages of THP. Fi= x this > >>>>>> by not using flush_dcache_folio() since it is not backportable. > >>>>>> > >>>>> > >>>>> The mention of being backportable suggests that we should backport = this, > >>>>> likely to 4.14+. So should it be marked as stable? > >>>> > >>>> Hmm, after more digging, I am not sure if the bug exists. For THP mi= gration, > >>>> flush_cache_range() is used in remove_migration_pmd(). The flush_dca= che_page() > >>>> was added by Lars Persson (cc=E2=80=99d) to solve the data corruptio= n on MIPS[1], > >>>> but THP migration is only enabled on x86_64, PPC_BOOK3S_64, and ARM6= 4. > >>> > >>> I only mention the THP case. After some more thinking, I think the Hu= geTLB > >>> should also be considered, Right? The HugeTLB is enabled on arm, arm6= 4, > >>> mips, parisc, powerpc, riscv, s390 and sh. > >>> > >> > >> +Mike for HugeTLB > >> > >> If HugeTLB page migration also misses flush_dcache_page() on its tail = pages, > >> you will need a different patch for the commit introducing hugetlb pag= e migration. > > > > Agree. I think arm (see the following commit) has handled this issue, w= hile most > > others do not. > > > > commit 0b19f93351dd ("ARM: mm: Add support for flushing HugeTLB pages= .") > > > > But I do not have any real devices to test if this issue exists on othe= r archs. > > In theory, it exists. > > > > Thanks for adding me to the discussion. > > I agree that this issue exists at least in theory for hugetlb pages as we= ll. > This made me look at other places with similar code for hugetlb. i.e. > Allocating a new page, copying data to new page and then establishing a > mapping (pte) to the new page. Hi Mike, Thanks for looking at this. > > - hugetlb_cow calls copy_user_huge_page() which ends up calling > copy_user_highpage that includes dcache flushing of the target for some > architectures, but not all. copy_user_page() inside copy_user_highpage() is already considering the cache maintenance on different architectures, which is documented in Documentation/core-api/cachetlb.rst. So there are no problems in this case. > - userfaultfd calls copy_huge_page_from_user which does not appear to do > any dcache flushing for the target page. Right. The new page should be flushed before setting up the mapping to the user space. > Do you think these code paths have the same potential issue? The latter does have the issue, the former does not. The fixes may look like the following: diff --git a/mm/hugetlb.c b/mm/hugetlb.c index a1baa198519a..828240aee3f9 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -5819,6 +5819,7 @@ int hugetlb_mcopy_atomic_pte(struct mm_struct *dst_mm= , goto out; } folio_copy(page_folio(page), page_folio(*pagep)); + flush_dcache_folio(page_folio(page)); put_page(*pagep); *pagep =3D NULL; } diff --git a/mm/memory.c b/mm/memory.c index e8ce066be5f2..ff6f48cdcc48 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -5400,6 +5400,7 @@ long copy_huge_page_from_user(struct page *dst_page, kunmap(subpage); else kunmap_atomic(page_kaddr); + flush_dcache_page(subpage); ret_val -=3D (PAGE_SIZE - rc); if (rc) Thanks.