On Thu, Jan 14, 2021 at 10:29 AM Minchan Kim [off-list ref] wrote:

On Fri, Dec 25, 2020 at 07:02:50PM +0800, Tian Tao wrote:

quoted

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>
---
 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.

I don't think it's a good idea. It just breaks zpool abstraction
in that it exposes internal implementation to user to avoid issue
zswap recently introduced. It also conflicts zpool_map_handle's
semantic.

Rather than introducing another break in zpool due to the new
zswap feature recenlty introduced, zswap could introduce
CONFIG_ZSWAP_HW_COMPRESSOR. Once it's configured, zsmalloc could
be disabled. And with disabling CONFIG_ZSWAP_HW_COMPRESSOR, zswap
doesn't need to make any bounce buffer copy so that no existing
zsmalloc user will see performance regression.

I agree with Minchan. There is no reason to add extra overhead for
configurations where there is no hardware available.