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 3C291C54E60 for ; Sun, 17 Mar 2024 06:12:55 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A41756B0085; Sun, 17 Mar 2024 02:12:54 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9EF956B0087; Sun, 17 Mar 2024 02:12:54 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8B86E6B0088; Sun, 17 Mar 2024 02:12:54 -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 79ED86B0085 for ; Sun, 17 Mar 2024 02:12:54 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 2122812081D for ; Sun, 17 Mar 2024 06:12:54 +0000 (UTC) X-FDA: 81905512668.20.F203AA0 Received: from mail-ej1-f46.google.com (mail-ej1-f46.google.com [209.85.218.46]) by imf05.hostedemail.com (Postfix) with ESMTP id 3B62810000B for ; Sun, 17 Mar 2024 06:12:51 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=xmBkibu+; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf05.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.46 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1710655972; a=rsa-sha256; cv=none; b=7s99t8WWevnpc2FzhfAqFYZ5zYDL13NJa7RhNGp5+ImkTJae6yH1N3vKj+r6Cgg6wjEGn5 y+8ocMAOS6HvTglQFapZaKLVtlkB2c1/sPNIhIXIfj7OxczrsOI8Q6qiaX4C4IEEawpfc/ 2IUWShEejjA4BPhD5ZODNjz+615Cc+4= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=xmBkibu+; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf05.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.46 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=1710655972; 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=b1/EUrAw0DouFxLVGsQPEl2eEcK2Kdi+7MtyL3HOCTg=; b=Wcjyx8b8WaZDso4tZBioyiPSoG/HBmU4J2KxrGhdrvad/TQRJj0ZCoAo+NvViH/1u3P1bf SsrpWBxYBm17fcf0AtjLMsFr6kdMigxT8I8W2BS47iJ+Hg0RVz/b5bSR6i6OZkPFZJhcK4 iY8ZcHrQ4A1zjtur02Sx5nZO2npneAM= Received: by mail-ej1-f46.google.com with SMTP id a640c23a62f3a-a2f22bfb4e6so451010766b.0 for ; Sat, 16 Mar 2024 23:12:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1710655971; x=1711260771; darn=kvack.org; 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=b1/EUrAw0DouFxLVGsQPEl2eEcK2Kdi+7MtyL3HOCTg=; b=xmBkibu+j1mBg1EIWSGvOhzHZ1t0rEQtD97/cbuft+q8YFIxyUB8b8Azmm9OzLJ2xe cL0NZMAX/+cSqcGEoGyFHODtAY8uKELkxU7D3BbguZ49SVn1Ln3iyCT5ruINwNs7i2Q6 4x69RFyzJmPq7p6Yh+LuWCk9WPjKQsTQtDl8Uke6tw1tVaRirdSnCa41quEZDYNpUDzb DjQaYHFdHIwYoeSUIob458yXlBh0xQwFvP4FTW8WDEfh+EgdHBbXCBv0NJhdxfM+99XJ AK7LrcS9yJhtEB+/yxwwTL5nCOZYlD5UcfTATqEvDOVwqjpihup2GlGqc0XxjjDbTfAx 6Oxw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710655971; x=1711260771; 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=b1/EUrAw0DouFxLVGsQPEl2eEcK2Kdi+7MtyL3HOCTg=; b=X3Zo5VBmDhDjrFZmEEQzCahqeJgWkxPK5amZcldK1GImFsRhwkf6bvG/Y7Yej/XPip BxgaNi+wtes1rsbjrh/Tw0UkvN1Fg+Gy5YlY1EKoz+TT94tpjQ9Ubv/YpUPwCrDPUC3F nJ+XELBvxk+/lGnRDJ/YOZvzvKezO2dISlUV3s9sGbdP6v8YcEZizBDOyeJZJ8U80O3e 6FL09jg8JUmMDAoseTEJwaViMDu2VnECXg5XDYh5ygAKIQvuxjxvkgWslGG5r8G6JIDo j/+iw9c054NAzgNpBNnTCTw8gCpnt0wvTCF79IGdBG0l2ByYVhcpUCdVUawTVSxTa0BC ZATA== X-Forwarded-Encrypted: i=1; AJvYcCXizgKpBGt44Uicn6sE6sttJBDnYKx8uGdnopfqgEcQRkig/0p4+QBF4zAj0Fcr98rxTeLtrbt54mrMwmeJZivbLhs= X-Gm-Message-State: AOJu0YwC0pZcpE8RRzxeP7kV8qL08hG48Nz6NsE6K787tUQGJ+wCJL4E I5DLfaU66oV8/yrcM3Np7ebMy0PfOPJAqooQJzBRpXaJUV0IsN+ezb7pY9OKPjkrmsXipQmSjAE bra9WA6nh+lR0QtLSWilgw53CfA+rhsTCthdk X-Google-Smtp-Source: AGHT+IGRePtBL60zVIJk36iKwEufwRgi6VaTp5KLz2bpTexL+aRZxvhL0SaG3Ljf5ChlRghwPUywlFW4sZ7o203biU4= X-Received: by 2002:a17:906:6d46:b0:a46:b8a8:98e9 with SMTP id a6-20020a1709066d4600b00a46b8a898e9mr192482ejt.25.1710655970494; Sat, 16 Mar 2024 23:12:50 -0700 (PDT) MIME-Version: 1.0 References: <20240312-zswap-xarray-v6-1-1b82027d7082@kernel.org> <20240316133302.GB372017@cmpxchg.org> In-Reply-To: <20240316133302.GB372017@cmpxchg.org> From: Yosry Ahmed Date: Sat, 16 Mar 2024 23:12:13 -0700 Message-ID: Subject: Re: [PATCH v6] zswap: replace RB tree with xarray To: Johannes Weiner , Chris Li Cc: Andrew Morton , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Nhat Pham , "Matthew Wilcox (Oracle)" , Chengming Zhou , Barry Song , Barry Song , Chengming Zhou Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 3B62810000B X-Stat-Signature: wmi1dk6mysa3cei9s5fcmo7uwxbaj4uj X-HE-Tag: 1710655971-962232 X-HE-Meta: U2FsdGVkX18B4ZR6XsW4/R59/ov/UL0tqOT4SJmjlSP8VwsyyedchBF72nGw8RTwBZYHnbyHSpB4tEnTZd/Uh8ssMQ4e5lQw1lw57hLemnW2uhvri9aF+HRkW/XdZERP+eZTK9qEQ5cKWJoPXlnTj3sC3zh4f5+MQOKbb0i9Y2p3FF3tuPU1Zxc36uT1MHsu27aus0wW7R+9jR9/gwinKvmUv9HLUqfJytFNrgRxR2zzmgDi6VH13zCBm+iDu9pn/U6Gh1P8pyT7kS+3C2WZM8qkvgih9FtzCpk1Bt0oD0fr44dAmc3cy1fcFV3S6J03W33QuR86ws4unTMLrgq8KrPbriZgTYIlRyCqhQrzOmNd9zOTm9Sk8eCuaRpIXX+h210n/tvas6849tEt1uSmp5hNd+VlanehyjanGlFe9haiyai68l2SeOW7suTNnAe0pvkMqpnraEtEZQUG+fqp0p49SiTs9Mk0mqhbLOvIjrFcYGmquBYthLiM/53nv5RYelbn02xkrqkCirG0UrjJENWx2UoV4WcURMtcaS0qo6N72ZhYpPHWNuZututfrNLqglUFdQt78THXpBeBg/Zt2MbTd08FXFznwUSUpk36lg5mncfKyocEY6hcfq/7VpI5xicnnyf9b4BjMRT3ozU3rSCnukQJF6NwtcuREuDZIJ7tovCm4H+MVEsk/+2EYpyjHpkggr244pKn0aKOSeNi0pO8Aj3vpRwUkdpRhwH4asSEEp6BqxVtxNz1JZefkfkgTQJRWRSj0xO2lA9V6X8GTlAZHrPFSqIYzcHpG7nnPQdLtwvqYqm9A2+OKY50VbpoQzUUmaq9TDAZTe3gkClJTvMgzu+awfpDKV7Kl48Jd9kCFbxHdZZ12k7COUVW19P4tEdw+URrw1Fgi79J+dlj7ASrVcuLv2DG16OD8LogyCEaUxZZY+fYI+iBgkp3LPNF9oA4TvjyZ71ah9KXJcB gobF4RtI 4EJZ2WX/qACjKcG3E8546KR/yymcScus+F3k6qFCoOudMZdbM7IDtKR2jpLdbdBytklNvL1DYgkgCatw4MQTqjK4p0fm5msCsn8g99aRIYqUa9E0UD8jjsIfGrpMYte1MbEguWL4k8kX4+VFcu3LX1FvjZiZt1OFmeBBmz3gbeZuXQDwMQwZhwP6I7eTISh0OJkKPAhdty98ZfZBJWKAgDL/XPbfSKKZ4CKnG40ZSMqzDMDOG1HuqhwtHFbNC+GBNAsNVE+p5HN0Rb0bn7WXfYc/22bm6W0AvYELM X-Bogosity: Ham, tests=bogofilter, spamicity=0.000040, 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 Sat, Mar 16, 2024 at 6:33=E2=80=AFAM Johannes Weiner wrote: > > On Fri, Mar 15, 2024 at 06:30:37PM -0700, Yosry Ahmed wrote: > > [..] > > > > > > @@ -1555,28 +1473,35 @@ bool zswap_store(struct folio *folio) > > > insert_entry: > > > entry->swpentry =3D swp; > > > entry->objcg =3D objcg; > > > - if (objcg) { > > > - obj_cgroup_charge_zswap(objcg, entry->length); > > > - /* Account before objcg ref is moved to tree */ > > > > > > I do not understand this comment, but it seems to care about the > > charging happening before the entry is added to the tree. This patch > > will move it after the tree insertion. > > > > Johannes, do you mind elaborating what this comment is referring to? > > It should be clarified, updated, or removed as part of this movement. > > Wait, I wrote that? ^_^ Well, past Johannes did :) > > The thinking was this: the objcg reference acquired in this context is > passed on to the tree. Once the entry is in the tree and the > tree->lock released, the entry is public and the current context > doesn't have its own pin on objcg anymore. Ergo, objcg is no longer > safe to access from this context. > > This is a conservative take, though, considering the wider context: > the swapcache itself, through folio lock, prevents invalidation; and > reclaim/writeback cannot happen before the entry is on the LRU. Actually, I think just the folio being locked in the swapcache is enough protection. Even if the entry is added to the LRU, the writeback code will find it in the swapcache and abort. > > After Chris's patch, the tree is no longer a serialization point for > stores. The swapcache and the LRU are. I had asked Chris upthread to > add an explicit comment about that. I think once he does that, the > objcg situation should be self-evident as well. Perhaps it should be clarified that the swapcache on its own is enough protection against both invalidation and reclaim/writeback, and the entry not being on the LRU is *additional* protection on top of that against reclaim/writeback. Right? > > So in the next version, please just remove this now stale one-liner. Thanks for confirming. Chris, please remove this comment and update the comment Johannes asked you to add as mentioned above. Thanks!