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 441C5C433F5 for ; Tue, 25 Jan 2022 06:27:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A0EAB6B0088; Tue, 25 Jan 2022 01:27:57 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 997606B0089; Tue, 25 Jan 2022 01:27:57 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 837946B008A; Tue, 25 Jan 2022 01:27:57 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0047.hostedemail.com [216.40.44.47]) by kanga.kvack.org (Postfix) with ESMTP id 701BC6B0088 for ; Tue, 25 Jan 2022 01:27:57 -0500 (EST) Received: from smtpin23.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 24D9F92EC4 for ; Tue, 25 Jan 2022 06:27:57 +0000 (UTC) X-FDA: 79067828994.23.1BE5FD0 Received: from mail-ua1-f41.google.com (mail-ua1-f41.google.com [209.85.222.41]) by imf24.hostedemail.com (Postfix) with ESMTP id 08F33180006 for ; Tue, 25 Jan 2022 06:27:55 +0000 (UTC) Received: by mail-ua1-f41.google.com with SMTP id c36so35474462uae.13 for ; Mon, 24 Jan 2022 22:27:55 -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=dME7WGjycfCPYxmjYIqrFH1YXeqs+InmpqysLjTPlzM=; b=amYBEOB2sMOECjGPhfMEQ/UEOLKvNfub12ZZk6Kgde8UISLhPq4P1tKSS/hAze0p9j nQK8P40wup1JaOHr9cpNR8PdlxRSiqyf6zbqlpbP8jTRjj+5Zxjq5qC1BYhFd6XGobq1 wvuQK1Y6HWvKRFS7JckDgPpQ/KCUvYBDwejHVepaDsKNPpdRwME7gDaIlm1ibJY4Uv71 EucJgwOtoNl1GoJUoH5SWV70wr2EDE9STpBxi2NGfryDM7a3IXoulow8sGajQVJoYgzD CZbXO6nmX+RyjSa3++7xyC348NY/OUAYAfSdoUbPmrUMNKmTpbpA9sBTihbzEsH+87xS KtDw== 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=dME7WGjycfCPYxmjYIqrFH1YXeqs+InmpqysLjTPlzM=; b=kluBbNK8lrfjM1JjO9wb6tRN/FdzjO/VwAC4V1CKm9dMASm9yEP03bTcyyKsrwsYWV EsjGLu7qmiHmosZI/UoJiFtdyAPxLYc/Uj6PYZee3tIy7GWQ+RLNU9vwv0BI6dYlNmDr nWUnAoOyQ9auZtTpAn8iRxsxHs/8f445A1c6c4Ze6X7MSfafcjNws8hoffLicqRgLfYr 44KY/JIbbCwvs0wuSMIL0txaFdJrMWx6P81tSF/Vr5xyntQzF/vaBGEZjjkkQvGHBQUl XicB2XwrIhjsOQsGXAeEkIehthWWqZ3iJIhR/Of0DcOamHtv6gm1ly6Tc/VKs3QS5aMO ohRg== X-Gm-Message-State: AOAM531LIKkUUH1Kd6IwuM8QrEDISsTt9E0mFq6kcCaB/bntYPZDAkpf qDyg3XkqSksjCtafHNP3A1ZcZRGTo4Cz49xBVgsB7PZBxqFzRw== X-Google-Smtp-Source: ABdhPJxNPvQ168PEeWD7QNNQOJuXjSDRCMLiP0b4oKX2jpsdp1/k4rOpI6Ja8FztN3a3gYj8rhYa9MlsOo7okUFG6wg= X-Received: by 2002:a05:6902:100c:: with SMTP id w12mr12601077ybt.317.1643090555052; Mon, 24 Jan 2022 22:02:35 -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> In-Reply-To: <7D7EB27F-DEA7-41AA-B24E-B61A2A1A5F07@nvidia.com> From: Muchun Song Date: Tue, 25 Jan 2022 14:01:59 +0800 Message-ID: Subject: Re: [PATCH v2 2/2] mm: fix missing cache flush for all tail pages of THP To: Zi Yan Cc: David Rientjes , Andrew Morton , "Kirill A. Shutemov" , Linux Memory Management List , LKML , Xiongchun duan , Lars Persson , Mike Kravetz Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 08F33180006 X-Stat-Signature: rhroxfbd9etr7hiphjxknwbq7yai1hhs Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=bytedance-com.20210112.gappssmtp.com header.s=20210112 header.b=amYBEOB2; dmarc=pass (policy=none) header.from=bytedance.com; spf=pass (imf24.hostedemail.com: domain of songmuchun@bytedance.com designates 209.85.222.41 as permitted sender) smtp.mailfrom=songmuchun@bytedance.com X-Rspam-User: nil X-HE-Tag: 1643092075-84571 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 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 one = page, > >>>> there is still D-cache maintenance issue for tail pages of THP. Fix = this > >>>> by not using flush_dcache_folio() since it is not backportable. > >>>> > >>> > >>> The mention of being backportable suggests that we should backport th= is, > >>> 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 migr= ation, > >> flush_cache_range() is used in remove_migration_pmd(). The flush_dcach= e_page() > >> was added by Lars Persson (cc=E2=80=99d) to solve the data corruption = on MIPS[1], > >> but THP migration is only enabled on x86_64, PPC_BOOK3S_64, and ARM64. > > > > I only mention the THP case. After some more thinking, I think the Huge= TLB > > should also be considered, Right? The HugeTLB is enabled on arm, arm64, > > mips, parisc, powerpc, riscv, s390 and sh. > > > > +Mike for HugeTLB > > If HugeTLB page migration also misses flush_dcache_page() on its tail pag= es, > you will need a different patch for the commit introducing hugetlb page m= igration. Agree. I think arm (see the following commit) has handled this issue, while= 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 other ar= chs. In theory, it exists. > > >> > >> To make code more consistent, I guess flush_cache_range() in remove_mi= gration_pmd() > >> can be removed, since it is superseded by the flush_dcache_page() belo= w. > > > > From my point of view, flush_cache_range() in remove_migration_pmd() is > > a wrong usage, which cannot replace flush_dcache_page(). I think the co= mmit > > c2cc499c5bcf ("mm compaction: fix of improper cache flush in migration = code") > > , which is similar to the situation here, can offer more infos. > > > > Thanks for the information. That helps. But remove_migration_pmd() did no= t cause > any issue at the commit pointed by Fixes but at the commit which enabled = THP > migration on IBM and ARM64, whichever came first. > > IIUC, there will be different versions of the fix targeting different sta= ble > trees: > > 1. pre-4.14, THP migration did not exist: you will need to fix the use of > flush_dcache_page() at that time for HugeTLB page migration. Both flushin= g > dcache page for all subpages and moving flush_dcache_page from > remove_migration_pte() to move_to_new_page(). 4.9 and 4.4 are affected. > But EOL of 4.4 is next month, so you might skip it. > > 2. 4.14 to before device public page is removed: your current fix will no= t > apply directly, but the for loop works. flush_cache_range() in > remove_migration_pmd() should be removed, since it is dead code based on > the commit you mentioned. It might not be worth the effort to find when > IBM and ARM64 enable THP migration. > > 3. after device public page is removed: your current fix will apply clean= ly > and the removal of flush_cache_range() in remove_migration_pmd() should > be added. > > Let me know if it makes sense. Make sense. Thanks.