[PATCH mm/zswap 0/2] Fix the compatibility of zsmalloc and zswap

patch #1 add a flag to zpool, then zswap used to determine if zpool drivers such as zbud/z3fold/zsmalloc whether can sleep in atoimc context. patch #2 set flag sleep_mapped to true indicates that zbud/z3fold can sleep in atomic context. zsmalloc didin't support sleep in atomic context, so not set that flag to true. Tian Tao (2): mm/zswap: add the flag can_sleep_mapped mm: set the sleep_mapped to true for zbud and z3fold include/linux/zpool.h | 3 +++ mm/z3fold.c | 1 + mm/zbud.c | 1 + mm/zpool.c | 13 +++++++++++++ mm/zswap.c | 50 +++++++++++++++++++++++++++++++++++++++++++++----- 5 files changed, 63 insertions(+), 5 deletions(-) -- 2.7.4

add a flag to zpool, named is "can_sleep_mapped", and have it set true for zbud/z3fold, set false for zsmalloc. Then zswap could go the current path if the flag is true; and if it's false, copy data from src to a temporary buffer, then unmap the handle, take the mutex, process the buffer instead of src to avoid sleeping function called from atomic context. Signed-off-by: Tian Tao <tiantao6@hisilicon.com> Reviewed-by: Vitaly Wool <vitaly.wool@konsulko.com> Acked-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> --- include/linux/zpool.h | 3 +++ mm/zpool.c | 13 +++++++++++++ mm/zswap.c | 50 +++++++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 61 insertions(+), 5 deletions(-) diff --git a/include/linux/zpool.h b/include/linux/zpool.h index 51bf430..e899701 100644 --- a/include/linux/zpool.h +++ b/include/linux/zpool.h @@ -73,6 +73,7 @@ u64 zpool_get_total_size(struct zpool *pool); * @malloc: allocate mem from a pool. * @free: free mem from a pool. * @shrink: shrink the pool. + * @sleep_mapped: whether zpool driver can sleep during map. * @map: map a handle. * @unmap: unmap a handle. * @total_size: get total size of a pool. @@ -100,6 +101,7 @@ struct zpool_driver { int (*shrink)(void *pool, unsigned int pages, unsigned int *reclaimed); + bool sleep_mapped; void *(*map)(void *pool, unsigned long handle, enum zpool_mapmode mm); void (*unmap)(void *pool, unsigned long handle); @@ -112,5 +114,6 @@ void zpool_register_driver(struct zpool_driver *driver); int zpool_unregister_driver(struct zpool_driver *driver); bool zpool_evictable(struct zpool *pool); +bool zpool_can_sleep_mapped(struct zpool *pool); #endif diff --git a/mm/zpool.c b/mm/zpool.c index 3744a2d..5ed7120 100644 --- a/mm/zpool.c +++ b/mm/zpool.c @@ -23,6 +23,7 @@ struct zpool { void *pool; const struct zpool_ops *ops; bool evictable; + bool can_sleep_mapped; struct list_head list; }; @@ -183,6 +184,7 @@ struct zpool *zpool_create_pool(const char *type, const char *name, gfp_t gfp, zpool->pool = driver->create(name, gfp, ops, zpool); zpool->ops = ops; zpool->evictable = driver->shrink && ops && ops->evict; + zpool->can_sleep_mapped = driver->sleep_mapped; if (!zpool->pool) { pr_err("couldn't create %s pool\n", type); @@ -393,6 +395,17 @@ bool zpool_evictable(struct zpool *zpool) return zpool->evictable; } +/** + * zpool_can_sleep_mapped - Test if zpool can sleep when do mapped. + * @zpool: The zpool to test + * + * Returns: true if zpool can sleep; false otherwise. + */ +bool zpool_can_sleep_mapped(struct zpool *zpool) +{ + return zpool->can_sleep_mapped; +} + MODULE_LICENSE("GPL"); MODULE_AUTHOR("Dan Streetman <ddstreet@ieee.org>"); MODULE_DESCRIPTION("Common API for compressed memory storage"); diff --git a/mm/zswap.c b/mm/zswap.c index 1e41c28..6e0bb61 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -935,13 +935,20 @@ static int zswap_writeback_entry(struct zpool *pool, unsigned long handle) struct scatterlist input, output; struct crypto_acomp_ctx *acomp_ctx; - u8 *src; + u8 *src, *tmp; unsigned int dlen; int ret; struct writeback_control wbc = { .sync_mode = WB_SYNC_NONE, }; + if (!zpool_can_sleep_mapped(pool)) { + + tmp = kmalloc(entry->length, GFP_ATOMIC); + if (!tmp) + return -ENOMEM; + } + /* extract swpentry from data */ zhdr = zpool_map_handle(pool, handle, ZPOOL_MM_RO); swpentry = zhdr->swpentry; /* here */ @@ -979,6 +986,14 @@ static int zswap_writeback_entry(struct zpool *pool, unsigned long handle) dlen = PAGE_SIZE; src = (u8 *)zhdr + sizeof(struct zswap_header); + if (!zpool_can_sleep_mapped(pool)) { + + memcpy(tmp, src, entry->length); + src = tmp; + + zpool_unmap_handle(pool, handle); + } + mutex_lock(acomp_ctx->mutex); sg_init_one(&input, src, entry->length); sg_init_table(&output, 1); @@ -1033,7 +1048,11 @@ static int zswap_writeback_entry(struct zpool *pool, unsigned long handle) spin_unlock(&tree->lock); end: - zpool_unmap_handle(pool, handle); + if (zpool_can_sleep_mapped(pool)) + zpool_unmap_handle(pool, handle); + else + kfree(tmp); + return ret; } @@ -1235,7 +1254,7 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset, struct zswap_entry *entry; struct scatterlist input, output; struct crypto_acomp_ctx *acomp_ctx; - u8 *src, *dst; + u8 *src, *dst, *tmp; unsigned int dlen; int ret; @@ -1256,12 +1275,29 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset, goto freeentry; } + if (!zpool_can_sleep_mapped(entry->pool->zpool)) { + + tmp = kmalloc(entry->length, GFP_ATOMIC); + if (!tmp) { + ret = -ENOMEM; + goto freeentry; + } + } + /* decompress */ dlen = PAGE_SIZE; src = zpool_map_handle(entry->pool->zpool, entry->handle, ZPOOL_MM_RO); if (zpool_evictable(entry->pool->zpool)) src += sizeof(struct zswap_header); + if (!zpool_can_sleep_mapped(entry->pool->zpool)) { + + memcpy(tmp, src, entry->length); + src = tmp; + + zpool_unmap_handle(entry->pool->zpool, entry->handle); + } + acomp_ctx = raw_cpu_ptr(entry->pool->acomp_ctx); mutex_lock(acomp_ctx->mutex); sg_init_one(&input, src, entry->length); @@ -1271,7 +1307,11 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset, ret = crypto_wait_req(crypto_acomp_decompress(acomp_ctx->req), &acomp_ctx->wait); mutex_unlock(acomp_ctx->mutex); - zpool_unmap_handle(entry->pool->zpool, entry->handle); + if (zpool_can_sleep_mapped(entry->pool->zpool)) + zpool_unmap_handle(entry->pool->zpool, entry->handle); + else + kfree(tmp); + BUG_ON(ret); freeentry: @@ -1279,7 +1319,7 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset, zswap_entry_put(tree, entry); spin_unlock(&tree->lock); - return 0; + return ret; } /* frees an entry in zswap */ -- 2.7.4

-----Original Message----- From: tiantao (H) Sent: Tuesday, January 19, 2021 3:09 PM To: Song Bao Hua (Barry Song) <song.bao.hua@hisilicon.com> Cc: linuxarm@openeuler.org Subject: [PATCH mm/zswap 1/2] mm/zswap: add the flag can_sleep_mapped
add a flag to zpool, named is "can_sleep_mapped", and have it set true for zbud/z3fold, set false for zsmalloc. Then zswap could go the current path if the flag is true; and if it's false, copy data from src to a temporary buffer, then unmap the handle, take the mutex, process the buffer instead of src to avoid sleeping function called from atomic context.
Signed-off-by: Tian Tao <tiantao6@hisilicon.com> Reviewed-by: Vitaly Wool <vitaly.wool@konsulko.com> Acked-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Reported-by: Mike Galbraith <efault@gmx.de>
--- include/linux/zpool.h | 3 +++ mm/zpool.c | 13 +++++++++++++ mm/zswap.c | 50 +++++++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 61 insertions(+), 5 deletions(-)
diff --git a/include/linux/zpool.h b/include/linux/zpool.h index 51bf430..e899701 100644 --- a/include/linux/zpool.h +++ b/include/linux/zpool.h @@ -73,6 +73,7 @@ u64 zpool_get_total_size(struct zpool *pool); * @malloc: allocate mem from a pool. * @free: free mem from a pool. * @shrink: shrink the pool. + * @sleep_mapped: whether zpool driver can sleep during map. * @map: map a handle. * @unmap: unmap a handle. * @total_size: get total size of a pool. @@ -100,6 +101,7 @@ struct zpool_driver { int (*shrink)(void *pool, unsigned int pages, unsigned int *reclaimed);
+ bool sleep_mapped; void *(*map)(void *pool, unsigned long handle, enum zpool_mapmode mm); void (*unmap)(void *pool, unsigned long handle); @@ -112,5 +114,6 @@ void zpool_register_driver(struct zpool_driver *driver); int zpool_unregister_driver(struct zpool_driver *driver);
bool zpool_evictable(struct zpool *pool); +bool zpool_can_sleep_mapped(struct zpool *pool);
#endif diff --git a/mm/zpool.c b/mm/zpool.c index 3744a2d..5ed7120 100644 --- a/mm/zpool.c +++ b/mm/zpool.c @@ -23,6 +23,7 @@ struct zpool { void *pool; const struct zpool_ops *ops; bool evictable; + bool can_sleep_mapped;
struct list_head list; }; @@ -183,6 +184,7 @@ struct zpool *zpool_create_pool(const char *type, const char *name, gfp_t gfp, zpool->pool = driver->create(name, gfp, ops, zpool); zpool->ops = ops; zpool->evictable = driver->shrink && ops && ops->evict; + zpool->can_sleep_mapped = driver->sleep_mapped;
if (!zpool->pool) { pr_err("couldn't create %s pool\n", type); @@ -393,6 +395,17 @@ bool zpool_evictable(struct zpool *zpool) return zpool->evictable; }
+/** + * zpool_can_sleep_mapped - Test if zpool can sleep when do mapped. + * @zpool: The zpool to test + * + * Returns: true if zpool can sleep; false otherwise. + */ +bool zpool_can_sleep_mapped(struct zpool *zpool) +{ + return zpool->can_sleep_mapped; +} + MODULE_LICENSE("GPL"); MODULE_AUTHOR("Dan Streetman <ddstreet@ieee.org>"); MODULE_DESCRIPTION("Common API for compressed memory storage"); diff --git a/mm/zswap.c b/mm/zswap.c index 1e41c28..6e0bb61 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -935,13 +935,20 @@ static int zswap_writeback_entry(struct zpool *pool, unsigned long handle) struct scatterlist input, output; struct crypto_acomp_ctx *acomp_ctx;
- u8 *src; + u8 *src, *tmp; unsigned int dlen; int ret; struct writeback_control wbc = { .sync_mode = WB_SYNC_NONE, };
+ if (!zpool_can_sleep_mapped(pool)) { + + tmp = kmalloc(entry->length, GFP_ATOMIC); + if (!tmp) + return -ENOMEM; + } + /* extract swpentry from data */ zhdr = zpool_map_handle(pool, handle, ZPOOL_MM_RO); swpentry = zhdr->swpentry; /* here */ @@ -979,6 +986,14 @@ static int zswap_writeback_entry(struct zpool *pool, unsigned long handle) dlen = PAGE_SIZE; src = (u8 *)zhdr + sizeof(struct zswap_header);
+ if (!zpool_can_sleep_mapped(pool)) { + + memcpy(tmp, src, entry->length); + src = tmp; + + zpool_unmap_handle(pool, handle); + } + mutex_lock(acomp_ctx->mutex); sg_init_one(&input, src, entry->length); sg_init_table(&output, 1); @@ -1033,7 +1048,11 @@ static int zswap_writeback_entry(struct zpool *pool, unsigned long handle) spin_unlock(&tree->lock);
end: - zpool_unmap_handle(pool, handle); + if (zpool_can_sleep_mapped(pool)) + zpool_unmap_handle(pool, handle); + else + kfree(tmp); + return ret; }
@@ -1235,7 +1254,7 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset, struct zswap_entry *entry; struct scatterlist input, output; struct crypto_acomp_ctx *acomp_ctx; - u8 *src, *dst; + u8 *src, *dst, *tmp; unsigned int dlen; int ret;
@@ -1256,12 +1275,29 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset, goto freeentry; }
+ if (!zpool_can_sleep_mapped(entry->pool->zpool)) { + + tmp = kmalloc(entry->length, GFP_ATOMIC); + if (!tmp) { + ret = -ENOMEM; + goto freeentry; + } + } + /* decompress */ dlen = PAGE_SIZE; src = zpool_map_handle(entry->pool->zpool, entry->handle, ZPOOL_MM_RO); if (zpool_evictable(entry->pool->zpool)) src += sizeof(struct zswap_header);
+ if (!zpool_can_sleep_mapped(entry->pool->zpool)) { + + memcpy(tmp, src, entry->length); + src = tmp; + + zpool_unmap_handle(entry->pool->zpool, entry->handle); + } + acomp_ctx = raw_cpu_ptr(entry->pool->acomp_ctx); mutex_lock(acomp_ctx->mutex); sg_init_one(&input, src, entry->length); @@ -1271,7 +1307,11 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset, ret = crypto_wait_req(crypto_acomp_decompress(acomp_ctx->req), &acomp_ctx->wait); mutex_unlock(acomp_ctx->mutex);
- zpool_unmap_handle(entry->pool->zpool, entry->handle); + if (zpool_can_sleep_mapped(entry->pool->zpool)) + zpool_unmap_handle(entry->pool->zpool, entry->handle); + else + kfree(tmp); + BUG_ON(ret);
freeentry: @@ -1279,7 +1319,7 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset, zswap_entry_put(tree, entry); spin_unlock(&tree->lock);
- return 0; + return ret; }
/* frees an entry in zswap */ -- 2.7.4

zpool driver add a flag to indicates whether the zpool driver can sleep while mapp, this patch set it true for z3fold and zbud. Signed-off-by: Tian Tao <tiantao6@hisilicon.com> Reviewed-by: Vitaly Wool <vitaly.wool@konsulko.com> Acked-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> --- mm/z3fold.c | 1 + mm/zbud.c | 1 + 2 files changed, 2 insertions(+) diff --git a/mm/z3fold.c b/mm/z3fold.c index dacb0d7..234b46f 100644 --- a/mm/z3fold.c +++ b/mm/z3fold.c @@ -1778,6 +1778,7 @@ static u64 z3fold_zpool_total_size(void *pool) static struct zpool_driver z3fold_zpool_driver = { .type = "z3fold", + .sleep_mapped = true, .owner = THIS_MODULE, .create = z3fold_zpool_create, .destroy = z3fold_zpool_destroy, diff --git a/mm/zbud.c b/mm/zbud.c index c49966e..7ec5f27 100644 --- a/mm/zbud.c +++ b/mm/zbud.c @@ -203,6 +203,7 @@ static u64 zbud_zpool_total_size(void *pool) static struct zpool_driver zbud_zpool_driver = { .type = "zbud", + .sleep_mapped = true, .owner = THIS_MODULE, .create = zbud_zpool_create, .destroy = zbud_zpool_destroy, -- 2.7.4

-----Original Message----- From: tiantao (H) Sent: Tuesday, January 19, 2021 3:09 PM To: Song Bao Hua (Barry Song) <song.bao.hua@hisilicon.com> Cc: linuxarm@openeuler.org Subject: [PATCH mm/zswap 0/2] Fix the compatibility of zsmalloc and zswap
patch #1 add a flag to zpool, then zswap used to determine if zpool drivers such as zbud/z3fold/zsmalloc whether can sleep in atoimc context. patch #2 set flag sleep_mapped to true indicates that zbud/z3fold can sleep in atomic context. zsmalloc didin't support sleep in atomic context, so not set that flag to true.
I guess the description is not accurate as nobody can sleep in atomic context. The difference is that zbud/z3fold don't require an atomic context but zsmalloc requires that since its map function holds a preempt-disabled lock.
Tian Tao (2): mm/zswap: add the flag can_sleep_mapped mm: set the sleep_mapped to true for zbud and z3fold
include/linux/zpool.h | 3 +++ mm/z3fold.c | 1 + mm/zbud.c | 1 + mm/zpool.c | 13 +++++++++++++ mm/zswap.c | 50 +++++++++++++++++++++++++++++++++++++++++++++----- 5 files changed, 63 insertions(+), 5 deletions(-)
-- 2.7.4
Thanks Barry
participants (2)
-
Song Bao Hua (Barry Song)
-
Tian Tao