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 69875C47DD9 for ; Fri, 22 Mar 2024 23:48:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EE8416B0092; Fri, 22 Mar 2024 19:48:34 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E985F6B0093; Fri, 22 Mar 2024 19:48:34 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D607A6B0095; Fri, 22 Mar 2024 19:48:34 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id C5D476B0092 for ; Fri, 22 Mar 2024 19:48:34 -0400 (EDT) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 9D77FA079B for ; Fri, 22 Mar 2024 23:48:34 +0000 (UTC) X-FDA: 81926316948.09.5562B10 Received: from mail-qk1-f171.google.com (mail-qk1-f171.google.com [209.85.222.171]) by imf07.hostedemail.com (Postfix) with ESMTP id 7207240010 for ; Fri, 22 Mar 2024 23:48:32 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=cHeWHK7Q; spf=pass (imf07.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.222.171 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1711151312; 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=aQTSZTryQ/MFlPGn7Q0hFHJjxKUSl2BjvJPBr2Zsm+8=; b=EGjxkPprqUZCpZmglUD4xc2k/erpve8r4KUVISVfwI2kA603esrCYFCpM22WjLQx0IQWrU uiey8IL85d+8do1qbCQWwDuyX89LCmRHmDqmmHlvRy9Z/vITHzKgQy/Z0eGzRDVj6z/qpa CbP8WE2yBNny9lE3AHmllzofu5tyn+s= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1711151312; a=rsa-sha256; cv=none; b=cxMteAxAEupOFDbegfvpF1UHmiIqx+x3gLXBqqOUEqDnC8FeqQt6OCvxCnbVLR4dBfSAGO CTYFw0ouaaNDVhrNVh3nCHF852M4DuT85Bk8Q0q1ybpnxsWj/R/w1CJ8uD6HBjLOZhJI3X qMNi1tdaplobLf9df23X6emlglSza4g= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=cHeWHK7Q; spf=pass (imf07.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.222.171 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org Received: by mail-qk1-f171.google.com with SMTP id af79cd13be357-789db18e169so203588285a.1 for ; Fri, 22 Mar 2024 16:48:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20230601.gappssmtp.com; s=20230601; t=1711151311; x=1711756111; darn=kvack.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=aQTSZTryQ/MFlPGn7Q0hFHJjxKUSl2BjvJPBr2Zsm+8=; b=cHeWHK7QCCG/pAJ2NA1IlT+a+f7ZYOCqdxQyM/O0fCoPWg9S75wdH5qlJmDJvu/vvH r2rjgIpm6J9XGMFuVKhpUhE9j3PNP5L+LtAMX5XL9AB2vsTl6qK0APwGxc8GJDDUP8uu CaYvD28VokxgXUYLOn5+YXmRCdUY6xP/7+rYvgFHUDlQTpLV4593eV77N7XfqxS9AnRO 2LxQMY/1v6HlDL4hOVeY9KL53WjcXcTCc4jJUj4EpV+ixNXe1fe/+aCOdBtQfBSWascE 3azfjWVUBoQIGuPjn0sTJxQbxPoQ85UrInypEbSkCnuO/ozrU6dz5KFc9H8QlSTtSdo5 0++w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711151311; x=1711756111; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=aQTSZTryQ/MFlPGn7Q0hFHJjxKUSl2BjvJPBr2Zsm+8=; b=T1LlemuOlV5wfn5qb4oDBW+IpyWCknCVzAFFCwY9GecoLNU2KGZyzokvx4eZcPoidX tAkhykI7OTF33sgYza9znD4kbkWaNq4bMNYZNnlUpxmT+x/otb2LPEQhMPmlGq67yP7x JstkJZ7Ujvy91z9WpL+0z9NsdGnKLMwvpaWdsfEfLeXiRmsZOLgz/NH9J3cFA+3fOskb XzbTrbLYzY0jQT/veG8Bg1YPOwJe22iyZwiR7Jg4dBccRfJIH63dmSBYDZxuk3Y2buTU 2F04H5YpkfIVG0/Ma5itQ7s5fhqGV0JD2aRaJsuXsbhkj5KcT9faEhwi7ofySmUpgy4A +gBQ== X-Forwarded-Encrypted: i=1; AJvYcCXC/8NFzEa7fnt2rKRgJL4kadL3uWO/77C977/y1qQUm4HCIzOqaAiqixxQ1nqgfZygZFPBdV19uQ/v3PsG+jFNA9Q= X-Gm-Message-State: AOJu0YywXKMpS53E3lnJlj46ypnBn7nf3SEBhL0zulSYEYZUilbDpT/n KdYpm8mDmDSGakyHFt5syL6VIm7m7YqYmTL/w9XD1B6syQFOkX9H5TGr7/9++UA= X-Google-Smtp-Source: AGHT+IEOFk5l+pdXqCde0pKSmp+mbC8Z5Cvz7pS7D37/Pv3CRjhdK62gKP0NzpZao7K84yhG3hJXeA== X-Received: by 2002:a05:620a:40d0:b0:78a:4598:2ae7 with SMTP id g16-20020a05620a40d000b0078a45982ae7mr674879qko.16.1711151311436; Fri, 22 Mar 2024 16:48:31 -0700 (PDT) Received: from localhost ([2620:10d:c091:400::5:16be]) by smtp.gmail.com with ESMTPSA id n17-20020a05620a223100b0078a384a4854sm274125qkh.7.2024.03.22.16.48.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 22 Mar 2024 16:48:31 -0700 (PDT) Date: Fri, 22 Mar 2024 19:48:26 -0400 From: Johannes Weiner To: Yosry Ahmed Cc: Barry Song <21cnbao@gmail.com>, chengming.zhou@linux.dev, nphamcs@gmail.com, akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Zhongkun He Subject: Re: [RFC PATCH] mm: add folio in swapcache if swapin from zswap Message-ID: <20240322234826.GA448621@cmpxchg.org> References: <20240322163939.17846-1-chengming.zhou@linux.dev> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Queue-Id: 7207240010 X-Rspam-User: X-Rspamd-Server: rspam11 X-Stat-Signature: oknfkqjjys39tbh1wyf74e63ar5m1bog X-HE-Tag: 1711151312-794083 X-HE-Meta: U2FsdGVkX18cPgWXtJt2PzyzOaVRIcjol2DaPCh+f3syfeGyvSK4SBmUzOYQ3WjjXP4i4KRYMjZa+mGChgj8P9nvAN2XCf8aqA6bWi7sZrY/oMpwZK5FgQTpiOI6q8gMsi5gn9gTm+y/RAC6cYKqmSPUzH3G5CcLJ1BwoPLS9d8UHSN7nUHYrp8vH8Bf5NfPPI/29WSr4HjkHUgRawdDLO2db8Ckf3GBaVJvPrcFv7dxPd9MRgWRSP00+m6TQJKwVvii1L8o5BwH+KLPJpaI+I4M0oJpLQr3L+OCqiwYM0eT4tz64/rNs4dT6PQWNq+tEj/H4i6qBBgesk9/KOdHYP2zH+q5vVE9O930PtHjqLqw6A2M2DG4NAZlpRhdQrGK4TTcgyml1ThOcL65g/UTjIxwFSPVvrTB5HPuFNiijUdILlu/iDjSNVd9hT7bX/xUGSIiS37qTX+dYosty0//YKg2RZr3+N6/cZn1LCdbyTR7w9z5V+gkDWNxpPSqrJTnPIWzEeOANzAgG7f07VUUBYBKy+SEMv001MQ/WA2u9mscW+34zo26DKN9ECqVhZjCRuJSfEik/qBmFk2W0l5TbQhPQJCTkS83Q2tAH+BaNJJoucdhOuZVvoqKQhOZgFTNoA0pXTsNBqOiZzxohrzmkUAK1wm5esQhPvq+Ce1hDrd2OsqpTp8wRh/ZPA3fx4xnT7IPuH+FucoYlmbClxLW3de1UcyzC3JsDl1BQHrXaczyf0r78ibznnAGSCPKhu+Cv6WdUEr6kd59O2Rk7ONx/rdpHch7PKEnwf/KNLEn2tsoUsn/Wp0iazcRkMgLW2BZcHdlyumZAHf0lP6Ny8kUncFstjTGIGEsMV6SnXu9n6LgUVVgDJU70y+kBoqrLQbgvZ8p0WEd00UR/NwF2iGJNUrd+BI43ZW49spM7LJuu+2FVgokGwZZMOEVHaDEUpsVTsYeQCn88Nt6bh4DZYB TW0OvRpZ AY52XzGKwfkAk2x/q/AKYkGqAxD0Z9XK0IU41qihx6Pht8swhbiYIrXyw2sB40JaygqqxQYquoVcIv4OI3agiWiF1vI+twgKisMiBnxBY8K3t4/pdf7FGMHV4yAV6dzgPhE8v6uS/qSvkriv62qjkpwdxDuCVL1aLhe3PDE6OHeT/Dd7cN8QpnOfzOSFEfKUkPEjGE2n+LVK8OvJDfuFfx8X+qQKBM+uhnTptI+MFsuEY/M9ZeeqNyzC2T3McwT47PFt4Xj1+vfqkTRQLR9A79nYw9Ga0vWP0WRXetHUSHADd/rI9Eti/DnVEbZo9725sfJZyHnsUifgSsqQKV6X9XxgGaXajII9Eoy525hwl6Ny2lg0kARKMl+Ofic3pxSeoZ2s7nAcn5arf8LR7W4FEPqRtlBqNL4AVm8Apc8TlTjRKveljNweymwdWs5rp0gzeVEPyxsZ0NaHmz37l6H3P0XWCkqQYrmHWI4ch+JmUShz6jPHGSkBav5sn5jB/h4zrqE/ad8kRF0961++ggE96Ifu1aM01caM8ARhs 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 Fri, Mar 22, 2024 at 10:33:13PM +0000, Yosry Ahmed wrote: > On Sat, Mar 23, 2024 at 10:41:32AM +1300, Barry Song wrote: > > On Sat, Mar 23, 2024 at 8:38 AM Yosry Ahmed wrote: > > > > > > On Fri, Mar 22, 2024 at 9:40 AM wrote: > > > > > > > > From: Chengming Zhou > > > > > > > > There is a report of data corruption caused by double swapin, which is > > > > only possible in the skip swapcache path on SWP_SYNCHRONOUS_IO backends. > > > > > > > > The root cause is that zswap is not like other "normal" swap backends, > > > > it won't keep the copy of data after the first time of swapin. So if > > > > I don't quite understand this, so once we load a page from zswap, zswap > > will free it even though do_swap_page might not set it to PTE? > > > > shouldn't zswap free the memory after notify_free just like zram? > > It's an optimization that zswap has, exclusive loads. After a page is > swapped in it can stick around in the swapcache for a while. In this > case, there would be two copies in memory with zram (compressed and > uncompressed). Zswap implements exclusive loads to drop the compressed > copy. The folio is marked as dirty so that any attempts to reclaim it > cause a new write (compression) to zswap. It is also for a lot of > cleanups and straightforward entry lifetime tracking in zswap. > > It is mostly fine, the problem here happens because we skip the > swapcache during swapin, so there is a possibility that we load the > folio from zswap then just drop it without stashing it anywhere. > > > > > > > the folio in the first time of swapin can't be installed in the pagetable > > > > successfully and we just free it directly. Then in the second time of > > > > swapin, we can't find anything in zswap and read wrong data from swapfile, > > > > so this data corruption problem happened. > > > > > > > > We can fix it by always adding the folio into swapcache if we know the > > > > pinned swap entry can be found in zswap, so it won't get freed even though > > > > it can't be installed successfully in the first time of swapin. > > > > > > A concurrent faulting thread could have already checked the swapcache > > > before we add the folio to it, right? In this case, that thread will > > > go ahead and call swap_read_folio() anyway. > > > > > > Also, I suspect the zswap lookup might hurt performance. Would it be > > > better to add the folio back to zswap upon failure? This should be > > > detectable by checking if the folio is dirty as I mentioned in the bug > > > report thread. > > > > I don't like the idea either as sync-io is the fast path for zram etc. > > or, can we use > > the way of zram to free compressed data? > > I don't think we want to stop doing exclusive loads in zswap due to this > interaction with zram, which shouldn't be common. > > I think we can solve this by just writing the folio back to zswap upon > failure as I mentioned. Instead of storing again, can we avoid invalidating the entry in the first place if the load is not "exclusive"? The reason for exclusive loads is that the ownership is transferred to the swapcache, so there is no point in keeping our copy. With an optimistic read that doesn't transfer ownership, this doesn't apply. And we can easily tell inside zswap_load() if we're dealing with a swapcache read or not by testing the folio. The synchronous read already has to pin the swp_entry_t to be safe, using swapcache_prepare(). That blocks __read_swap_cache_async() which means no other (exclusive) loads and no invalidates can occur. The zswap entry is freed during the regular swap_free() path, which the sync fault calls on success. Otherwise we keep it. diff --git a/mm/zswap.c b/mm/zswap.c index 535c907345e0..686364a6dd86 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -1622,6 +1622,7 @@ bool zswap_load(struct folio *folio) swp_entry_t swp = folio->swap; pgoff_t offset = swp_offset(swp); struct page *page = &folio->page; + bool swapcache = folio_test_swapcache(folio); struct zswap_tree *tree = swap_zswap_tree(swp); struct zswap_entry *entry; u8 *dst; @@ -1634,7 +1635,8 @@ bool zswap_load(struct folio *folio) spin_unlock(&tree->lock); return false; } - zswap_rb_erase(&tree->rbroot, entry); + if (swapcache) + zswap_rb_erase(&tree->rbroot, entry); spin_unlock(&tree->lock); if (entry->length) @@ -1649,9 +1651,10 @@ bool zswap_load(struct folio *folio) if (entry->objcg) count_objcg_event(entry->objcg, ZSWPIN); - zswap_entry_free(entry); - - folio_mark_dirty(folio); + if (swapcache) { + zswap_entry_free(entry); + folio_mark_dirty(folio); + } return true; }