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;
Message-ID: <039a9107-756e-bc0a-6e72-fbe08408de38@oracle.com>
Date:   Tue, 25 Jan 2022 13:24:05 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH v2 2/2] mm: fix missing cache flush for all tail pages of
 THP
Content-Language: en-US
To:     Muchun Song <songmuchun@bytedance.com>, Zi Yan <ziy@nvidia.com>
Cc:     David Rientjes <rientjes@google.com>,
        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>,
        Xiongchun duan <duanxiongchun@bytedance.com>,
        Lars Persson <lars.persson@axis.com>
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>
 <CAMZfGtXsLyagpo8rM6RmayAFR_hgk0987X1usxYWRZLeA5H45Q@mail.gmail.com>
 <7D7EB27F-DEA7-41AA-B24E-B61A2A1A5F07@nvidia.com>
 <CAMZfGtVCUbTa6xv2R9x+_a5_dK4ZLUX8ECNkF8mkJgEy4kdnfQ@mail.gmail.com>
From:   Mike Kravetz <mike.kravetz@oracle.com>
In-Reply-To: <CAMZfGtVCUbTa6xv2R9x+_a5_dK4ZLUX8ECNkF8mkJgEy4kdnfQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?U2w0dEdmcWVvK3A3dUo3b3FtMFQxN2pNOGNIN0JHVEJCRmFWQTZyUGUwMGc0?=
 =?utf-8?B?TUlsSkJmaTRrNnQ1MzFsaVJZL24vWjlFYW50akNRQ1RHT1I5b0JMV0REV0M5?=
 =?utf-8?B?UU9QRzZOWTkyZkRMbjRORjkyaEdNTTBvekc1OG9jLzZqWGVFOEF1aXc3VWxL?=
 =?utf-8?B?ZkF1OW9PK0hlVHVBWllrdk1oWVNMQnVqMlFvM0ExMC90NVFXY2F0aXNpRjJL?=
 =?utf-8?B?eGFZbjBsYjhJODM2K3dUYjhPcWg0MVB5V2s5QkJxUWFlbklIQzY1RExGQnN6?=
 =?utf-8?B?SFBQUXJLL1ZhTmZ6emlRTnNqYVp6ampyK0VKejkzakVidDdFaWV6TkFDL3Qw?=
 =?utf-8?B?cUtHbU1QSkZZVlFDaGZIRURWOHNWdG9lVTQxSVl1WmxkU1BPaThFT3oyUFdE?=
 =?utf-8?B?eWc3ZkZob1dxV05SZmpxY3JOQWd2U2dkQXBmOWsxZlVvQ3NIc0YwY05aSnVD?=
 =?utf-8?B?WDFjZFJXYjZGZDRSWFRyR2RtRzBWWjNYNmNnQjdZWURaWGMzM0VnZVBBZCt6?=
 =?utf-8?B?VlNRMmVkQjlPcDlCWVBQOHhSTmVtRWdidjRCZ3ZiMjFtYjE2MStWRVpaTnJE?=
 =?utf-8?B?Tkd2YnJtQlMyZzV0UXlxKytkVnZoMmJvNjVyUUR4c3BKQXlaYnRtMGFMT2dY?=
 =?utf-8?B?T3VEM3RLKzVKREFKUzl5amZHWU9lcEhJenY3UXB4LzArOExDMlpzUUZqazc5?=
 =?utf-8?B?VG40NlF3WkRMaktPL1ljNU42YlpXS2dWdkNyaGZCemdzMHZkMTdhM1R6SWE3?=
 =?utf-8?B?L0ZiUnA3eWFPbEtQNHdsUjk1d3UvZGJHTE9zaTJWSzNBQzl0aVhMMGx1MktS?=
 =?utf-8?B?UEEyeGdwWTdKK0Y4UmZPNmVBZzhKbkVLY2FGd0tPcGFyVzA5YmlpZjFWVGNE?=
 =?utf-8?B?eHZId296Z0JxS1JWN3U1aDZrVk5uYkNST2hGV3FXMnNPRFFyTkozWkZGQ1R1?=
 =?utf-8?B?bFoxenI5bmdPSTNVUHhFSWdRQitJaG81blRoR0xtU0FrWUgyZGZyRHErcDZp?=
 =?utf-8?B?Z0RRNHlSRlhsWkJyakFxci9KQ0p2eHQwc1c4QVZ1U2ZOZm02d3RjSkRwbkZF?=
 =?utf-8?B?SVRMM2Y3TG4wSDdqbXd0N3RmSjhzSDMzWThFTzFST3hzQy91dE5QL2NCcmRK?=
 =?utf-8?B?ZHlkVFgyb01pcEJzQUI2Mlo2MkFvWnlHUTlONXluOHJvb0hXY05wckJlc3dl?=
 =?utf-8?B?UVBwY1R1TzdEQ3FiK2xwa09vK21ub2N1TUFZVjRFZHZtL0g4ZzFDYi91NWZF?=
 =?utf-8?B?SHVCY3RFbjdVVkpCdWl1S0VFN3pPUGUzRERzU0l0MnhXOTlQWnBFNG5QSVpa?=
 =?utf-8?B?Y1E2MmN0MGhCZ25tRGY3bE5VVEpEWC93b1dUSmhQV3ZHVDk0d25jOHc5OEM5?=
 =?utf-8?B?a1VnMFFOTVd2dmptNlY5M05NM1pobklPYnFCdTh4QytGM1psMlVNamNURk5C?=
 =?utf-8?B?WnB5YlltKy9Bck14ZFlXejBPeWszMTJqUmV1OFVrMENNSE5aY1cvbUtNTzl3?=
 =?utf-8?B?eWdVcUQ1MHBDU0FaRlg2bEdtbVhGUUpaN3hjdTMrclIwQkdKSnFPckpZMVNN?=
 =?utf-8?B?SG1ORmtHd3h1cjRxendTNkdFVUNrZGtTTTh2M1VsWVJpazRNRVgydnJmUmVR?=
 =?utf-8?B?MjlBZnpFZDBSYnhjNm1KZWtweS9SWkRhV3puWFR2TjhLaE1mbXVaSUF3c3M5?=
 =?utf-8?B?Ti9DZFRHWlM1MHlBclVNODJPOThUNjJ4UW0ybnJnY2xyZUZvcjliaDFXR1c1?=
 =?utf-8?B?TUFCNXM3akxIQ0k2UzE5Q3orK1JjK2dGWWhESDBYRkFnMk5DKzN2dFBnTy9I?=
 =?utf-8?B?QjQrbXRKcWppMkwxUGd6OGcyTzVBMFM1SjNoa2FVK1NNZllOWGtmQ2Q1dWI4?=
 =?utf-8?B?Rmt2bWkzT1I1Q2xLQnhER2d4dUgyS1U2dVR1RWZHYUoyUVVKUThFQ2Znd2p4?=
 =?utf-8?B?MnpCNlcwdFVDcGg3QVZoYmlhcVVYcVJzL21OWGlpcmxqUWJOTjVWS3VTZEtS?=
 =?utf-8?B?NTRvKzdRRVZmVVY0NWFPVEh3ZExaNk9xSTQrakFWTE1kcnZXbEViT1BBTGd0?=
 =?utf-8?B?QVU1RW53OHl6UzA5clk0blRpMURzbktuYXJ2UFFLbHBxUlZ3czJSTDBTVnRO?=
 =?utf-8?B?MGxOUkJibTROcGowM00xazhEM1pHSjZ5QkpzeVZjWmg2T1BrcjVibHNoSlV3?=
 =?utf-8?Q?6WgjzGLZA5PGDem4ogf8jgA=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e44229e2-da4c-4c32-0123-08d9e049062e
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Jan 2022 21:24:09.3048
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: XNjy35nu98rowh9JEgv/cXkjaFTuNW8IPJo6/vreexQd2lXQadXEY3YfCn947Eg1Eta6ERsPA3ogmEMaz2urEQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR10MB3112
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10238 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 mlxscore=0 malwarescore=0
 spamscore=0 phishscore=0 suspectscore=0 adultscore=0 mlxlogscore=999
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2201110000
 definitions=main-2201250127
X-Proofpoint-GUID: v9MJolAaPBGbmcoPrQ0WfgURf3NWRYgk
X-Proofpoint-ORIG-GUID: v9MJolAaPBGbmcoPrQ0WfgURf3NWRYgk
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 1/24/22 22:01, Muchun Song wrote:
> On Tue, Jan 25, 2022 at 10:42 AM Zi Yan <ziy@nvidia.com> wrote:
>>
>> On 24 Jan 2022, at 20:55, Muchun Song wrote:
>>
>>> On Tue, Jan 25, 2022 at 3:22 AM Zi Yan <ziy@nvidia.com> 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 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 migration,
>>>> flush_cache_range() is used in remove_migration_pmd(). The flush_dcache_page()
>>>> was added by Lars Persson (cc’d) 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 HugeTLB
>>> 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 pages,
>> you will need a different patch for the commit introducing hugetlb page migration.
> 
> 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 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 well.
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.

- 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.
- userfaultfd calls copy_huge_page_from_user which does not appear to do
  any dcache flushing for the target page.

Do you think these code paths have the same potential issue?
-- 
Mike Kravetz