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.