| Message ID | 20240311161214.1145168-2-hannes@cmpxchg.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] mm: zswap: optimize zswap pool size tracking | expand |
On Mon, Mar 11, 2024 at 12:12:14PM -0400, Johannes Weiner wrote: > All zswap backends track their pool sizes in pages. Currently they > multiply by PAGE_SIZE for zswap, only for zswap to divide again in > order to do limit math. Report pages directly. Nice. Although I would prefer renaming the zpool interface to total_pages and renaming the zpool backends functions as well to use pages rather than size. Either way: Acked-by: Yosry Ahmed <yosryahmed@google.com>
On Mon, Mar 11, 2024 at 10:12:02PM +0000, Yosry Ahmed wrote: > On Mon, Mar 11, 2024 at 12:12:14PM -0400, Johannes Weiner wrote: > > All zswap backends track their pool sizes in pages. Currently they > > multiply by PAGE_SIZE for zswap, only for zswap to divide again in > > order to do limit math. Report pages directly. > > Nice. Although I would prefer renaming the zpool interface to > total_pages and renaming the zpool backends functions as well to use > pages rather than size. Ha, I was on the fence, since it's kind of churny. But if you don't mind, then it works for me as well. > Either way: > Acked-by: Yosry Ahmed <yosryahmed@google.com> Thanks. diff --git a/include/linux/zpool.h b/include/linux/zpool.h index 3296438eec06..a67d62b79698 100644 --- a/include/linux/zpool.h +++ b/include/linux/zpool.h @@ -53,7 +53,7 @@ void *zpool_map_handle(struct zpool *pool, unsigned long handle, void zpool_unmap_handle(struct zpool *pool, unsigned long handle); -u64 zpool_get_total_size(struct zpool *pool); +u64 zpool_get_total_pages(struct zpool *pool); /** @@ -91,7 +91,7 @@ struct zpool_driver { enum zpool_mapmode mm); void (*unmap)(void *pool, unsigned long handle); - u64 (*total_size)(void *pool); + u64 (*total_pages)(void *pool); }; void zpool_register_driver(struct zpool_driver *driver); diff --git a/mm/z3fold.c b/mm/z3fold.c index 9bacacd4168c..2ebfed32871b 100644 --- a/mm/z3fold.c +++ b/mm/z3fold.c @@ -1237,12 +1237,12 @@ static void z3fold_unmap(struct z3fold_pool *pool, unsigned long handle) } /** - * z3fold_get_pool_size() - gets the z3fold pool size in pages + * z3fold_get_pool_pages() - gets the z3fold pool size in pages * @pool: pool whose size is being queried * * Returns: size in pages of the given pool. */ -static u64 z3fold_get_pool_size(struct z3fold_pool *pool) +static u64 z3fold_get_pool_pages(struct z3fold_pool *pool) { return atomic64_read(&pool->pages_nr); } @@ -1402,9 +1402,9 @@ static void z3fold_zpool_unmap(void *pool, unsigned long handle) z3fold_unmap(pool, handle); } -static u64 z3fold_zpool_total_size(void *pool) +static u64 z3fold_zpool_total_pages(void *pool) { - return z3fold_get_pool_size(pool); + return z3fold_get_pool_pages(pool); } static struct zpool_driver z3fold_zpool_driver = { @@ -1417,7 +1417,7 @@ static struct zpool_driver z3fold_zpool_driver = { .free = z3fold_zpool_free, .map = z3fold_zpool_map, .unmap = z3fold_zpool_unmap, - .total_size = z3fold_zpool_total_size, + .total_pages = z3fold_zpool_total_pages, }; MODULE_ALIAS("zpool-z3fold"); diff --git a/mm/zbud.c b/mm/zbud.c index b7d8a22bbf5f..e9836fff9438 100644 --- a/mm/zbud.c +++ b/mm/zbud.c @@ -365,13 +365,13 @@ static void zbud_unmap(struct zbud_pool *pool, unsigned long handle) } /** - * zbud_get_pool_size() - gets the zbud pool size in pages + * zbud_get_pool_pages() - gets the zbud pool size in pages * @pool: pool whose size is being queried * * Returns: size in pages of the given pool. The pool lock need not be * taken to access pages_nr. */ -static u64 zbud_get_pool_size(struct zbud_pool *pool) +static u64 zbud_get_pool_pages(struct zbud_pool *pool) { return pool->pages_nr; } @@ -410,9 +410,9 @@ static void zbud_zpool_unmap(void *pool, unsigned long handle) zbud_unmap(pool, handle); } -static u64 zbud_zpool_total_size(void *pool) +static u64 zbud_zpool_total_pages(void *pool) { - return zbud_get_pool_size(pool); + return zbud_get_pool_pages(pool); } static struct zpool_driver zbud_zpool_driver = { @@ -425,7 +425,7 @@ static struct zpool_driver zbud_zpool_driver = { .free = zbud_zpool_free, .map = zbud_zpool_map, .unmap = zbud_zpool_unmap, - .total_size = zbud_zpool_total_size, + .total_pages = zbud_zpool_total_pages, }; MODULE_ALIAS("zpool-zbud"); diff --git a/mm/zpool.c b/mm/zpool.c index 410808aee7fe..b9fda1fa857d 100644 --- a/mm/zpool.c +++ b/mm/zpool.c @@ -321,16 +321,16 @@ void zpool_unmap_handle(struct zpool *zpool, unsigned long handle) } /** - * zpool_get_total_size() - The total size of the pool + * zpool_get_total_pages() - The total size of the pool * @zpool: The zpool to check * * This returns the total size in pages of the pool. * * Returns: Total size of the zpool in pages. */ -u64 zpool_get_total_size(struct zpool *zpool) +u64 zpool_get_total_pages(struct zpool *zpool) { - return zpool->driver->total_size(zpool->pool); + return zpool->driver->total_pages(zpool->pool); } /** diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c index 398f3856817f..b42d3545ca85 100644 --- a/mm/zsmalloc.c +++ b/mm/zsmalloc.c @@ -399,7 +399,7 @@ static void zs_zpool_unmap(void *pool, unsigned long handle) zs_unmap_object(pool, handle); } -static u64 zs_zpool_total_size(void *pool) +static u64 zs_zpool_total_pages(void *pool) { return zs_get_total_pages(pool); } @@ -414,7 +414,7 @@ static struct zpool_driver zs_zpool_driver = { .free = zs_zpool_free, .map = zs_zpool_map, .unmap = zs_zpool_unmap, - .total_size = zs_zpool_total_size, + .total_pages = zs_zpool_total_pages, }; MODULE_ALIAS("zpool-zsmalloc"); diff --git a/mm/zswap.c b/mm/zswap.c index 7ed79caf1e1e..9fdf4c76d5ea 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -519,7 +519,7 @@ unsigned long zswap_total_pages(void) int i; for (i = 0; i < ZSWAP_NR_ZPOOLS; i++) - total += zpool_get_total_size(pool->zpools[i]); + total += zpool_get_total_pages(pool->zpools[i]); } rcu_read_unlock();
On Mon, Mar 11, 2024 at 10:36:37PM -0400, Johannes Weiner wrote: > On Mon, Mar 11, 2024 at 10:12:02PM +0000, Yosry Ahmed wrote: > > On Mon, Mar 11, 2024 at 12:12:14PM -0400, Johannes Weiner wrote: > > > All zswap backends track their pool sizes in pages. Currently they > > > multiply by PAGE_SIZE for zswap, only for zswap to divide again in > > > order to do limit math. Report pages directly. > > > > Nice. Although I would prefer renaming the zpool interface to > > total_pages and renaming the zpool backends functions as well to use > > pages rather than size. > > Ha, I was on the fence, since it's kind of churny. But if you don't > mind, then it works for me as well. If we are cleaning up, might as well do it all the way :) > > > Either way: > > Acked-by: Yosry Ahmed <yosryahmed@google.com> > > Thanks. LGTM. Feel free to carry the Ack forward.
On 2024/3/12 00:12, Johannes Weiner wrote: > All zswap backends track their pool sizes in pages. Currently they > multiply by PAGE_SIZE for zswap, only for zswap to divide again in > order to do limit math. Report pages directly. > > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org> With the incremental diff, feel free to add: Reviewed-by: Chengming Zhou <chengming.zhou@linux.dev> Thanks. > --- > mm/z3fold.c | 2 +- > mm/zbud.c | 2 +- > mm/zpool.c | 4 ++-- > mm/zsmalloc.c | 2 +- > mm/zswap.c | 4 ++-- > 5 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/mm/z3fold.c b/mm/z3fold.c > index 7ab05621052d..9bacacd4168c 100644 > --- a/mm/z3fold.c > +++ b/mm/z3fold.c > @@ -1404,7 +1404,7 @@ static void z3fold_zpool_unmap(void *pool, unsigned long handle) > > static u64 z3fold_zpool_total_size(void *pool) > { > - return z3fold_get_pool_size(pool) * PAGE_SIZE; > + return z3fold_get_pool_size(pool); > } > > static struct zpool_driver z3fold_zpool_driver = { > diff --git a/mm/zbud.c b/mm/zbud.c > index 2190cc1f37b3..b7d8a22bbf5f 100644 > --- a/mm/zbud.c > +++ b/mm/zbud.c > @@ -412,7 +412,7 @@ static void zbud_zpool_unmap(void *pool, unsigned long handle) > > static u64 zbud_zpool_total_size(void *pool) > { > - return zbud_get_pool_size(pool) * PAGE_SIZE; > + return zbud_get_pool_size(pool); > } > > static struct zpool_driver zbud_zpool_driver = { > diff --git a/mm/zpool.c b/mm/zpool.c > index 846410479c2f..410808aee7fe 100644 > --- a/mm/zpool.c > +++ b/mm/zpool.c > @@ -324,9 +324,9 @@ void zpool_unmap_handle(struct zpool *zpool, unsigned long handle) > * zpool_get_total_size() - The total size of the pool > * @zpool: The zpool to check > * > - * This returns the total size in bytes of the pool. > + * This returns the total size in pages of the pool. > * > - * Returns: Total size of the zpool in bytes. > + * Returns: Total size of the zpool in pages. > */ > u64 zpool_get_total_size(struct zpool *zpool) > { > diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c > index 7d7cb3eaabe0..398f3856817f 100644 > --- a/mm/zsmalloc.c > +++ b/mm/zsmalloc.c > @@ -401,7 +401,7 @@ static void zs_zpool_unmap(void *pool, unsigned long handle) > > static u64 zs_zpool_total_size(void *pool) > { > - return zs_get_total_pages(pool) << PAGE_SHIFT; > + return zs_get_total_pages(pool); > } > > static struct zpool_driver zs_zpool_driver = { > diff --git a/mm/zswap.c b/mm/zswap.c > index 7c39327a7cc2..fe4343e416e0 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -507,7 +507,7 @@ static unsigned long zswap_max_pages(void) > unsigned long zswap_total_pages(void) > { > struct zswap_pool *pool; > - u64 total = 0; > + unsigned long total = 0; > > rcu_read_lock(); > list_for_each_entry_rcu(pool, &zswap_pools, list) { > @@ -518,7 +518,7 @@ unsigned long zswap_total_pages(void) > } > rcu_read_unlock(); > > - return total >> PAGE_SHIFT; > + return total; > } > > /*********************************
On Mon, Mar 11, 2024 at 11:12 PM Johannes Weiner <hannes@cmpxchg.org> wrote: > > All zswap backends track their pool sizes in pages. Currently they > multiply by PAGE_SIZE for zswap, only for zswap to divide again in > order to do limit math. Report pages directly. I've always found this to be weird. Perhaps the original author of this API wants to support more fine-grained memory consumption tracking? > > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org> Anyway, let's just work with realities, rather than hypotheticals :) With the renaming fixlet: Reviewed-by: Nhat Pham <nphamcs@gmail.com>
diff --git a/mm/z3fold.c b/mm/z3fold.c index 7ab05621052d..9bacacd4168c 100644 --- a/mm/z3fold.c +++ b/mm/z3fold.c @@ -1404,7 +1404,7 @@ static void z3fold_zpool_unmap(void *pool, unsigned long handle) static u64 z3fold_zpool_total_size(void *pool) { - return z3fold_get_pool_size(pool) * PAGE_SIZE; + return z3fold_get_pool_size(pool); } static struct zpool_driver z3fold_zpool_driver = { diff --git a/mm/zbud.c b/mm/zbud.c index 2190cc1f37b3..b7d8a22bbf5f 100644 --- a/mm/zbud.c +++ b/mm/zbud.c @@ -412,7 +412,7 @@ static void zbud_zpool_unmap(void *pool, unsigned long handle) static u64 zbud_zpool_total_size(void *pool) { - return zbud_get_pool_size(pool) * PAGE_SIZE; + return zbud_get_pool_size(pool); } static struct zpool_driver zbud_zpool_driver = { diff --git a/mm/zpool.c b/mm/zpool.c index 846410479c2f..410808aee7fe 100644 --- a/mm/zpool.c +++ b/mm/zpool.c @@ -324,9 +324,9 @@ void zpool_unmap_handle(struct zpool *zpool, unsigned long handle) * zpool_get_total_size() - The total size of the pool * @zpool: The zpool to check * - * This returns the total size in bytes of the pool. + * This returns the total size in pages of the pool. * - * Returns: Total size of the zpool in bytes. + * Returns: Total size of the zpool in pages. */ u64 zpool_get_total_size(struct zpool *zpool) { diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c index 7d7cb3eaabe0..398f3856817f 100644 --- a/mm/zsmalloc.c +++ b/mm/zsmalloc.c @@ -401,7 +401,7 @@ static void zs_zpool_unmap(void *pool, unsigned long handle) static u64 zs_zpool_total_size(void *pool) { - return zs_get_total_pages(pool) << PAGE_SHIFT; + return zs_get_total_pages(pool); } static struct zpool_driver zs_zpool_driver = { diff --git a/mm/zswap.c b/mm/zswap.c index 7c39327a7cc2..fe4343e416e0 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -507,7 +507,7 @@ static unsigned long zswap_max_pages(void) unsigned long zswap_total_pages(void) { struct zswap_pool *pool; - u64 total = 0; + unsigned long total = 0; rcu_read_lock(); list_for_each_entry_rcu(pool, &zswap_pools, list) { @@ -518,7 +518,7 @@ unsigned long zswap_total_pages(void) } rcu_read_unlock(); - return total >> PAGE_SHIFT; + return total; } /*********************************
All zswap backends track their pool sizes in pages. Currently they multiply by PAGE_SIZE for zswap, only for zswap to divide again in order to do limit math. Report pages directly. Signed-off-by: Johannes Weiner <hannes@cmpxchg.org> --- mm/z3fold.c | 2 +- mm/zbud.c | 2 +- mm/zpool.c | 4 ++-- mm/zsmalloc.c | 2 +- mm/zswap.c | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-)