On Wed, Jan 31, 2024 at 3:23 AM Johannes Weiner <hannes@xxxxxxxxxxx> wrote: > > On Tue, Jan 30, 2024 at 12:13:30PM -0800, Nhat Pham wrote: > > On Mon, Jan 29, 2024 at 5:42 PM Johannes Weiner <hannes@xxxxxxxxxxx> wrote: > > > > > > Move pool refcounting functions into the pool section. First the > > > destroy functions, then the get and put which uses them. > > > > > > __zswap_pool_empty() has an upward reference to the global > > > zswap_pools, to sanity check it's not the currently active pool that's > > > being freed. That gets the forward decl for zswap_pool_cuyrrent(). > > > > nit: zswap_pool_cuyrrent() -> zswap_pool_current() :-) > > Whoops, my bad. > > Andrew, would you mind removing that typo inside your copy? > > > Also, would it make sense to move zswap_pool_current() above > > __zswap_pool_empty() to get rid of the forward declaration? I guess > > it's now grouped with current_get() etc. - those don't seem to use the > > empty check, so maybe they can also go above __zswap_pool_empty()? > > There is a grouping to these functions: > > - low-level functions that create and destroy individual struct zswap_pool > (create, destroy, get, release, empty, put) > - high-level functions that operate on pool collections, i.e. zswap_pools > (current, last, find) > > They were actually already grouped like that, just in the reverse > order. The only way to avoid ALL forward decls would be to interleave > the layers, but I think the grouping makes sense so I wanted to > preserve that. I went with low to high ordering, and forward decl the > odd one where a low-level function does one high-level sanity check. > > Does that make sense? Makes sense to me - just double checking :) Reviewed-by: Nhat Pham <nphamcs@xxxxxxxxx>
