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 656C3C54E58 for ; Mon, 25 Mar 2024 09:22:49 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B73046B0085; Mon, 25 Mar 2024 05:22:48 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B222F6B0087; Mon, 25 Mar 2024 05:22:48 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A112A6B0088; Mon, 25 Mar 2024 05:22:48 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 934186B0085 for ; Mon, 25 Mar 2024 05:22:48 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id CA2D8C0B30 for ; Mon, 25 Mar 2024 09:22:47 +0000 (UTC) X-FDA: 81935021574.02.7B76F99 Received: from out-187.mta0.migadu.com (out-187.mta0.migadu.com [91.218.175.187]) by imf08.hostedemail.com (Postfix) with ESMTP id AA496160013 for ; Mon, 25 Mar 2024 09:22:45 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=iUaRKlNS; spf=pass (imf08.hostedemail.com: domain of chengming.zhou@linux.dev designates 91.218.175.187 as permitted sender) smtp.mailfrom=chengming.zhou@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1711358566; 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:dkim-signature; bh=eOlZm0+5XlJcPzDimS2ld8aXFCHL2fRhCLxa+S5e368=; b=3xCbFO0EPatMrfIRS+Tb9Ya3Lh7FflKb6PhXBN2+FgtBIopnDHzpPzPEk4b5tcsMNDwIDR Mf8ONqTuCsISo/cn6jOIA6Rgn/wOORfoPl0VJ1rIAyREJxCDQjSun1Bs88BNLlBUy/Cxgr v1Ec08Y132yCm9SDFMCG6JhF2Ne/C6o= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1711358566; a=rsa-sha256; cv=none; b=2ncevBnUhAz/Y5w8Kagsj5dG+14qcynEI2FvhEy2mP1Gl4/VLB9757wM+keT2nNKPSNRkq +G3xHdZSniOjt/fStg+gL/Smp/Ajn4GxTZh0RHYxRDc1LJnOepVifIkf77G75r8HYexCv4 2rCtVoe/IpF1XqlyJAGp+1cSxsg4v5Y= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=iUaRKlNS; spf=pass (imf08.hostedemail.com: domain of chengming.zhou@linux.dev designates 91.218.175.187 as permitted sender) smtp.mailfrom=chengming.zhou@linux.dev; dmarc=pass (policy=none) header.from=linux.dev Message-ID: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1711358563; h=from:from: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=eOlZm0+5XlJcPzDimS2ld8aXFCHL2fRhCLxa+S5e368=; b=iUaRKlNSkeZqVpQBNnWAM8JvB4mvcqL9WsD9sTLOx62VlXRPwbRfz0wOYGYQyTqbsBg6jG FhFVtjvuIJFv/Q3a2J6GxDPC79M19bffV9YEN8L54jK8O5kXK6O55bKVjmnHxZPvt8zzb3 KYbBB6abu5qZhUZueEtLBSOgsxnzI3c= Date: Mon, 25 Mar 2024 17:22:25 +0800 MIME-Version: 1.0 Subject: Re: [PATCH] mm: zswap: fix data loss on SWP_SYNCHRONOUS_IO devices Content-Language: en-US To: Yosry Ahmed Cc: Barry Song <21cnbao@gmail.com>, Johannes Weiner , Andrew Morton , Zhongkun He , Chengming Zhou , Chris Li , Nhat Pham , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Kairui Song References: <20240324210447.956973-1-hannes@cmpxchg.org> <1e7ce417-b9dd-4d62-9f54-0adf1ccdae35@linux.dev> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Chengming Zhou In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT X-Stat-Signature: ru8ih3b948spxxkj586sctoyc3ayyur6 X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: AA496160013 X-Rspam-User: X-HE-Tag: 1711358565-606806 X-HE-Meta: U2FsdGVkX1+gIWgTZ/1QPVHCQY/7qP2zeMXsUAjSgNF6jBW4ZshomzbtrvGUD5SjovkS9bhYjl303d1sz7orLCGrhLVKHagMCinSzhthAVKaBcQYd1cv6DtcY/9jYU72s79f7cSoMFJumVpRZ1hVLioPwDTnqrnracMp9ZanVw8xgSTxszrgOa6kWEy3HxA+T6M4wYkqAs8G1HoFIayBMp5w5eRfhDrft+MwWLWv3xLPNIyEIJiErYBdJRREzY8WF69o0xl44eKxapIQuOLBxVrV43llvHuB7/45q6yqrxfzfe3fAbTwD3NeWd1osjQ2ne03tBgE0/5tp1NiExWIhv0g3CPCRWWLO13S631u4UZ+LtRqJ94bhAevOecfQa1fzek+byE865/f9Irq/T/gBJQ/J4+uLX4Nr9b4nrO6Gxen6ioOg8VexdBmxGekzjD2ZUrZDK0q1qpRKXuvN31Qd7oddiZ/mcS05cw2wlhIkR8smMdh0n4s04DcjlTT/09X5GSPZGA761l9odwbGgxC+K2XBd9BjoaZBMotbR/M0aNwtmWKkfuw0as1uoNGdKGAWTZ2IDY/LVghiPrz7UVaVwuNS5RhZq4rGF9RHnOLfpOVaZ4zwG/F+3i1yBl9Rm/Bz5/oKBFjfAyQRyVyI90JtB9YxxIhZOhX+Btn2XBWStHNdVCTHcyFz49+UOkFdukpgJZuEPMIRt1P+fUx29ZffPbfvpyocFcsAth2woBFKsItXksip2xLIg52oBGrwIuG6/vvPrcBU4si8vCfWKHJXP/+1uR1RfFGCjZJ9hrZHLGsvXN2OtbJh35guf+/HZxHUzFYe8gGmDbrTOaRuDPuTffBwhsRdTo1bJ3QShj6DU4BSeEmAGWW2g5YegK0YOJPmJPvI4yHhuG7TqSCYxmFY0Qbzc/pczh3PobEaAd/jjAEg/z0SqVwR//d5hRn/1SDyFrcLw359CeHmSaIQLA Ni39GRzo dOKgJRBbg+wC6kjxCm8o6NJKuA2C2crDkS/BPDFn+z3b9tkije1pHsf0ybKgAWKwZzsc90adu45Xg9TK919GE07s54KPeekCMYHYSuThGmZc3IyfFwFN0mjQaGz7eCxGM5aRdX0RQVPotktNFt69YxkjGc/c6IiCtcV0Hm9yJu7TC2XU2rwuCdR2O0S6rRPa3sgCMNAHxTe5p0P8FPLbg2Z+bPoLsOlKKMm5XSZRQVEmgiWcUFWga8OHu6OkRpEZGz6u8UZMF6BpgGOSdZQ3bz9KxVlpiRFO1SEGMElwTZW2FKPONHFvpLxBQ9VV9rRB5hF1FoQrSBxfMPMY5cSpikmjUuvp8dJpNbtp0MeAoEYF6NS0xXgFgrZx07gO9hkbPtLUCTx3LJ9Mbg+w= 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 2024/3/25 16:38, Yosry Ahmed wrote: > On Mon, Mar 25, 2024 at 12:33 AM Chengming Zhou > wrote: >> >> On 2024/3/25 15:06, Yosry Ahmed wrote: >>> On Sun, Mar 24, 2024 at 9:54 PM Barry Song <21cnbao@gmail.com> wrote: >>>> >>>> On Mon, Mar 25, 2024 at 10:23 AM Yosry Ahmed wrote: >>>>> >>>>> On Sun, Mar 24, 2024 at 2:04 PM Johannes Weiner wrote: >>>>>> >>>>>> Zhongkun He reports data corruption when combining zswap with zram. >>>>>> >>>>>> The issue is the exclusive loads we're doing in zswap. They assume >>>>>> that all reads are going into the swapcache, which can assume >>>>>> authoritative ownership of the data and so the zswap copy can go. >>>>>> >>>>>> However, zram files are marked SWP_SYNCHRONOUS_IO, and faults will try >>>>>> to bypass the swapcache. This results in an optimistic read of the >>>>>> swap data into a page that will be dismissed if the fault fails due to >>>>>> races. In this case, zswap mustn't drop its authoritative copy. >>>>>> >>>>>> Link: https://lore.kernel.org/all/CACSyD1N+dUvsu8=zV9P691B9bVq33erwOXNTmEaUbi9DrDeJzw@mail.gmail.com/ >>>>>> Reported-by: Zhongkun He >>>>>> Fixes: b9c91c43412f ("mm: zswap: support exclusive loads") >>>>>> Cc: stable@vger.kernel.org [6.5+] >>>>>> Signed-off-by: Johannes Weiner >>>>>> Tested-by: Zhongkun He >>>> >>>> Acked-by: Barry Song >>>> >>>>> >>>>> Do we also want to mention somewhere (commit log or comment) that >>>>> keeping the entry in the tree is fine because we are still protected >>>>> from concurrent loads/invalidations/writeback by swapcache_prepare() >>>>> setting SWAP_HAS_CACHE or so? >>>> >>>> It seems that Kairui's patch comprehensively addresses the issue at hand. >>>> Johannes's solution, on the other hand, appears to align zswap behavior >>>> more closely with that of a traditional swap device, only releasing an entry >>>> when the corresponding swap slot is freed, particularly in the sync-io case. >>> >>> It actually worked out quite well that Kairui's fix landed shortly >>> before this bug was reported, as this fix wouldn't have been possible >>> without it as far as I can tell. >>> >>>> >>>> Johannes' patch has inspired me to consider whether zRAM could achieve >>>> a comparable outcome by immediately releasing objects in swap cache >>>> scenarios. When I have the opportunity, I plan to experiment with zRAM. >>> >>> That would be interesting. I am curious if it would be as >>> straightforward in zram to just mark the folio as dirty in this case >>> like zswap does, given its implementation as a block device. >>> >> >> This makes me wonder who is responsible for marking folio dirty in this swapcache >> bypass case? Should we call folio_mark_dirty() after the swap_read_folio()? > > In shrink_folio_list(), we try to add anonymous folios to the > swapcache if they are not there before checking if they are dirty. > add_to_swap() calls folio_mark_dirty(), so this should take care of Right, thanks for your clarification, so should be no problem here. Although it was a fix just for MADV_FREE case. > it. There is an interesting comment there though. It says that PTE > should be dirty, so unmapping the folio should have already marked it > as dirty by the time we are adding it to the swapcache, except for the > MADV_FREE case. It seems to say the folio will be dirtied when unmap later, supposing the PTE is dirty. > > However, I think we actually unmap the folio after we add it to the > swapcache in shrink_folio_list(). Also, I don't immediately see why > the PTE would be dirty. In do_swap_page(), making the PTE dirty seems If all anon pages on LRU list are faulted by write, it should be true. We could just use the zero page if faulted by read, right? > to be conditional on the fault being a write fault, but I didn't look > thoroughly, maybe I missed it. It is also possible that the comment is > just outdated. Yeah, dirty is only marked on write fault. Thanks.