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 487BFC47DD9 for ; Fri, 22 Mar 2024 13:36:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C04776B0082; Fri, 22 Mar 2024 09:36:10 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B8BEF6B0083; Fri, 22 Mar 2024 09:36:10 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A2BE96B0087; Fri, 22 Mar 2024 09:36: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 8D2BD6B0082 for ; Fri, 22 Mar 2024 09:36:10 -0400 (EDT) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 349B2121197 for ; Fri, 22 Mar 2024 13:36:09 +0000 (UTC) X-FDA: 81924773658.09.E636C8E Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by imf11.hostedemail.com (Postfix) with ESMTP id 88A9A4001A for ; Fri, 22 Mar 2024 13:36:06 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=ptLKrBtb; spf=pass (imf11.hostedemail.com: domain of chrisl@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=chrisl@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1711114567; 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=b4NGyPDpVDTT3d+LMeo1IhJ6qBpdHELPHlqxJ2QV4o0=; b=s3NgSDWyz9GUvjxOHIQp8aRIl5ULPVG6LI7gAWmddRYp/ZEm+YIc0a+0JBLW1WbChFQmKt Ocfkl4D/VKjoYeVBTHNLi1Qt3xhqUyZZY5913Uem0pf4qLX3VMM8Uvwm6mOdWQPJ/jb8ZP Ee6PyQ7Q03ax4/ke/p8PVpGTt9KYItk= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1711114567; a=rsa-sha256; cv=none; b=IvFPlRQZ50T30N9C4Fi1U0Qh9G26oHlPq7dsaIA0mQi5+30TKixKNW4HHcOZHsRVqKlM2V d6mV/bKemYtWuMPP5VxOxv+BUx1GiMegYx3QbK7MyLJGjBU0C69ZEUVPTn/sN37Cnyn/ZV x9XPT3ShG0IAB6qQ31yvuHZCtowETHQ= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=ptLKrBtb; spf=pass (imf11.hostedemail.com: domain of chrisl@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=chrisl@kernel.org; dmarc=pass (policy=none) header.from=kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 30993CE194D for ; Fri, 22 Mar 2024 13:35:57 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 648D8C433B1 for ; Fri, 22 Mar 2024 13:35:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1711114556; bh=u0PRJPH4zZcFF3xqQA9fGoW/XmvWMaMyEmDCCXX09wU=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=ptLKrBtbhchtawol//pqO8ctqLfceMYNRVf2k8KlxloAAyepmU2L5ys4vO73YQbth WX6VqEXu4YaLfFdofK5Clz3eaiC3S59dYFD18zYRbY2YKxoepuxeHIDcfVN3jNrfxE Ae6ibCWeE92J4cwYF/95AjR00uM5RNqyQ5+UehyZG42j3IRMfV1wKOKvF5WD9pP5Gu q4qirbN834oFjj8uzQFJO3Yb+uwzYrtTGwKfNqPu3TxOJ+ea6aYOQXpGcBHqx7QyGA KCWsFT4caDHVxwMtKPT5/pXCJlmw3wzuqIsYBgrRwlse1dbF17d8YIzFsOgsqYijcF 8/zqAkWiQS54g== Received: by mail-il1-f169.google.com with SMTP id e9e14a558f8ab-366a4bcb2a8so9435335ab.3 for ; Fri, 22 Mar 2024 06:35:56 -0700 (PDT) X-Forwarded-Encrypted: i=1; AJvYcCUqL2iucYyXAt3WVaIocYuxySXYc/Do8GRba5QX80tU8Vep42IXGZ5ymAClerf3+NbiYj8PF4uQvRfzqAzMI4OXDIE= X-Gm-Message-State: AOJu0YzSlH0kGr6NTLH9OfrLosTLpOAH/g7RGdT7kQV2gScUAoBPUzxP MW8cOzvtp0iZYK5zYxknFgkK90hod7J5neh7Bp42vHWZ9uwpUBkb1epmdtTCd4VhMOnDuhHITF0 cTOegDVdOCaUzeP46XMmyZaM0C1tDR4+RNCTv X-Google-Smtp-Source: AGHT+IHqG3BNO/6Gg345YJ6k829I7v6On+D2mOXMOpo54HKOVzrfbcNkKWNSvPjTFGCJmCjixRNTr8OgiPkFHD85TJA= X-Received: by 2002:a05:6e02:1252:b0:368:727e:ec71 with SMTP id j18-20020a056e02125200b00368727eec71mr1486400ilq.21.1711114555719; Fri, 22 Mar 2024 06:35:55 -0700 (PDT) MIME-Version: 1.0 References: <20240321-zswap-fill-v1-1-b6180dbf7c27@kernel.org> <20240322031907.GA237176@cmpxchg.org> In-Reply-To: <20240322031907.GA237176@cmpxchg.org> From: Chris Li Date: Fri, 22 Mar 2024 06:35:43 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] zswap: initialize entry->pool on same filled entry To: Johannes Weiner Cc: Yosry Ahmed , Andrew Morton , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Nhat Pham , Chengming Zhou Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 88A9A4001A X-Rspam-User: X-Stat-Signature: 8fonk41khdsjce6rce8gindf3fyedoxw X-Rspamd-Server: rspam03 X-HE-Tag: 1711114566-866644 X-HE-Meta: U2FsdGVkX1++47FCPNZ1D/0Fa6Z7yZhl4/HHuPtC2IiCfZshQeUk+sxBp/b2RawpZTvYEJjRTMxTGVblSSnwEVOp+x9vx7t39XqGoYAwcqeF8Lku4gAn1fnT1THC+xknduAhkqTfPbOFgx3hwXrBTb1PJbp09mYXgln0wVhOTIfKrDselGbmhULVmYx+2lLg5cpWxDyf+ZJESxkTtbli3YqdRitoP1lgjlEB2Tlks4UQCcIVuD2kDRrZO+C+NKcHszOJtHWzIISN4AZhoyDAaQhFW6EgdS8LMv7eSHsk6NITahfHYYBy/jr9fXSlU7DSdhCltQbircWqOaP4G0j/whPO8QqAZfDd8iCIUAMw4t7yNgjIez0DXFGQaQ0srffl0piqmF59CTDean37KcizQSuaz880rzIK5xRbaNlgMGePcMgTaximzCP5VYCq9Lk/pb5L1iq2UEpRwSWT5vIA0+Ct/VqtsAFCoNxssFHjzsqaXr4KIH+nlFBftShbaf+VEAkMKldPX2MP0BDm0bhKQFjmvfIvDT36qm732ZKZhnte7saSQOK2zRQqdFqDlHs60uD38PxJCG+hNW9v1sBLFipazep0UvFzHI2j3xgbw7Csu5zkiXsXHuns8PxzFTjTH/ywtzz1Vc9w9bl7Mdj/xjP96aIcdAc+YU0DPX5EqeP1MOR2ksTXk6jjDpFqodnwfsHv3D6EpfSFoTpb7EbIhOx1xaLQUaA11LJ2C3Czhujyk0W3xPu2JTkfgQTmSGUxgekyE8I1KQVejj5TPqE4hf8/QEquob9B49q6IYKMb5tuqXK3Uek7/ZvFhTmsi/tq/JogROINUwiMGMErkIv1VSj0qi1fU1M0BwcQddUKNukNtMrk4ctjiG/sL6Y+b6VM8YMC+VXhgaPDTyIeszTSvLmjPzvOP2hKYI+s/7fHiR04xxcDgtPknpBVSig011woY2zpv1BVRY6UMNo3uzW SuyNvUTt sv0mP9PPuGnHTen3JVCbsK8oHIH58pPO6Meh74KM3/4kNumNrI6ABRShR+BagxiEpIFYg5b6Wa1d+QuaTGdB8Ho6eqF7VTW3jdPMZCed8T7l11GnLo2xXPHSIQGMpfIji3csDfaQVc59XjRWScqO8kaXskA2ynGpmo7kD/x2DHYEJ0/Gy8TiGE71T1KQ0Z35/HeF5uc0H5nEtnFNPjm1yI69ABrXHmQngqZV7GcWcOvV/7PQQG+aH5ErHug818gFowoEYasesmxU+o5v3GaZv4aiT8XXoqghZlIlbYQXLPi/vsjxsHEL5pkdwuYiWt7KuNMQ/Mz3+lGXc70ovsNrlkqikSfS1r7oOH6hG 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, Mar 21, 2024 at 8:19=E2=80=AFPM Johannes Weiner wrote: > > On Thu, Mar 21, 2024 at 04:56:05PM -0700, Yosry Ahmed wrote: > > On Thu, Mar 21, 2024 at 4:53=E2=80=AFPM Chris Li wr= ote: > > > > > > Current zswap will leave the entry->pool uninitialized if > > > the page is same filled. The entry->pool pointer can > > > contain data written by previous usage. > > > > > > Initialize entry->pool to zero for the same filled zswap entry. > > > > > > Signed-off-by: Chris Li > > > --- > > > Per Yosry's suggestion to split out this clean up > > > from the zxwap rb tree to xarray patch. > > > > > > https://lore.kernel.org/all/ZemDuW25YxjqAjm-@google.com/ > > > --- > > > mm/zswap.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/mm/zswap.c b/mm/zswap.c > > > index b31c977f53e9..f04a75a36236 100644 > > > --- a/mm/zswap.c > > > +++ b/mm/zswap.c > > > @@ -1527,6 +1527,7 @@ bool zswap_store(struct folio *folio) > > > kunmap_local(src); > > > entry->length =3D 0; > > > entry->value =3D value; > > > + entry->pool =3D 0; > > > > This should be NULL. > > > > That being said, I am working on a series that should make non-filled > > entries not use a zswap_entry at all. So I think this cleanup is > > unnecessary, especially that it is documented in the definition of > > struct zswap_entry that entry->pool is invalid for same-filled > > entries. > > Yeah I don't think it's necessary to initialize. The field isn't valid > when it's a same-filled entry, just like `handle` would contain > nonsense as it's unionized with value. > > What would actually be safer is to make the two subtypes explicit, and > not have unused/ambiguous/overloaded members at all: > > struct zswap_entry { > unsigned int length; > struct obj_cgroup *objcg; > }; > > struct zswap_compressed_entry { > struct zswap_entry entry; > struct zswap_pool *pool; > unsigned long handle; > struct list_head lru; > swp_entry_t swpentry; > }; > > struct zswap_samefilled_entry { > struct zswap_entry entry; > unsigned long value; > }; I think the 3 struct with embedded and container of is a bit complex, because the state breaks into different struct members How about: struct zswap_entry { unsigned int length; struct obj_cgroup *objcg; union { struct /* compressed */ { struct zswap_pool *pool; unsigned long handle; swp_entry_t swpentry; struct list_head lru; }; struct /* same filled */ { unsigned long value; }; }; }; That should have the same effect of the above three structures. Easier to visualize the containing structure. What do you say? Chris > > Then put zswap_entry pointers in the tree and use the appropriate > container_of() calls in just a handful of central places. This would > limit the the points where mistakes can be made, and suggests how the > code paths to handle them should split naturally. > > Might be useful even with your series, since it disambiguates things > first, and separates the cleanup bits from any functional changes, > instead of having to do kind of everything at once... >