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 B907CC35278 for ; Tue, 12 Dec 2023 13:22:10 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id F20CD6B02E0; Tue, 12 Dec 2023 08:22:09 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id DE3DD6B02DF; Tue, 12 Dec 2023 08:22:09 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C835D6B02E0; Tue, 12 Dec 2023 08:22:09 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id B2B8D6B02DD for ; Tue, 12 Dec 2023 08:22:09 -0500 (EST) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 56026409D6 for ; Tue, 12 Dec 2023 13:22:09 +0000 (UTC) X-FDA: 81558229578.19.5562999 Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by imf27.hostedemail.com (Postfix) with ESMTP id 89D2840015 for ; Tue, 12 Dec 2023 13:22:06 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=none; spf=pass (imf27.hostedemail.com: domain of libaokun1@huawei.com designates 45.249.212.187 as permitted sender) smtp.mailfrom=libaokun1@huawei.com; dmarc=pass (policy=quarantine) header.from=huawei.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1702387327; a=rsa-sha256; cv=none; b=62kCekF5fF50/JJDIV1YKjB/E/hnRC9R9GuoWS4zdPNMpaAymA5iigl+YYkKwvcZBSac6/ P+KPJls3PjaRErDwRDN46Y35mye/hJjwKGo8A9ZY1spiceZwG9+fd7q6Y+pJ/pM1MZbqBq IdoG3KJGO57bMnr9FqRVTWEDWnU/qa8= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=none; spf=pass (imf27.hostedemail.com: domain of libaokun1@huawei.com designates 45.249.212.187 as permitted sender) smtp.mailfrom=libaokun1@huawei.com; dmarc=pass (policy=quarantine) header.from=huawei.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1702387327; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=UuBxJWUY6CqrGeDjzjDVP/h3Rbeku6JnyStR7mzZST8=; b=Jj1S45NA05R6tEPBi+P0YDG8hIlqcVGUNRiRmHr39dS9+vBcr5f0pfD7IoM7bi3QsjjUT7 1ddbgqUhO2/wIJAUqkrM5NAT+2CB1tvjDFATohwWAE4r4ZqBcS+i745O3i5hFMf9FSjNLZ +KvhAMDuvojErQkPfdz5nxWUKckUeV8= Received: from mail.maildlp.com (unknown [172.19.163.48]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4SqK4p4tzWzvS07; Tue, 12 Dec 2023 21:21:14 +0800 (CST) Received: from dggpeml500021.china.huawei.com (unknown [7.185.36.21]) by mail.maildlp.com (Postfix) with ESMTPS id 9296E1800BB; Tue, 12 Dec 2023 21:22:00 +0800 (CST) Received: from [10.174.177.174] (10.174.177.174) by dggpeml500021.china.huawei.com (7.185.36.21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35; Tue, 12 Dec 2023 21:21:59 +0800 Message-ID: <45178e80-bba4-4afe-2c74-50facd78169d@huawei.com> Date: Tue, 12 Dec 2023 21:21:59 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.1.2 Subject: Re: [RFC PATCH] mm/filemap: avoid buffered read/write race to read inconsistent data Content-Language: en-US To: Jan Kara CC: , , , , , , , , , , , , , , Baokun Li References: <20231212093634.2464108-1-libaokun1@huawei.com> <20231212124157.ew6q6jp2wsezvqzd@quack3> From: Baokun Li In-Reply-To: <20231212124157.ew6q6jp2wsezvqzd@quack3> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.174.177.174] X-ClientProxiedBy: dggems704-chm.china.huawei.com (10.3.19.181) To dggpeml500021.china.huawei.com (7.185.36.21) X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 89D2840015 X-Stat-Signature: mzc1eze7xwcujq6tbjtt1nuxyr5nbzo6 X-HE-Tag: 1702387326-961350 X-HE-Meta: U2FsdGVkX1+W2R6xeNGy7JyvvTp8Wc8yG8VEoE0nJxs0pVdteHyRhCJA6vYVMiVhNwch4Jt4qkTr4jeVdRmxe1WPQOjA3ugfNxNp46sJnRluhk7cMKdIhxkwyYnZ7UFaBZV/7JXao402HDHnCCZPUJBesLgbg83DoOM6gLl3/gjgDftHW6C2Xt4BUlrboJjMikykQD3/H2GL2CWSHqbwEnltDoU7lALwi/71OlG17W3JNXyvJnUWdxVcY4tmtRAJO1MVvTVBz8fhFi0TKMBYSy7U8AoGJrdV46T7k00eE6y1Q5SLSmRrHcJaIgceFFa4VjW4JOJZrEF/Uh6oy8NJ3hyNFMNZ94XWyPiERHJWxeeREuvcxkUDQXyrWVwoZ/gLlvVwh0iW0C1KDHT9kdxYPZOV6aSoPbpJgX7fs4k3jk2pIRqb4ihQdqKi2v2KfR9PfNA1/8GeDV+hywzTKz/oysKjOMgTzBFkES7ZTjvyMnex82HaHDKtTKtfbLL0e9TPh0gYsmKlTtdswLaA03+nIzikKKl9x6CRL0EbRwR9cglzEI9PZjAci/LHKHJ/3NUjHLU5Ru/Ha3pMBJBBYG+emkxQahBVuc2Oq/61s1Siatpmrk/rIk2FqklZkVHWlrZCJ4xWag/RN/MaF0jvBD/4agV3zr0isX9Y4q6KhWC1cAbkLc8haXFznikfvnHjfG4FzlP38Yk3AIT3whWhl9Myky1tHc/DPgWIK5zrmDyHZFxwJIbgkQZZ7+YwblWqoQZxoQnDPgwDQ6NeM1y6HoiGdXb4fgubnCInDiQIdcXaHu3IIjkBWlh5NvmiZMwJViCGDUXCOAbZLl0eminjY+u0Ri9b+DXkqh0j+L3mQz9+YTem0ReumyXRP9VQmMHjGbOjrVQ5axW04HWUd7xd9JOSaGVTTFLq+VGNb4YQLwDE8UgOgJR9w+jaSDrOYjBvPAp2r7kZ/YBL604coydAACk y+vLnoxR mBFpKG6pf4SKQ4G/Q8xf+udUg30HlJpJvwCZnzOo0aftyExqwqRbj24eWR8PbXmHYGX5knlBbFEZRlpThQ1TE9WbRdpIXr05nG/KxGpfZjKNLTJGY+YILcvT9RbHQxEnDhXaRkLSRHiRxdo2dIBV5jQQDyKYX2F6JcmSsN1qJmH9f5esD2hp52JwIeUHqkPsQNSLVALumKrI/iLyjLEDrW0WZ/Mfr0h3KY4YEPCciv6WlZJ4wcQD44+9eOOX4YQ61eyODjhC39/Igq6U5x0gSuuR2IW/CkVO2iihL 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: List-Subscribe: List-Unsubscribe: On 2023/12/12 20:41, Jan Kara wrote: > On Tue 12-12-23 17:36:34, Baokun Li wrote: >> The following concurrency may cause the data read to be inconsistent with >> the data on disk: >> >> cpu1 cpu2 >> ------------------------------|------------------------------ >> // Buffered write 2048 from 0 >> ext4_buffered_write_iter >> generic_perform_write >> copy_page_from_iter_atomic >> ext4_da_write_end >> ext4_da_do_write_end >> block_write_end >> __block_commit_write >> folio_mark_uptodate >> // Buffered read 4096 from 0 smp_wmb() >> ext4_file_read_iter set_bit(PG_uptodate, folio_flags) >> generic_file_read_iter i_size_write // 2048 >> filemap_read unlock_page(page) >> filemap_get_pages >> filemap_get_read_batch >> folio_test_uptodate(folio) >> ret = test_bit(PG_uptodate, folio_flags) >> if (ret) >> smp_rmb(); >> // Ensure that the data in page 0-2048 is up-to-date. >> >> // New buffered write 2048 from 2048 >> ext4_buffered_write_iter >> generic_perform_write >> copy_page_from_iter_atomic >> ext4_da_write_end >> ext4_da_do_write_end >> block_write_end >> __block_commit_write >> folio_mark_uptodate >> smp_wmb() >> set_bit(PG_uptodate, folio_flags) >> i_size_write // 4096 >> unlock_page(page) >> >> isize = i_size_read(inode) // 4096 >> // Read the latest isize 4096, but without smp_rmb(), there may be >> // Load-Load disorder resulting in the data in the 2048-4096 range >> // in the page is not up-to-date. >> copy_page_to_iter >> // copyout 4096 >> >> In the concurrency above, we read the updated i_size, but there is no read >> barrier to ensure that the data in the page is the same as the i_size at >> this point, so we may copy the unsynchronized page out. Hence adding the >> missing read memory barrier to fix this. >> >> This is a Load-Load reordering issue, which only occurs on some weak >> mem-ordering architectures (e.g. ARM64, ALPHA), but not on strong >> mem-ordering architectures (e.g. X86). And theoretically the problem > AFAIK x86 can also reorder loads vs loads so the problem can in theory > happen on x86 as well. According to what I read in the perfbook at the link below,  Loads Reordered After Loads does not happen on x86. pdf sheet 562 corresponds to page 550, Table 15.5: Summary of Memory Ordering https://mirrors.edge.kernel.org/pub/linux/kernel/people/paulmck/perfbook/perfbook-1c.2023.06.11a.pdf >> doesn't only happen on ext4, filesystems that call filemap_read() but >> don't hold inode lock (e.g. btrfs, f2fs, ubifs ...) will have this >> problem, while filesystems with inode lock (e.g. xfs, nfs) won't have >> this problem. >> >> Cc: stable@kernel.org >> Signed-off-by: Baokun Li >> --- >> mm/filemap.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/mm/filemap.c b/mm/filemap.c >> index 71f00539ac00..6324e2ac3e74 100644 >> --- a/mm/filemap.c >> +++ b/mm/filemap.c >> @@ -2607,6 +2607,9 @@ ssize_t filemap_read(struct kiocb *iocb, struct iov_iter *iter, >> goto put_folios; >> end_offset = min_t(loff_t, isize, iocb->ki_pos + iter->count); >> >> + /* Ensure that the page cache within isize is updated. */ > Barries have to be in pairs to work and it is a good practice to document > this. So here I'd have comment like: > /* > * Pairs with a barrier in > * block_write_end()->mark_buffer_dirty() or other page > * dirtying routines like iomap_write_end() to ensure > * changes to page contents are visible before we see > * increased inode size. > */ > > Honza That's a very accurate description! Thanks a lot! I will add this comment in the next version. >> + smp_rmb(); >> + >> /* >> * Once we start copying data, we don't want to be touching any >> * cachelines that might be contended: >> -- >> 2.31.1 >> Thanks! -- With Best Regards, Baokun Li .