From patchwork Sun Feb 4 03:05:59 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chengming Zhou X-Patchwork-Id: 13544431 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 3DF48C4828D for ; Sun, 4 Feb 2024 03:06:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B51E86B0075; Sat, 3 Feb 2024 22:06:21 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id AB32F6B0078; Sat, 3 Feb 2024 22:06:21 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 953216B007B; Sat, 3 Feb 2024 22:06:21 -0500 (EST) 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 880226B0075 for ; Sat, 3 Feb 2024 22:06:21 -0500 (EST) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 41CD4C011F for ; Sun, 4 Feb 2024 03:06:21 +0000 (UTC) X-FDA: 81752632962.02.B53EDAA Received: from out-188.mta1.migadu.com (out-188.mta1.migadu.com [95.215.58.188]) by imf04.hostedemail.com (Postfix) with ESMTP id 7C4CE40008 for ; Sun, 4 Feb 2024 03:06:18 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=none; spf=pass (imf04.hostedemail.com: domain of chengming.zhou@linux.dev designates 95.215.58.188 as permitted sender) smtp.mailfrom=chengming.zhou@linux.dev; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=bytedance.com (policy=quarantine) ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1707015978; 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=Csm8/8Af2EeUKCuq16yFfXjClpCr+YQkSwdEsQEV9IY=; b=2AJunHtpdg3AgMHOab3N1zYDyZ2IOrqguMJc5sxrjmF0i/9oQ3uO6Ur2eyCqf1yXqtgybH ZhDBAKCW6AK4k0q1irm9y07xT2nozm//Qtezw6HgzmWtjtwRsAFhQ6eD3i429GuHP+Yh1A UxAN+2EtSXo6ob8V3ef5f4imnQPZzwo= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=none; spf=pass (imf04.hostedemail.com: domain of chengming.zhou@linux.dev designates 95.215.58.188 as permitted sender) smtp.mailfrom=chengming.zhou@linux.dev; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=bytedance.com (policy=quarantine) ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1707015978; a=rsa-sha256; cv=none; b=YAIF8F5tUl3B35UcDDHVLpR9qQn8kiH9ml+Ha1iwRpgRHXh2+ccwawiLjKPRHJGiACxPs9 W73lwPGnZ3wPGaRIC7nkpODTAAuUvNT5BzoYQSTMtssbPOYB3z2VOdZ0Mvy/4usSWk/M3D AS2Sir0KdjpLGySkgnTfGzyHmHgcKUE= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Chengming Zhou Date: Sun, 04 Feb 2024 03:05:59 +0000 Subject: [PATCH v2 1/6] mm/zswap: add more comments in shrink_memcg_cb() MIME-Version: 1.0 Message-Id: <20240201-b4-zswap-invalidate-entry-v2-1-99d4084260a0@bytedance.com> References: <20240201-b4-zswap-invalidate-entry-v2-0-99d4084260a0@bytedance.com> In-Reply-To: <20240201-b4-zswap-invalidate-entry-v2-0-99d4084260a0@bytedance.com> To: Nhat Pham , Yosry Ahmed , Andrew Morton , Johannes Weiner Cc: linux-mm@kvack.org, Nhat Pham , Chengming Zhou , linux-kernel@vger.kernel.org, Yosry Ahmed , Johannes Weiner X-Migadu-Flow: FLOW_OUT X-Rspamd-Queue-Id: 7C4CE40008 X-Rspamd-Pre-Result: action=add header; module=dmarc; Action set by DMARC X-Rspam-User: X-Stat-Signature: 9toxb88n4bpbmc7koksr3t61t3iahds8 X-Rspamd-Server: rspam01 X-Rspam: Yes X-HE-Tag: 1707015978-421325 X-HE-Meta: U2FsdGVkX18TOZ9nQ9LY+roOPvgClu87BjrPVZACEnkR5CMpQR5x0GFv73H7oVywAQFDUaLhfqTAP9V+/XHTD+GFb6xqsGnCPlEOvMpIhYnXSL22/BDthLTFHwF3J9uKCA+21kVyvPrV/f2GhY2f43RYX+V/qW0O8WvM6R0Z8J4pqlIvJYZd3HHBoNhTCMmIaAoMSmLopM+v7BjEbPFyTq9e8WVopr73Efx1PU2hynjOuRP4tNkZyM68AwSanauaZTzmPKMdAZ78pWNBo1eeTlSbB1OFY4Kwt/R7OQ2L2xjYVRBbQ45ANjeVsF/dz9I6auNF1R93eG0QnDVJ894L57cOW8wPkss74ixRgEF2zDqWojpx9Ighctuj9YsQXkPtidR4q2g2x6NsvhXGwNYMBIaDU+RvuW474K26gJtNcS8EZWkeHD9nSHh/p+dMcjnHeENQ2aAlxJUjyMvw81HJzsjfhG3Z0Bzpao5UMG14NF0ND5u8STuhqtH/8B7hXfnYqhuQmm7eG8hB55IEXz1FIq7bdPb34ulP6x+4QPoXniV7JunYzmw4cGrqBhImCobB5o41lAC/pGJPjsmVrHLhx5yaLxCxc8WsA9AxOPDiWRiduXBrtjswxy1DMInPTB0V72NYIde5Stsi9h094mEq+E9/2Qp2WJmDfLgvab5HlTHHLgLZbgPxiSnsWf+hCPvSsDMiqGVmKGviAA/D/ZyBKJnTQh7uVoFgBbqjyOAsbGnLHbVwT/3v+b6mAVTv496SA8XxjZlcAVd8Kr3xAaNFHh2W6bDhwxiErXNT+cdUt93bj9Kj7X154ocqSQLkNz5SPB9TjiVOwY10wt2EMM2yX0DEuKMVVpg43pRke5WJEbtmT1ghSEpmlSebDNa7ddhNwF1eHU7DUUixIsXjpjJhVKRACDyrIhuOHXtx7hfOwtjRUGj0r6zDEL0yMeP/aR20LmUWvY7nyNinW5mcxWA 87N7Z9ll hlquOoMJc5dfptUAhJbP5OnqzWI3Tqt1CeSuFXjdczA6Eknu2ORrl3Iod7ht7TlNge1rcRR/HjO4NZ6tEW9ps8xJqqTalYIX+JuLEmTfH3toE/m3Yhr2vGpBLWNX0Rak4RcSyBLKXCP5pQFmMNq4WQOpYLCx3ROqLgOHNw9pZiWIonG0Kqr69ay0tuO9NQitBITqCAjKSdTjPRQwZVnhae8hXTy09kA9JLooCREekZQyLjc6/8jf6FRkwdn6wpgsnRD2BwITUxNij5yXIqS20++J9iXiV25fBpltCFO2bLQZnHfFwXVjfCaA/iqF9e54J/7IIjlsJTd+Rnk2XPV1U2MU37cByt6A9yAXiOkCQCIN2wRf6Ij8zPkoex6OLbfP/uNa0Mi5IEQ9OU4cFVpPwgAQ8o+jmmlee59XQIswfCB65dcpeem1CZkhEkq5WXpGjvwfSNHHdhjMIbbDBgiQleEqYRYlBZi/pAcppBX1sC5JpcAynr/b9OYyHRFbBxGBGOakK0q8ANHWINDE= 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: Add more comments in shrink_memcg_cb() to describe the deref dance which is implemented to fix race problem between lru writeback and swapoff, and the reason why we rotate the entry at the beginning. Also fix the stale comments in zswap_writeback_entry(), and add more comments to state that we only deref the tree after we get the swapcache reference. Suggested-by: Yosry Ahmed Suggested-by: Johannes Weiner Acked-by: Yosry Ahmed Reviewed-by: Nhat Pham Signed-off-by: Johannes Weiner Signed-off-by: Chengming Zhou --- mm/zswap.c | 43 ++++++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/mm/zswap.c b/mm/zswap.c index 4aea03285532..735f1a6ef336 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -1207,10 +1207,12 @@ static int zswap_writeback_entry(struct zswap_entry *entry, /* * folio is locked, and the swapcache is now secured against - * concurrent swapping to and from the slot. Verify that the - * swap entry hasn't been invalidated and recycled behind our - * backs (our zswap_entry reference doesn't prevent that), to - * avoid overwriting a new swap folio with old compressed data. + * concurrent swapping to and from the slot, and concurrent + * swapoff so we can safely dereference the zswap tree here. + * Verify that the swap entry hasn't been invalidated and recycled + * behind our backs, to avoid overwriting a new swap folio with + * old compressed data. Only when this is successful can the entry + * be dereferenced. */ tree = swap_zswap_tree(swpentry); spin_lock(&tree->lock); @@ -1263,22 +1265,29 @@ static enum lru_status shrink_memcg_cb(struct list_head *item, struct list_lru_o int writeback_result; /* - * Rotate the entry to the tail before unlocking the LRU, - * so that in case of an invalidation race concurrent - * reclaimers don't waste their time on it. + * As soon as we drop the LRU lock, the entry can be freed by + * a concurrent invalidation. This means the following: * - * If writeback succeeds, or failure is due to the entry - * being invalidated by the swap subsystem, the invalidation - * will unlink and free it. + * 1. We extract the swp_entry_t to the stack, allowing + * zswap_writeback_entry() to pin the swap entry and + * then validate the zwap entry against that swap entry's + * tree using pointer value comparison. Only when that + * is successful can the entry be dereferenced. * - * Temporary failures, where the same entry should be tried - * again immediately, almost never happen for this shrinker. - * We don't do any trylocking; -ENOMEM comes closest, - * but that's extremely rare and doesn't happen spuriously - * either. Don't bother distinguishing this case. + * 2. Usually, objects are taken off the LRU for reclaim. In + * this case this isn't possible, because if reclaim fails + * for whatever reason, we have no means of knowing if the + * entry is alive to put it back on the LRU. * - * But since they do exist in theory, the entry cannot just - * be unlinked, or we could leak it. Hence, rotate. + * So rotate it before dropping the lock. If the entry is + * written back or invalidated, the free path will unlink + * it. For failures, rotation is the right thing as well. + * + * Temporary failures, where the same entry should be tried + * again immediately, almost never happen for this shrinker. + * We don't do any trylocking; -ENOMEM comes closest, + * but that's extremely rare and doesn't happen spuriously + * either. Don't bother distinguishing this case. */ list_move_tail(item, &l->list);