From 8a171ccd9258c9528af413562b5bd6b994cf9c2e Mon Sep 17 00:00:00 2001 From: Sebastian Gottschall Date: Wed, 30 Sep 2020 22:22:34 +0200 Subject: [PATCH] do a cyclic seek for unused memory objects in pool In non regular use cases allocated memory might stay persistent in memory pool. This small patch checks every minute if there are old objects which can be released from memory pool. Right now with regular use, the pool is checked for old objects on each allocation attempt from this pool. so basically polling by its use. Now consider what happens if someone writes a lot of files and stops use of the volume or even unmounts it. So the code will no longer check if objects can be released from the pool. Already allocated objects will still stay in pool cache. this is no big issue for common use. But someone discovered this issue while doing tests. personally i know this behavior and I'm aware of it. Its no big issue. just a enhancement Reviewed-by: Brian Behlendorf Reviewed-by: Kjeld Schouten-Lebbing Signed-off-by: Sebastian Gottschall Closes #10938 Closes #10969 --- include/sys/zstd/zstd.h | 1 + module/zfs/arc.c | 10 ++++++++++ module/zstd/zfs_zstd.c | 18 ++++++++++++++++-- 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/include/sys/zstd/zstd.h b/include/sys/zstd/zstd.h index f965df319..e42e44c23 100644 --- a/include/sys/zstd/zstd.h +++ b/include/sys/zstd/zstd.h @@ -90,6 +90,7 @@ int zfs_zstd_decompress_level(void *s_start, void *d_start, size_t s_len, size_t d_len, uint8_t *level); int zfs_zstd_decompress(void *s_start, void *d_start, size_t s_len, size_t d_len, int n); +void zfs_zstd_cache_reap_now(void); #ifdef __cplusplus } diff --git a/module/zfs/arc.c b/module/zfs/arc.c index 1cce068e6..c54d53908 100644 --- a/module/zfs/arc.c +++ b/module/zfs/arc.c @@ -308,6 +308,7 @@ #include #include #include +#include #ifndef _KERNEL /* set with ZFS_DEBUG=watch, to enable watchpoints on frozen buffers */ @@ -4972,6 +4973,7 @@ static boolean_t arc_reap_cb_check(void *arg, zthr_t *zthr) { int64_t free_memory = arc_available_memory(); + static int reap_cb_check_counter = 0; /* * If a kmem reap is already active, don't schedule more. We must @@ -4996,6 +4998,14 @@ arc_reap_cb_check(void *arg, zthr_t *zthr) arc_no_grow = B_FALSE; } + /* + * Called unconditionally every 60 seconds to reclaim unused + * zstd compression and decompression context. This is done + * here to avoid the need for an independent thread. + */ + if (!((reap_cb_check_counter++) % 60)) + zfs_zstd_cache_reap_now(); + return (B_FALSE); } diff --git a/module/zstd/zfs_zstd.c b/module/zstd/zfs_zstd.c index c10b7a84d..34c56b7a7 100644 --- a/module/zstd/zfs_zstd.c +++ b/module/zstd/zfs_zstd.c @@ -238,7 +238,7 @@ zstd_mempool_alloc(struct zstd_pool *zstd_mempool, size_t size) * Check if objects fits the size, if so we take it and * update the timestamp. */ - if (!mem && pool->mem && size <= pool->size) { + if (size && !mem && pool->mem && size <= pool->size) { pool->timeout = gethrestime_sec() + ZSTD_POOL_TIMEOUT; mem = pool->mem; @@ -257,7 +257,7 @@ zstd_mempool_alloc(struct zstd_pool *zstd_mempool, size_t size) } } - if (mem) { + if (!size || mem) { return (mem); } @@ -688,6 +688,19 @@ zstd_mempool_deinit(void) zstd_mempool_cctx = NULL; } +/* release unused memory from pool */ + +void +zfs_zstd_cache_reap_now(void) +{ + /* + * calling alloc with zero size seeks + * and releases old unused objects + */ + zstd_mempool_alloc(zstd_mempool_cctx, 0); + zstd_mempool_alloc(zstd_mempool_dctx, 0); +} + extern int __init zstd_init(void) { @@ -735,4 +748,5 @@ ZFS_MODULE_VERSION(ZSTD_VERSION_STRING); EXPORT_SYMBOL(zfs_zstd_compress); EXPORT_SYMBOL(zfs_zstd_decompress_level); EXPORT_SYMBOL(zfs_zstd_decompress); +EXPORT_SYMBOL(zfs_zstd_cache_reap_now); #endif