From 3e7e19e028fdc300e9d97000be135e7fff2b0c64 Mon Sep 17 00:00:00 2001 From: Rob Norris Date: Wed, 1 Oct 2025 14:08:52 +1000 Subject: [PATCH] pool_iter_refresh: don't refresh pools twice In "all pools" mode, pool_iter_refresh() will call zpool_iter(), which will call zpool_refresh_stats() before calling add_pool(). If we already have the pool, this is a different handle, so we just release it and return. Back in pool_iter_refresh(), we then call zpool_stats_refresh() again for our handle on the same pool. All together, this means we're doing two ZFS_IOC_POOL_STATS calls into the kernel for every pool in the system. This isn't wrong, but it does double the pressure on global locks. Instead, we add a new function zpool_refresh_stats_from_handle() that simply copies the pool config and state from one handle to another, and use it to update our handle before we release it in add_pool(), so we only have one call per pool per interval. Sponsored-by: Klara, Inc. Sponsored-by: Wasabi Technology, Inc. Reviewed-by: Tony Hutter Reviewed-by: Brian Behlendorf Signed-off-by: Rob Norris Closes #17807 --- cmd/zpool/zpool_iter.c | 2 + include/libzfs.h | 2 + lib/libuutil/libuutil.abi | 105 +------------------ lib/libzfs/libzfs.abi | 172 +++++++------------------------- lib/libzfs/libzfs_config.c | 17 ++++ lib/libzfs_core/libzfs_core.abi | 105 +------------------ 6 files changed, 61 insertions(+), 342 deletions(-) diff --git a/cmd/zpool/zpool_iter.c b/cmd/zpool/zpool_iter.c index ae8d00eeb..fef602736 100644 --- a/cmd/zpool/zpool_iter.c +++ b/cmd/zpool/zpool_iter.c @@ -103,6 +103,8 @@ add_pool(zpool_handle_t *zhp, zpool_list_t *zlp) new->zn_last_refresh = zlp->zl_last_refresh; uu_avl_insert(zlp->zl_avl, new, idx); } else { + zpool_refresh_stats_from_handle(node->zn_handle, zhp); + node->zn_last_refresh = zlp->zl_last_refresh; zpool_close(zhp); free(new); return (-1); diff --git a/include/libzfs.h b/include/libzfs.h index 3fcdc176a..14930fb90 100644 --- a/include/libzfs.h +++ b/include/libzfs.h @@ -479,6 +479,8 @@ _LIBZFS_H zpool_status_t zpool_import_status(nvlist_t *, const char **, _LIBZFS_H nvlist_t *zpool_get_config(zpool_handle_t *, nvlist_t **); _LIBZFS_H nvlist_t *zpool_get_features(zpool_handle_t *); _LIBZFS_H int zpool_refresh_stats(zpool_handle_t *, boolean_t *); +_LIBZFS_H void zpool_refresh_stats_from_handle(zpool_handle_t *, + zpool_handle_t *); _LIBZFS_H int zpool_get_errlog(zpool_handle_t *, nvlist_t **); _LIBZFS_H void zpool_add_propname(zpool_handle_t *, const char *); diff --git a/lib/libuutil/libuutil.abi b/lib/libuutil/libuutil.abi index 6c736c61e..2a740afa0 100644 --- a/lib/libuutil/libuutil.abi +++ b/lib/libuutil/libuutil.abi @@ -616,6 +616,7 @@ + @@ -1020,13 +1021,6 @@ - - - - - - - @@ -1061,93 +1055,6 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -1237,8 +1144,6 @@ - - @@ -1254,14 +1159,6 @@ - - - - - - - - diff --git a/lib/libzfs/libzfs.abi b/lib/libzfs/libzfs.abi index 184ea4a55..f988d27a2 100644 --- a/lib/libzfs/libzfs.abi +++ b/lib/libzfs/libzfs.abi @@ -571,6 +571,7 @@ + @@ -641,7 +642,7 @@ - + @@ -1458,103 +1459,8 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -1566,14 +1472,6 @@ - - - - - - - - @@ -3194,6 +3092,10 @@ + + + + @@ -3238,6 +3140,11 @@ + + + + + @@ -9398,10 +9305,6 @@ - - - - @@ -9774,8 +9677,8 @@ - - + + @@ -9805,30 +9708,31 @@ - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/lib/libzfs/libzfs_config.c b/lib/libzfs/libzfs_config.c index 0d2102191..9d704e430 100644 --- a/lib/libzfs/libzfs_config.c +++ b/lib/libzfs/libzfs_config.c @@ -307,6 +307,23 @@ zpool_refresh_stats(zpool_handle_t *zhp, boolean_t *missing) return (0); } +/* + * Copies the pool config and state from szhp to dzhp. szhp and dzhp must + * represent the same pool. Used by pool_list_refresh() to avoid another + * round-trip into the kernel to get stats already collected earlier in the + * function. + */ +void +zpool_refresh_stats_from_handle(zpool_handle_t *dzhp, zpool_handle_t *szhp) +{ + VERIFY0(strcmp(dzhp->zpool_name, szhp->zpool_name)); + nvlist_free(dzhp->zpool_old_config); + dzhp->zpool_old_config = dzhp->zpool_config; + dzhp->zpool_config = fnvlist_dup(szhp->zpool_config); + dzhp->zpool_config_size = szhp->zpool_config_size; + dzhp->zpool_state = szhp->zpool_state; +} + /* * The following environment variables are undocumented * and should be used for testing purposes only: diff --git a/lib/libzfs_core/libzfs_core.abi b/lib/libzfs_core/libzfs_core.abi index 7464b3adb..263cad045 100644 --- a/lib/libzfs_core/libzfs_core.abi +++ b/lib/libzfs_core/libzfs_core.abi @@ -617,6 +617,7 @@ + @@ -988,13 +989,6 @@ - - - - - - - @@ -1029,93 +1023,6 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -1191,8 +1098,6 @@ - - @@ -1208,14 +1113,6 @@ - - - - - - - -