linux-kernel - Re: [PATCH v7 4/8] mm: zswap: Refactor code to delete stored offsets in case of errors.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240925141100.GC875661@cmpxchg.org>
Date: Wed, 25 Sep 2024 10:11:00 -0400
From: Johannes Weiner <hannes@...xchg.org>
To: Yosry Ahmed <yosryahmed@...gle.com>
Cc: "Sridhar, Kanchana P" <kanchana.p.sridhar@...el.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	"nphamcs@...il.com" <nphamcs@...il.com>,
	"chengming.zhou@...ux.dev" <chengming.zhou@...ux.dev>,
	"usamaarif642@...il.com" <usamaarif642@...il.com>,
	"shakeel.butt@...ux.dev" <shakeel.butt@...ux.dev>,
	"ryan.roberts@....com" <ryan.roberts@....com>,
	"Huang, Ying" <ying.huang@...el.com>,
	"21cnbao@...il.com" <21cnbao@...il.com>,
	"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
	"Zou, Nanhai" <nanhai.zou@...el.com>,
	"Feghali, Wajdi K" <wajdi.k.feghali@...el.com>,
	"Gopal, Vinodh" <vinodh.gopal@...el.com>
Subject: Re: [PATCH v7 4/8] mm: zswap: Refactor code to delete stored offsets
 in case of errors.

On Tue, Sep 24, 2024 at 05:43:22PM -0700, Yosry Ahmed wrote:
> What I meant is "zswap_tree_delete(struct xarray *tree, pgoff_t
> offset)", and loop and call this  in zswap_store(). This would be
> consistent on looping and calling zswap_store_page().
> 
> But we can keep the helper as-is actually and just rename it to
> zswap_tree_delete() and move the loop inside. No strong preference.

Both helpers seem unnecesary.

zswap_tree_store() is not called in a loop directly. It's called from
zswap_store_page(), which is essentially what zswap_store() is now,
and that was fine with the open-coded insert.

zswap_tree_delete() just hides what's going on. zswap_store() has the
for-loop to store the subpages, so it makes sense it has the for loop
for unwinding on rejection as well. This makes it easier on the reader
to match up attempt and unwind.

Please just drop both.