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 B7FD9C0015E for ; Thu, 27 Jul 2023 18:15:10 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3BCF46B0075; Thu, 27 Jul 2023 14:15:10 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 36C4C6B0078; Thu, 27 Jul 2023 14:15:10 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2367E6B007B; Thu, 27 Jul 2023 14:15:10 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 13B3D6B0075 for ; Thu, 27 Jul 2023 14:15:10 -0400 (EDT) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 91A34C1168 for ; Thu, 27 Jul 2023 18:15:09 +0000 (UTC) X-FDA: 81058193538.16.AD4BF95 Received: from mail-ej1-f41.google.com (mail-ej1-f41.google.com [209.85.218.41]) by imf15.hostedemail.com (Postfix) with ESMTP id 87947A0026 for ; Thu, 27 Jul 2023 18:15:07 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=riHjHppP; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf15.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.41 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1690481707; 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=cDXv3GJoBhlQr8pJG4ewngE2yTfvQ54xOjhSQv72UGM=; b=5jPr2WdjDPdPjrtu4kUnoclrpyiwS97B5BDQdPN4n4rgXDNLr2Gbf5f+oLcThtSLvb1Aq+ 3HrbP4cBQcjznL24408yl677WwrsZbXlhCOzKgd0YiRAyCtd0mFPboO5COIlt1ZWD3r0ZW DppPMXAH5nMy/4CiTr7NrmTc43gHqhs= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=riHjHppP; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf15.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.41 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1690481707; a=rsa-sha256; cv=none; b=edb8XGWVmvtPEA/XnYUqLOtay9eo9ydLWx3c6wMwZyeQw0C7wgrI84SSLIkTEfwICrDFle K7fX5QdfDuQ+Xw371Wau0SCnGUOZbLAvu2+qISDNQB9/dQZtmOkG/BiCCplW2YVy+hU5C/ GlLyb+M1tac1YnGLhRm0ZUE4locwt6w= Received: by mail-ej1-f41.google.com with SMTP id a640c23a62f3a-99357737980so165932466b.2 for ; Thu, 27 Jul 2023 11:15:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1690481706; x=1691086506; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=cDXv3GJoBhlQr8pJG4ewngE2yTfvQ54xOjhSQv72UGM=; b=riHjHppP5NEozcwiEl5iB7KyWW+jJ8XGwJB6AaeCCX81SAoydQGvrnLtUth9zm28kg GdNWiC0B3pWMzmKZC9XzE89drFEXtLyLZT70sTmA+Jf2ch+1jmAib07EusGuLCLhpdkw 5GiYJ1Qa4av/NDXOrUdR2bK2twvH88e5Q1/Blqaq8asjz+9TvTon2hkoC2BQTe5UcIt7 Q2Qas0SyPR8GUaQyFP040jmAp/PN8srIJDDhqvkYmfODXqGVi16LL+ZyhUSFgb49KFey HR0fvNYdpn/suObjC7F2359RSzItBB8iqPCm1h/39mSqel1YrrOu7CrNrUOamyd7v/AR nA6Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690481706; x=1691086506; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=cDXv3GJoBhlQr8pJG4ewngE2yTfvQ54xOjhSQv72UGM=; b=Ix2iioHeiKM+f0R9TVilXzVgbvcSbuYobZjAAgdYLZm82z3sac+iujBtUWOOgp3KsR 4ESr2QTQCymXD5HN9MMqmk2yU2QCLn6Q4mN+6J0ar/tLcTWxr776BSfCYwbIEtCbeOCm EGqkw9uEwEsqO4h56gA/AAMkZAF9e0DVl5SncUTfZ3QYUBYRnG/x8HjPjZzp5HI8Y8Z4 0smpa2PKbM9/ipqqzbnTuALyFF4UhM2AjvPVKiF7Z/YfxqKUhthBStij4SS3xaMj8eBz 90vVmO/wfSnbS4FZ/OcFDg+3CEgWKb6nS7Kbz8ayKco49yINAOOusfVJt4c9gu8HCzJ3 IDJw== X-Gm-Message-State: ABy/qLYullAjW7TVNEqY2/bZgm3yuCHG4ceqmn7sbRE2Icb0ctUpjJwx D7LcVxrn1/UuT3BUUTh4dfBQN8i/10ywg80VRCj1DQ== X-Google-Smtp-Source: APBJJlEVxmSgSqqQ8XGRbfXYOgqCkt9UjQRI0/mK5ZuVzQyFMrcfTJN87jfsvbkdR97fG3091Oa9xERnN3FI3L9gjow= X-Received: by 2002:a17:906:59:b0:993:dd1d:8251 with SMTP id 25-20020a170906005900b00993dd1d8251mr2807834ejg.28.1690481705936; Thu, 27 Jul 2023 11:15:05 -0700 (PDT) MIME-Version: 1.0 References: <20230727162343.1415598-1-hannes@cmpxchg.org> <20230727162343.1415598-3-hannes@cmpxchg.org> In-Reply-To: <20230727162343.1415598-3-hannes@cmpxchg.org> From: Yosry Ahmed Date: Thu, 27 Jul 2023 11:14:29 -0700 Message-ID: Subject: Re: [PATCH 2/3] mm: zswap: tighten up entry invalidation To: Johannes Weiner Cc: Andrew Morton , Nhat Pham , Domenico Cerasuolo , linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 87947A0026 X-Stat-Signature: emegk1ur51r8763tok9ww6ttu4hi5ek3 X-Rspam-User: X-HE-Tag: 1690481707-574599 X-HE-Meta: U2FsdGVkX195gHOSUiemS+mw8FwmEl/XGoOLeXQsw7kXRIOdsjcrf/E8XDGjHyt33rtVf+FUatJTfxODzy+sdVPn7QHGPGJs5trTrPZj5frwWsfSh/n4NwSCKQO3hIX0+8max6lIR2ZGwGZV1z8VFdLef13AN9BTz1YuIXjQ4RhJ39vCXuANTIVyW/Q8ajkIrUA5w0KSFlHfNZdxfzYG6nQNv3l1Q/BO8ypw28zIX119SwrCS6n1PMnRhGmMDyYID0jVKobj2+0QYB8DpgFkxY5GYIcUBCVRPITKexCnkL8GnmLX/EPpETuCPYl8aLP9TbaUZlXVFNJJjcQiq1n0IJcL63yVzwJxhudAZ3wTd/NQrcLY5BDAlx/INFqRYqOtHQiIFkM2hO1K8IFPPJctUO38QvSPn4uTyLisBRQcFxXtteamxGSPF0HFVwUUEw2+wO0rwGpNejB2L3NmpcH7p9VmqxdwpuNoTP1RiSQrVlenjBudighwewhETPjGbhmMlwVfSkZaDEETG5N0Fc2iYX4nDQWoNc5kbq/r0P9X0R9C2ZM+TKFfqcHAz9toMMf2vYu/1Cj3BTYwupomSRY7wcBX1Tf931JR7tBy4/OltWGirrZ//iYx0ufbm2AJarloDv1h9s0sh8NS/LTHKbvGp9rlii3vtZm99Am2o2VoUOJZS4uAga6LWUAQoDi4p0pPvsn+sFXKp0dCzOOeQN2oiPmzpkxZMfAnZTknxpRvnmV9pchhtTnUP4bm47FjN2Mi/k2uv5FMFRcpVw2gfVMGPdU9FwvfRrZxsVe9zHZOxENFEBnbudpPBj1KhNzsretlSUu1HTmTHplxFBTWJtev2v6RlZLpEoQjoXjaJeQ+6kD5Eufb0J0Ooek0+K5hlOT8T4R7lddH9mInsI9n4BbSxqio0soNxtKwaYjRPCz9/BPf4uf6QEt9decXVlgpV/cbQXJpKV25xVVSGRRSsQV 491n3jLx kEnGoPjskCZXwMfZC3ZpnQXpJm6KFCQaRq1Bn7vBZrlCznltk3zWpmLqP7aqmZjkoTRpdEzTkh8JoE5tNyrZ3xzQzqU5ye/LJRjc25TCje29AabhrNUhOlBBS5WQsIA5e1pawDTbaJgZW+xQgDngDFKD5sASBN43+cg9slMfLidp02dNFIuf7U4pE9+4eTMqENrVSm8s38ErDtSEMRcyQs/skUkUyyxCcIvaRfwSTm9BaUJGrAWP2nj1U8o/hgsScH3G8+FG08e3ZBukwBrqfBS0XAVpREYcPlSs6uWI7cWix9aBU7IDaI1Vmgg== 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: On Thu, Jul 27, 2023 at 9:23=E2=80=AFAM Johannes Weiner wrote: > > Removing a zswap entry from the tree is tied to an explicit operation > that's supposed to drop the base reference: swap invalidation, > exclusive load, duplicate store. Don't silently remove the entry on > final put, but instead warn if an entry is in tree without reference. > > While in that diff context, convert a BUG_ON to a WARN_ON_ONCE. No > need to crash on a refcount underflow. > > Signed-off-by: Johannes Weiner I have always found it confusing that we explicitly remove the zswap entry from the entry in the contexts you mentioned, yet we have zswap_rb_erase() called in zswap_entry_put(). In fact, I think in some contexts this leads to zswap_rb_erase() being called unnecessarily twice on the same entry (e.g. once from invalidation, then once again when an outstanding local ref is dropped). It's probably harmless with the current implementation, but such a design can easily go wrong. Thanks for the cleanup, it would be interesting to see if this warning is actually fired. Reviewed-by: Yosry Ahmed > --- > mm/zswap.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index e123b1c7981c..e34ac89e6098 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -402,9 +402,9 @@ static void zswap_entry_put(struct zswap_tree *tree, > { > int refcount =3D --entry->refcount; > > - BUG_ON(refcount < 0); > + WARN_ON_ONCE(refcount < 0); > if (refcount =3D=3D 0) { > - zswap_rb_erase(&tree->rbroot, entry); > + WARN_ON_ONCE(!RB_EMPTY_NODE(&entry->rbnode)); > zswap_free_entry(entry); > } > } > -- > 2.41.0 >