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 52DFAC282C6 for ; Tue, 4 Mar 2025 00:05:44 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 97FED6B007B; Mon, 3 Mar 2025 19:05:43 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 9312D6B0082; Mon, 3 Mar 2025 19:05:43 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7F8A56B0085; Mon, 3 Mar 2025 19:05:43 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 5F6296B007B for ; Mon, 3 Mar 2025 19:05:43 -0500 (EST) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id D5B7B1410CD for ; Tue, 4 Mar 2025 00:05:42 +0000 (UTC) X-FDA: 83181924924.10.D01E18A Received: from out-176.mta0.migadu.com (out-176.mta0.migadu.com [91.218.175.176]) by imf11.hostedemail.com (Postfix) with ESMTP id F40B240003 for ; Tue, 4 Mar 2025 00:05:29 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=jBCZYoTb; spf=pass (imf11.hostedemail.com: domain of yosry.ahmed@linux.dev designates 91.218.175.176 as permitted sender) smtp.mailfrom=yosry.ahmed@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=1741046730; 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=ukHXgY57eZmQzhJsN++ZQcOGiRMcIqTi4ELxs+Q3I8w=; b=p6+LNjBD4JTI0hNcJIOINgaYoX87puGCZKrQK/IzGC0j/8D0qH3wQiL4G/mykUWX+8K75S OMou0H9LUOG76hnB6A1PlDtF8cfThR33XG9Q1OwsmBG+7MqShDcLO5JOX4ZZbzursOylJJ g5x+tJnlShWpjHXKZ55Cto71j7Y7SU4= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=jBCZYoTb; spf=pass (imf11.hostedemail.com: domain of yosry.ahmed@linux.dev designates 91.218.175.176 as permitted sender) smtp.mailfrom=yosry.ahmed@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1741046730; a=rsa-sha256; cv=none; b=3ouub6Lp3ZXu12D9qwNc8212YCB7L0OJSh0z4qHmu724ov5dugNvjn+1ABlb88ZR3ksOBk IVb8uPUL17RSN2ChoEBcF6KWmq4GcomtgEMg7KL3NFzsmQCHfuOagQhqCO/xUjzKUZUCpq lN0PF6lK91ezeOxbqtEx5niP3kz3fOk= Date: Tue, 4 Mar 2025 00:05:21 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1741046726; 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: in-reply-to:in-reply-to:references:references; bh=ukHXgY57eZmQzhJsN++ZQcOGiRMcIqTi4ELxs+Q3I8w=; b=jBCZYoTbetyrsAf2FZMSKN+yN1N4DDhHG0fIxfV2nw3HBqnXm24w/8Tk4vf85HBJJQlhmx hrVOXCeXhxKNfcMLbOI4kEelUvSWzSZ7/u7mJRPbpE1fu4ZEVtiq/KxN0dErbe0q2KWw4E YhDJIZwZiqaOvdL2bwUdqz8nAbs+Lh0= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Yosry Ahmed To: Johannes Weiner 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 v3] page_io: zswap: do not crash the kernel on decompression failure Message-ID: References: <20250303200627.2102890-1-nphamcs@gmail.com> <20250303215524.GD120597@cmpxchg.org> <20250303231654.GE120597@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250303231654.GE120597@cmpxchg.org> X-Migadu-Flow: FLOW_OUT X-Rspam-User: X-Stat-Signature: 5w6goxi5tyauxkc4cick4um857hc4azb X-Rspamd-Queue-Id: F40B240003 X-Rspamd-Server: rspam07 X-HE-Tag: 1741046729-294357 X-HE-Meta: U2FsdGVkX19TjChws4Zt/Q0KgYi82WNRTEnML5gw9UmQauBbcjWEDABVkoFCJOnNQ8JbIwLkjRJAyCzrPzgFi/broVnfhTVMxbLSvEimqLeHLmD9lXaVR62x3F+wleuRcvHIBIL5eRDdHUyNmTCd1fwcP5Q7VpIyE0k6YefmTtLPzaX/Q6xZCuU1ti3OHK+zSLwfVARsvmrirygLpqDMRLex3QiRNkXTwSfTvvT4fWfLXuCdSGn3eJ6S+w55OpD/3/Jnl0wgdTHjfR2F+o5pUnPGo6WKRkrGzIwCwwiDa+qmQL8CMsAnNy+QQ/LI2hj0WpW2fhnmJvieIzeT4GCEYN3wkK0x7qxMVed5cJHgjAuYOBHmpn1wCG8/gLRT+bPqKg20vhpXPEUQmu4mXAWg17BeFJuwsT+wdrZXPL6Zev8ObQuZJ7MYK7aNSVD+edJx7SEoG/yaI7lHj/SSSUswisIZU0JOHbcDcAA+6d4CPq/ebHI/saPr+ndG2E/RIBnJ1tEYtv4V4JBCZIFXpl8W+n8D7Dox3N9JYWuQHj9Qn+6rlxKgD9P8K2TM4Jrxw8XtWrXZK50N3yVSnTfxNx/syNG4R2n3djHEqLJvpVjtPWFR1wdcf3PSbjjKUYO+Pp2XHqEwTt904PjMyiVW2KzQtBroXj+onQzzEFYXuIMlq/CkkcJZBK0Qu527T9i57JzbkfbfPhK19JI+K/26aifD9dp34pBeKU7EuHzzeeiFXKglu5u/n75XdsSZh6P0+nPG5Xc1HS2ptLpysf61z3oBLRFUy2KAS7mYQ0GUx1w0oCvcdATazKmKQSUb7vysEEg5ShxhW68dxnV0FdKQEWu5h7bD5MH4lAfnuu8IenWYtUmNIxZal3+MDKn+dUhQHpVbFTmi7PS/9OLrXQp0XzBFAXBqcs9pu5j7ABVH8dzh+NoYWSMgo7QdxfKIU3B5eajT2I+EdGnNcOPBlDSDaq8 uY/QgxXp 4kQbwfxSaXIDkmNfWvt7RcMTbUfU654m3ULJ7whE2JKx3cYhO8nO7xZF+KPF+ijysKcnwrxx032S/r5hlqnKrphKqtFgzACaXy93PJA6Gx2KWmgXLAlMNR6Cx98gS72AmfKM1fU88C37+O4H5aQQSI7MTp1vpVK1xctDg+tGC9jfE+TOdKXSKT1I5mrqN3Kr9dPLqcErdot3MWguezuf5tB7ATYH7uY9s0GwcMIb/lloKGPs4lmQvGuR+jK9V1g+4kGGMsejX5fmdsQA= 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 Mon, Mar 03, 2025 at 06:16:54PM -0500, Johannes Weiner wrote: > On Mon, Mar 03, 2025 at 10:34:46PM +0000, Yosry Ahmed wrote: > > On Mon, Mar 03, 2025 at 04:55:24PM -0500, Johannes Weiner wrote: > > > pr_alert_ratelimited("Decompression error from zswap (%d:%lu %s %u->%d)\n", > > > swptype, swpoffset, name, clen, dlen); > > > > Yeah this looks much more concise. It's a bit harder to parser the dmesg > > as you have to cross check the code, but hopefully this is something > > that people rarely have to do. > > > > I don't feel strongly about adding a helper in this case, unless we want > > to add local variables (like Johannes did above), in which case a helper > > would be a good way to hide them. > > pr_alert_ratelimited("Decompression error from zswap (%d:%lu %s %u->%d)\n", > swp_type(entry->swpentry), swp_offset(entry->swpentry), > entry->pool->tfm_name, entry->length, acomp_ctx->req->dlen); > > Seriously, this does not warrant another function. I don't disagree, especially after this was shortened. I wanted a helper when this was a huge function call making zswap_decompress() more annoying to parse. I thought you wanted local variables to make the meaning clear after shortening the message, so my stance was to add a helper iff we do that. Otherwise what you have above LGTM. > > It's also valuable to keep warnings inside the problem context instead > of socking them away somewhere. It makes it clear that decompression > failure is a serious situation. We also expect this to trigger almost > never and it won't be tested routinely, so the best chance to fight > bitrot is to keep all those derefs close by. Imagine if this triggers > and the data is misleading or it crashes the system because some rules > around entry, acomp_ctx, the pool or whatever changed. Or if the work > involved in decompression changed and this is incomplete/unhelpful. Good point about bitrotting if rules change.