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 BF4A4C021BE for ; Thu, 27 Feb 2025 06:16:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 52C6A6B0089; Thu, 27 Feb 2025 01:16:22 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 4DCF76B008A; Thu, 27 Feb 2025 01:16:22 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3A5AA6B008C; Thu, 27 Feb 2025 01:16:22 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 1C2F46B0089 for ; Thu, 27 Feb 2025 01:16:22 -0500 (EST) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 94686B1AB7 for ; Thu, 27 Feb 2025 06:16:21 +0000 (UTC) X-FDA: 83164714962.24.2F15E8E Received: from mail-qk1-f173.google.com (mail-qk1-f173.google.com [209.85.222.173]) by imf06.hostedemail.com (Postfix) with ESMTP id 467B7180002 for ; Thu, 27 Feb 2025 06:16:19 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=rlhBUmYl; spf=pass (imf06.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.222.173 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=1740636979; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=hIRNfZw7sAL9nU9Dv2qbYVT7pWVShOhPHJeE1WYpjeI=; b=eJa66YmAoivG2yF/H7NswKBDYmUIUyHhDUAjpnE3Cn+8jmPKHCvVxMz7Gy2yFttzwLygga eXI2Q0NKkqkJ4D9+6x928RhTQIuk/UXbBl+WO/Y6w75RZn6zCMOyluxLVCXymC4ptCax/E 88jP350eHqh6uhsnmx9FsWhq1ySlAO8= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1740636979; a=rsa-sha256; cv=none; b=5dpk95FysWfwNNDIcDM+cId4KKKQ9C+HkmL4QYTwlAO1R4czDzaGN7MD9DDcB6NSMuM2hW 9LddXrpg3+OEsO6CJ3HiVzPcEKL/6iv846RQlogQZ/nyv91hfToyRrqFFJQwxNLWpiiEBS V7GuvZ3U3GJIlJUCo3K4kfZMxr1ObY8= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=rlhBUmYl; spf=pass (imf06.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.222.173 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org Received: by mail-qk1-f173.google.com with SMTP id af79cd13be357-7c0a3d6a6e4so53631685a.1 for ; Wed, 26 Feb 2025 22:16:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20230601.gappssmtp.com; s=20230601; t=1740636978; x=1741241778; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=hIRNfZw7sAL9nU9Dv2qbYVT7pWVShOhPHJeE1WYpjeI=; b=rlhBUmYlJOVBRbj8iKBYRgzKAuIvg+uhh6xM41Ki5jWDFeV/hETHilSoTFWUGfL7hI 9pzchCKwFdXW72n7+cH1shRxtFbWhnsP13vLXKpPBvmWnrTzvymXN7a1BVHZuA+TnVWi tDPnx8ODFQ4j35SYUUUSaAmzSjnIEmf0+IhYNug2eV5vKUNKFMDsy4TQ/hz8lzlxVmG7 4Kw8j64iNf4R/s1ODiUtJWGTHyp4nZpU5Mq+6MxcAkU/jD3WOhlmzMwzXWMuk279bFNS wDIRAgXPzW41Jym0yBYmAUo2l/tQKfBBmhwrzJfzs+2wy3lb0ZDoAglUcvDnudE45RTQ /CyA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740636978; x=1741241778; h=in-reply-to: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=hIRNfZw7sAL9nU9Dv2qbYVT7pWVShOhPHJeE1WYpjeI=; b=kcbj2o4xBe+Aln719op+Q8gHhUeEySkSgefH37GwetKiSaNKfi1F7jcP13C5fiaO1a OuyOya+VQ203dgRMZZjAK7ndOBcXFpRQbO9piuFjZFMGwZTP7tDJUjjAd49BRJhjI6j9 EpDOePb6L+Nhif3z7mUfLPjJp7VW1xjZGrIQfOq5hyhMJ+69dw3RrnptC2zazXuJRDTD I2dTc800deUevP0eW55vKWokGPeg7JPugu8O569SPYPMVuFXTBb0+feOzvX6RZQN2YCE g1XXb1Nk788WHINsf8jvLxtfJqi8osM0Ez3qB5WqFBKNJYVQ9wHj0ypU+t3O+qHGAxEZ NRAQ== X-Forwarded-Encrypted: i=1; AJvYcCXhgaHPk4HHVQV2XAMmnS4dYm5IprDIBO2wLiM/zZE28TciMhQxzamxxBYle22cm6PRVOd+j6yNuQ==@kvack.org X-Gm-Message-State: AOJu0YzqEjDnaQtdZYdNaGYMYz5n4Ak7FztuuPTtWpFi5r6Z3Bj9A3Wx FozjtynnOCYJj8b5pfVCEtEUAMnDjeTVRZ/mEDkiln4UbiDkLlXx48R1UFznpAw= X-Gm-Gg: ASbGncvRAC6mmdgdmb9ezbm1K9jnivfcf4FGVSCeciuiG9N9TRsltUSwxpL2NrixXkL 3JiRXR0fKdYY7Tz5wF/3xUE6cp3OYbXeczxxNTiGS8Dhf5dfUtsLGQN440PmTMZNyNAgdqVknf/ VZj2sQ0Jhy9vJI4isUuOQ5DkhU5VTUjsYuSMdHEwmdYLj08u0BN8GC3HwItpdSQTzwwE6lZkR5e eDENB4oxoJLsxC7bkjwMadab3quxgWsDxuqGj6EzernjFF9EOegjyqZNsA8xIbKBZi6jYYe/pEE ZL551u1QPnOgDI7fWFfpk2IA X-Google-Smtp-Source: AGHT+IGpheiFaPQV+vHiSGfdo4uJer00Fm7VGOok0pckB2A3V+XmwGYe7G6QUkbygmdGPwoLEH9X6w== X-Received: by 2002:a05:620a:6002:b0:7c0:b0a8:52e6 with SMTP id af79cd13be357-7c0cef4953emr3752564485a.46.1740636978196; Wed, 26 Feb 2025 22:16:18 -0800 (PST) Received: from localhost ([2603:7000:c01:2716:da5e:d3ff:fee7:26e7]) by smtp.gmail.com with UTF8SMTPSA id af79cd13be357-7c36fee9844sm72875785a.19.2025.02.26.22.16.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Feb 2025 22:16:17 -0800 (PST) Date: Thu, 27 Feb 2025 01:16:16 -0500 From: Johannes Weiner To: Yosry Ahmed Cc: Nhat Pham , akpm@linux-foundation.org, chengming.zhou@linux.dev, linux-mm@kvack.org, kernel-team@meta.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] zswap: do not crash the kernel on decompression failure Message-ID: <20250227061616.GD110982@cmpxchg.org> References: <20250227001445.1099203-1-nphamcs@gmail.com> <20250227043141.GB110982@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Queue-Id: 467B7180002 X-Rspamd-Server: rspam07 X-Stat-Signature: 5irhcacrb8esxahbcesuc9gxozt5k5wi X-HE-Tag: 1740636979-601234 X-HE-Meta: U2FsdGVkX1+a/X+2VO8XHMNA77n3MpkQ0SHPr3RgHRQ88BcWeLah027UpFc26ECu+yd8nM7C7VZxHQZQhrLdal1jR5ACBSNUxgoYRgDMRkv3oPWMJWXEdgSsSocaP441UrtqzT31YGXWde4i/k46TiGtT1tin+ezwx94cx0G9OhZy8hgrjGoH9x3Xb23iwSlS6jBvpUIMv6jFBz58Cb8DihlbCKYC7rkhRBNUjX5n/YmsaOLCmpYyP+QIAXCekzy7rFX7lLNFQN1c+UW9gRULfgTmREZyqxvAaQ7I2mCNtiYOPqC8JOfHOkvZpaoR0gWFoJw0kOp/kJKF3lDM96y39Ztiog++I+xNq5/IRFVcFrUTCCnowBBy2ZVCpwqJ4Y8sbnB2qMrCyHJAcs8zUbMe/s9Pd/XI9ytxfxWMz/Ui+SoyXnQSUXBfoFFUEN5cT0rI+bPDRgFZqKsT0qiUl3W6oSOC4u7lHjdc+6LkQnzYWgSU9gBv+RR2e5aAaRSd8dzZznu7Ms0GRi4qZ9uu8rkjsBbNR7EjWPfhSAlBOJ/vRW+KQca5UxJQ3E7nALbk7F3mRX5nbKnHlLbvZbf9UQSeSYVoEjnMsI8Ndcxobl5tUQDUj17B4cG3NgaC5q8+AWT0ORBodufpoJh/Sz2L0i7rbKApDx5cdRcrupSYTxao3RFJwn/HEmnITgnQgEyNzx5Qmwazy79zF+Gd78WwLLz7VmS/kpyaByEaOATd5Cgunx9OA/KVkkGyUDa/S3BGRaIr9YSUjmgfphj6WKj3jBqNFfNZo5vYv3cMe0IGWMTcvNtdZw7eN3yYDaFAYWp05H8iTr44fzUc8BJAdA4T4ieWGZBP8vSYeZPurE/aLx6y6T8zJ5Pp6kwyN/0RbHk8cmG6dR9sm4SS83Am/lKCr5QPeoXZ2MBynhPzGenhEs5UEhzP17zMkTUfWhYWM4QdxpRcktWJa/tL9BDl/3reqa skT/w9l4 9Hi6q4e9MRdJoxlkQ61e8pQI7hDMIQ2ELQRXUbodPyyczzu6uDhjEFpLuMuR1iy2ZOY1vlrEaDEVV+C0/4/DOFKWlOWZolniy4oTAv/hBSJaj6eXCjEL6oNjddDv3SquqzYs8Ms8OX2k2dWAjfyuvrm6cp9mHFTnC+UiYED/p4Y04zMtMRFPambpv077xpMdIEjVB5ZZsalFnxAhSfQ4w+UJuHP8Vyau7zDJJCgZ51FSX4k7nbxA3ii/Us0FqFLzIUNOwI5tcJATGV6kHf1OkoNb2rfIxOigQrJGsyjFZx6Cn45uOvPKoN5hrFKEXCFOJV/Qh86iKhe6K8y4rOlgLJgRstwVX6GGJERzXGQc7se6v6PkbQW30d8z0LMrWl4V78TkW2DbajkeLz0heY6BQ60tytot+qbUXvs6vvXIQfLG600ZtfDUSJhACL99DhewfMQcHiHHmPSuS6e9sKi51smNLsA== 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 Thu, Feb 27, 2025 at 05:44:29AM +0000, Yosry Ahmed wrote: > On Wed, Feb 26, 2025 at 11:31:41PM -0500, Johannes Weiner wrote: > > On Thu, Feb 27, 2025 at 01:19:31AM +0000, Yosry Ahmed wrote: > > > On Wed, Feb 26, 2025 at 04:14:45PM -0800, Nhat Pham wrote: > > > > if (WARN_ON_ONCE(folio_test_large(folio))) > > > > return true; > > > > > > > > + entry = xa_load(tree, offset); > > > > + if (!entry) > > > > + return false; > > > > + > > > > > > A small comment here pointing out that we are deliberatly not setting > > > uptodate because of the failure may make things more obvious, or do you > > > think that's not needed? > > > > > > > + if (!zswap_decompress(entry, folio)) > > > > + return true; > > > > How about an actual -ev and have this in swap_read_folio(): > > Good idea, I was going to suggest an enum but this is simpler. > > > > > ret = zswap_load(folio); > > if (ret != -ENOENT) { > > folio_unlock(folio); > > goto finish; > > } > > > > read from swapfile... > > > > Then in zswap_load(), move uptodate further up like this (I had > > previously suggested this): > > > > if (!zswap_decompress(entry, folio)) > > return -EIO; > > > > folio_mark_uptodate(folio); > > > > and I think it would be clear, even without or just minimal comments. > > Another possibility is moving folio_mark_uptodate() back to > swap_read_folio(), which should make things even clearer imo as the > success/failure logic is all in one place: That works. bdev, swapfile and zeromap set the flag in that file. > ret = zswap_load(folio); > if (ret != -ENOENT) { > folio_unlock(folio); > /* Comment about not marking uptodate */ > if (!ret) > folio_mark_uptodate(); > goto finish; > } Personally, I like this one ^. The comment isn't needed IMO, as now zswap really isn't doing anything special compared to the others. > or we can make it crystal clear we have 3 distinct cases: > > ret = zswap_load(folio); > if (!ret) { > folio_unlock(folio); > folio_mark_uptodate(); > goto finish; > } else if (ret != -ENOENT) { > /* Comment about not marking uptodate */ > folio_unlock(folio); > goto finish; > } This seems unnecessarily repetetive.