linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv2 0/3] Allow ZRAM to use any zpool-compatible backend
@ 2019-12-19 14:19 Vitaly Wool
  2019-12-19 14:21 ` [PATCHv2 1/3] zpool: add compaction api Vitaly Wool
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Vitaly Wool @ 2019-12-19 14:19 UTC (permalink / raw)
  To: Linux-MM, Andrew Morton, Dan Streetman, Minchan Kim
  Cc: Sergey Senozhatsky, LKML, Vlastimil Babka, Shakeel Butt,
	Henry Burns, Theodore Ts'o

The coming patchset is a new take on the old issue: ZRAM can currently be
used only with zsmalloc even though this may not be the optimal
combination for some configurations. The previous (unsuccessful) attempt
dates back to 2015 [1] and is notable for the heated discussions it has
caused.

This patchset addresses the increasing demand to deploy ZRAM in systems
where zsmalloc is not a perfect match or is not applicable at all. An
example of a system of the first type is an embedded system using ZRAM
block device as a swap where quick application launch is critical for
good user experience since z3fold is substantially faster on read than
zsmalloc [2].

A system of the second type would, for instance, be the one with
hardware on-the-fly RAM compression/decompression where the saved RAM
space could be used for ZRAM but would require a special allocator.
 
The preliminary results for this work have been delivered at Linux
Plumbers this year [3]. The talk at LPC ended in a consensus to continue
the work and pursue the goal of decoupling ZRAM from zsmalloc.

The current patchset has been stress tested on arm64 and x86_64 devices,
including the Dell laptop I'm writing this message on now, not to mention
several QEmu confugirations.

The first version of this patchset can be found at [4].
Changelog since V1:
* better formatting
* allocator backend is now configurable on a per-ZRAM device basis
* allocator backend is runtime configurable via sysfs 

[1] https://lkml.org/lkml/2015/9/14/356
[2] https://lkml.org/lkml/2019/10/21/743
[3] https://linuxplumbersconf.org/event/4/contributions/551/
[4] https://lkml.org/lkml/2019/10/10/1046

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCHv2 1/3] zpool: add compaction api
  2019-12-19 14:19 [PATCHv2 0/3] Allow ZRAM to use any zpool-compatible backend Vitaly Wool
@ 2019-12-19 14:21 ` Vitaly Wool
  2019-12-19 14:26 ` [PATCHv2 2/3] zsmalloc: add compaction and huge class callbacks Vitaly Wool
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Vitaly Wool @ 2019-12-19 14:21 UTC (permalink / raw)
  To: Linux-MM, Andrew Morton, Dan Streetman, Minchan Kim
  Cc: Sergey Senozhatsky, LKML, Vlastimil Babka, Shakeel Butt,
	Henry Burns, Theodore Ts'o

This patch adds the following functions to the zpool API:
- zpool_compact()
- zpool_get_num_compacted()
- zpool_huge_class_size()

The first one triggers compaction for the underlying allocator, the
second retrieves the number of pages migrated due to compaction for
the whole time of this pool's existence and the third one returns
the huge class size.

This API extension is done to align zpool API with zsmalloc API.

Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.com>
---
 include/linux/zpool.h | 14 +++++++++++++-
 mm/zpool.c            | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/include/linux/zpool.h b/include/linux/zpool.h
index 51bf43076165..31f0c1360569 100644
--- a/include/linux/zpool.h
+++ b/include/linux/zpool.h
@@ -61,8 +61,13 @@ void *zpool_map_handle(struct zpool *pool, unsigned long handle,
 
 void zpool_unmap_handle(struct zpool *pool, unsigned long handle);
 
+unsigned long zpool_compact(struct zpool *pool);
+
+unsigned long zpool_get_num_compacted(struct zpool *pool);
+
 u64 zpool_get_total_size(struct zpool *pool);
 
+size_t zpool_huge_class_size(struct zpool *zpool);
 
 /**
  * struct zpool_driver - driver implementation for zpool
@@ -75,7 +80,10 @@ u64 zpool_get_total_size(struct zpool *pool);
  * @shrink:	shrink the pool.
  * @map:	map a handle.
  * @unmap:	unmap a handle.
- * @total_size:	get total size of a pool.
+ * @compact:	try to run compaction over a pool
+ * @get_num_compacted:	get amount of compacted pages for a pool
+ * @total_size:	get total size of a pool
+ * @huge_class_size: huge class threshold for pool pages.
  *
  * This is created by a zpool implementation and registered
  * with zpool.
@@ -104,7 +112,11 @@ struct zpool_driver {
 				enum zpool_mapmode mm);
 	void (*unmap)(void *pool, unsigned long handle);
 
+	unsigned long (*compact)(void *pool);
+	unsigned long (*get_num_compacted)(void *pool);
+
 	u64 (*total_size)(void *pool);
+	size_t (*huge_class_size)(void *pool);
 };
 
 void zpool_register_driver(struct zpool_driver *driver);
diff --git a/mm/zpool.c b/mm/zpool.c
index 863669212070..55e69213c2eb 100644
--- a/mm/zpool.c
+++ b/mm/zpool.c
@@ -362,6 +362,30 @@ void zpool_unmap_handle(struct zpool *zpool, unsigned long handle)
 	zpool->driver->unmap(zpool->pool, handle);
 }
 
+ /**
+ * zpool_compact() - try to run compaction over zpool
+ * @pool       The zpool to compact
+ *
+ * Returns: the number of migrated pages
+ */
+unsigned long zpool_compact(struct zpool *zpool)
+{
+	return zpool->driver->compact ? zpool->driver->compact(zpool->pool) : 0;
+}
+
+
+/**
+ * zpool_get_num_compacted() - get the number of migrated/compacted pages
+ * @pool       The zpool to get compaction statistic for
+ *
+ * Returns: the total number of migrated pages for the pool
+ */
+unsigned long zpool_get_num_compacted(struct zpool *zpool)
+{
+	return zpool->driver->get_num_compacted ?
+		zpool->driver->get_num_compacted(zpool->pool) : 0;
+}
+
 /**
  * zpool_get_total_size() - The total size of the pool
  * @zpool:	The zpool to check
@@ -375,6 +399,18 @@ u64 zpool_get_total_size(struct zpool *zpool)
 	return zpool->driver->total_size(zpool->pool);
 }
 
+/**
+ * zpool_huge_class_size() - get size for the "huge" class
+ * @pool	The zpool to check
+ *
+ * Returns: size of the huge class
+ */
+size_t zpool_huge_class_size(struct zpool *zpool)
+{
+	return zpool->driver->huge_class_size ?
+		zpool->driver->huge_class_size(zpool->pool) : 0;
+}
+
 /**
  * zpool_evictable() - Test if zpool is potentially evictable
  * @zpool:	The zpool to test
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCHv2 2/3] zsmalloc: add compaction and huge class callbacks
  2019-12-19 14:19 [PATCHv2 0/3] Allow ZRAM to use any zpool-compatible backend Vitaly Wool
  2019-12-19 14:21 ` [PATCHv2 1/3] zpool: add compaction api Vitaly Wool
@ 2019-12-19 14:26 ` Vitaly Wool
  2019-12-19 14:27 ` [PATCHv2 3/3] zram: use common zpool interface Vitaly Wool
  2019-12-20  3:13 ` [PATCHv2 0/3] Allow ZRAM to use any zpool-compatible backend Minchan Kim
  3 siblings, 0 replies; 6+ messages in thread
From: Vitaly Wool @ 2019-12-19 14:26 UTC (permalink / raw)
  To: Linux-MM, Andrew Morton, Dan Streetman, Minchan Kim
  Cc: Sergey Senozhatsky, LKML, Vlastimil Babka, Shakeel Butt,
	Henry Burns, Theodore Ts'o

Add compaction callbacks for zpool compaction API extension.
Add huge_class_size callback too to be fully aligned.

Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.com>
---
 mm/zsmalloc.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 2b2b9aae8a3c..43f43272b998 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -437,11 +437,29 @@ static void zs_zpool_unmap(void *pool, unsigned long handle)
 	zs_unmap_object(pool, handle);
 }
 
+static unsigned long zs_zpool_compact(void *pool)
+{
+	return zs_compact(pool);
+}
+
+static unsigned long zs_zpool_get_compacted(void *pool)
+{
+	struct zs_pool_stats stats;
+
+	zs_pool_stats(pool, &stats);
+	return stats.pages_compacted;
+}
+
 static u64 zs_zpool_total_size(void *pool)
 {
 	return zs_get_total_pages(pool) << PAGE_SHIFT;
 }
 
+static size_t zs_zpool_huge_class_size(void *pool)
+{
+	return zs_huge_class_size(pool);
+}
+
 static struct zpool_driver zs_zpool_driver = {
 	.type =			  "zsmalloc",
 	.owner =		  THIS_MODULE,
@@ -453,6 +471,9 @@ static struct zpool_driver zs_zpool_driver = {
 	.map =			  zs_zpool_map,
 	.unmap =		  zs_zpool_unmap,
 	.total_size =		  zs_zpool_total_size,
+	.compact =		  zs_zpool_compact,
+	.get_num_compacted =	  zs_zpool_get_compacted,
+	.huge_class_size =	  zs_zpool_huge_class_size,
 };
 
 MODULE_ALIAS("zpool-zsmalloc");
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCHv2 3/3] zram: use common zpool interface
  2019-12-19 14:19 [PATCHv2 0/3] Allow ZRAM to use any zpool-compatible backend Vitaly Wool
  2019-12-19 14:21 ` [PATCHv2 1/3] zpool: add compaction api Vitaly Wool
  2019-12-19 14:26 ` [PATCHv2 2/3] zsmalloc: add compaction and huge class callbacks Vitaly Wool
@ 2019-12-19 14:27 ` Vitaly Wool
  2019-12-20  3:13 ` [PATCHv2 0/3] Allow ZRAM to use any zpool-compatible backend Minchan Kim
  3 siblings, 0 replies; 6+ messages in thread
From: Vitaly Wool @ 2019-12-19 14:27 UTC (permalink / raw)
  To: Linux-MM, Andrew Morton, Dan Streetman, Minchan Kim
  Cc: Sergey Senozhatsky, LKML, Vlastimil Babka, Shakeel Butt,
	Henry Burns, Theodore Ts'o

This patch modifies ZRAM use a common "small allocator" API
(zpool). This is done to address increasing demand to deploy
ZRAM in systems where zsmalloc (which is currently the only
available ZRAM allocator backend) is not a perfect match or is
not applicable at all. An example of a system of the first type
is an embedded system using ZRAM block device as a swap where
quick application launch is critical for good user experience
since z3fold is substantially faster on read than zsmalloc [1].
A system of the second type is, for instance, the one with
hardware on-the-fly RAM compression/decompression where the
saved RAM space could be used for ZRAM but would require a
special allocator. These cases wre also discussed in detail
at LPC'2019 [2].

zpool-registered backend can be selected using ZRAM sysfs
interface on a per-device basis. 'zsmalloc' is taken by default.

This patch is transparent with regard to the existing ZRAM
functionality and does not change its default behavior.

[1] https://lkml.org/lkml/2019/10/21/743
[2] https://linuxplumbersconf.org/event/4/contributions/551/

Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.com>
---
 drivers/block/zram/Kconfig    |   3 +-
 drivers/block/zram/zram_drv.c | 100 ++++++++++++++++++++++++----------
 drivers/block/zram/zram_drv.h |   6 +-
 3 files changed, 76 insertions(+), 33 deletions(-)

diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
index fe7a4b7d30cf..7248d5aa3468 100644
--- a/drivers/block/zram/Kconfig
+++ b/drivers/block/zram/Kconfig
@@ -1,8 +1,9 @@
 # SPDX-License-Identifier: GPL-2.0
 config ZRAM
 	tristate "Compressed RAM block device support"
-	depends on BLOCK && SYSFS && ZSMALLOC && CRYPTO
+	depends on BLOCK && SYSFS && CRYPTO
 	select CRYPTO_LZO
+	select ZPOOL
 	help
 	  Creates virtual block devices called /dev/zramX (X = 0, 1, ...).
 	  Pages written to these disks are compressed and stored in memory
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 4285e75e52c3..189c326cbeee 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -43,6 +43,8 @@ static DEFINE_MUTEX(zram_index_mutex);
 static int zram_major;
 static const char *default_compressor = "lzo-rle";
 
+static const char *default_allocator = "zsmalloc";
+
 /* Module params (documentation at end) */
 static unsigned int num_devices = 1;
 /*
@@ -245,6 +247,14 @@ static ssize_t disksize_show(struct device *dev,
 	return scnprintf(buf, PAGE_SIZE, "%llu\n", zram->disksize);
 }
 
+static ssize_t allocator_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct zram *zram = dev_to_zram(dev);
+
+	return scnprintf(buf, PAGE_SIZE, "%s\n", zram->allocator);
+}
+
 static ssize_t mem_limit_store(struct device *dev,
 		struct device_attribute *attr, const char *buf, size_t len)
 {
@@ -277,7 +287,7 @@ static ssize_t mem_used_max_store(struct device *dev,
 	down_read(&zram->init_lock);
 	if (init_done(zram)) {
 		atomic_long_set(&zram->stats.max_used_pages,
-				zs_get_total_pages(zram->mem_pool));
+			zpool_get_total_size(zram->mem_pool) >> PAGE_SHIFT);
 	}
 	up_read(&zram->init_lock);
 
@@ -1021,7 +1031,7 @@ static ssize_t compact_store(struct device *dev,
 		return -EINVAL;
 	}
 
-	zs_compact(zram->mem_pool);
+	zpool_compact(zram->mem_pool);
 	up_read(&zram->init_lock);
 
 	return len;
@@ -1049,17 +1059,14 @@ static ssize_t mm_stat_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
 	struct zram *zram = dev_to_zram(dev);
-	struct zs_pool_stats pool_stats;
 	u64 orig_size, mem_used = 0;
-	long max_used;
+	long max_used, num_compacted = 0;
 	ssize_t ret;
 
-	memset(&pool_stats, 0x00, sizeof(struct zs_pool_stats));
-
 	down_read(&zram->init_lock);
 	if (init_done(zram)) {
-		mem_used = zs_get_total_pages(zram->mem_pool);
-		zs_pool_stats(zram->mem_pool, &pool_stats);
+		mem_used = zpool_get_total_size(zram->mem_pool);
+		num_compacted = zpool_get_num_compacted(zram->mem_pool);
 	}
 
 	orig_size = atomic64_read(&zram->stats.pages_stored);
@@ -1069,11 +1076,11 @@ static ssize_t mm_stat_show(struct device *dev,
 			"%8llu %8llu %8llu %8lu %8ld %8llu %8lu %8llu\n",
 			orig_size << PAGE_SHIFT,
 			(u64)atomic64_read(&zram->stats.compr_data_size),
-			mem_used << PAGE_SHIFT,
+			mem_used,
 			zram->limit_pages << PAGE_SHIFT,
 			max_used << PAGE_SHIFT,
 			(u64)atomic64_read(&zram->stats.same_pages),
-			pool_stats.pages_compacted,
+			num_compacted,
 			(u64)atomic64_read(&zram->stats.huge_pages));
 	up_read(&zram->init_lock);
 
@@ -1134,7 +1141,7 @@ static void zram_meta_free(struct zram *zram, u64 disksize)
 	for (index = 0; index < num_pages; index++)
 		zram_free_page(zram, index);
 
-	zs_destroy_pool(zram->mem_pool);
+	zpool_destroy_pool(zram->mem_pool);
 	vfree(zram->table);
 }
 
@@ -1147,14 +1154,18 @@ static bool zram_meta_alloc(struct zram *zram, u64 disksize)
 	if (!zram->table)
 		return false;
 
-	zram->mem_pool = zs_create_pool(zram->disk->disk_name);
+	zram->mem_pool = zpool_create_pool(zram->allocator,
+					zram->disk->disk_name,
+					GFP_NOIO, NULL);
 	if (!zram->mem_pool) {
+		pr_err("%s: not enough memory or wrong allocator '%s'\n",
+			__func__, zram->allocator);
 		vfree(zram->table);
 		return false;
 	}
 
 	if (!huge_class_size)
-		huge_class_size = zs_huge_class_size(zram->mem_pool);
+		huge_class_size = zpool_huge_class_size(zram->mem_pool);
 	return true;
 }
 
@@ -1198,7 +1209,7 @@ static void zram_free_page(struct zram *zram, size_t index)
 	if (!handle)
 		return;
 
-	zs_free(zram->mem_pool, handle);
+	zpool_free(zram->mem_pool, handle);
 
 	atomic64_sub(zram_get_obj_size(zram, index),
 			&zram->stats.compr_data_size);
@@ -1247,7 +1258,7 @@ static int __zram_bvec_read(struct zram *zram, struct page *page, u32 index,
 
 	size = zram_get_obj_size(zram, index);
 
-	src = zs_map_object(zram->mem_pool, handle, ZS_MM_RO);
+	src = zpool_map_handle(zram->mem_pool, handle, ZPOOL_MM_RO);
 	if (size == PAGE_SIZE) {
 		dst = kmap_atomic(page);
 		memcpy(dst, src, PAGE_SIZE);
@@ -1261,7 +1272,7 @@ static int __zram_bvec_read(struct zram *zram, struct page *page, u32 index,
 		kunmap_atomic(dst);
 		zcomp_stream_put(zram->comp);
 	}
-	zs_unmap_object(zram->mem_pool, handle);
+	zpool_unmap_handle(zram->mem_pool, handle);
 	zram_slot_unlock(zram, index);
 
 	/* Should NEVER happen. Return bio error if it does. */
@@ -1336,7 +1347,7 @@ static int __zram_bvec_write(struct zram *zram, struct bio_vec *bvec,
 	if (unlikely(ret)) {
 		zcomp_stream_put(zram->comp);
 		pr_err("Compression failed! err=%d\n", ret);
-		zs_free(zram->mem_pool, handle);
+		zpool_free(zram->mem_pool, handle);
 		return ret;
 	}
 
@@ -1355,33 +1366,34 @@ static int __zram_bvec_write(struct zram *zram, struct bio_vec *bvec,
 	 * if we have a 'non-null' handle here then we are coming
 	 * from the slow path and handle has already been allocated.
 	 */
-	if (!handle)
-		handle = zs_malloc(zram->mem_pool, comp_len,
+	if (handle == 0)
+		ret = zpool_malloc(zram->mem_pool, comp_len,
 				__GFP_KSWAPD_RECLAIM |
 				__GFP_NOWARN |
 				__GFP_HIGHMEM |
-				__GFP_MOVABLE);
-	if (!handle) {
+				__GFP_MOVABLE,
+				&handle);
+	if (ret) {
 		zcomp_stream_put(zram->comp);
 		atomic64_inc(&zram->stats.writestall);
-		handle = zs_malloc(zram->mem_pool, comp_len,
-				GFP_NOIO | __GFP_HIGHMEM |
-				__GFP_MOVABLE);
-		if (handle)
+		ret = zpool_malloc(zram->mem_pool, comp_len,
+				GFP_NOIO | __GFP_HIGHMEM | __GFP_MOVABLE,
+				&handle);
+		if (ret == 0)
 			goto compress_again;
 		return -ENOMEM;
 	}
 
-	alloced_pages = zs_get_total_pages(zram->mem_pool);
+	alloced_pages = zpool_get_total_size(zram->mem_pool) >> PAGE_SHIFT;
 	update_used_max(zram, alloced_pages);
 
 	if (zram->limit_pages && alloced_pages > zram->limit_pages) {
 		zcomp_stream_put(zram->comp);
-		zs_free(zram->mem_pool, handle);
+		zpool_free(zram->mem_pool, handle);
 		return -ENOMEM;
 	}
 
-	dst = zs_map_object(zram->mem_pool, handle, ZS_MM_WO);
+	dst = zpool_map_handle(zram->mem_pool, handle, ZPOOL_MM_WO);
 
 	src = zstrm->buffer;
 	if (comp_len == PAGE_SIZE)
@@ -1391,7 +1403,7 @@ static int __zram_bvec_write(struct zram *zram, struct bio_vec *bvec,
 		kunmap_atomic(src);
 
 	zcomp_stream_put(zram->comp);
-	zs_unmap_object(zram->mem_pool, handle);
+	zpool_unmap_handle(zram->mem_pool, handle);
 	atomic64_add(comp_len, &zram->stats.compr_data_size);
 out:
 	/*
@@ -1752,6 +1764,32 @@ static ssize_t disksize_store(struct device *dev,
 	return err;
 }
 
+static ssize_t allocator_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t len)
+{
+	struct zram *zram = dev_to_zram(dev);
+	int ret;
+
+	down_write(&zram->init_lock);
+	if (init_done(zram)) {
+		pr_info("Cannot change disksize for initialized device\n");
+		ret = -EBUSY;
+		goto out;
+	}
+
+	if (len > sizeof(zram->allocator)) {
+		pr_info("Allocator backend name too long\n");
+		ret = -EINVAL;
+		goto out;
+	}
+	strlcpy(zram->allocator, buf, len);
+	ret = len;
+
+out:
+	up_write(&zram->init_lock);
+	return ret;
+}
+
 static ssize_t reset_store(struct device *dev,
 		struct device_attribute *attr, const char *buf, size_t len)
 {
@@ -1834,6 +1872,7 @@ static DEVICE_ATTR_WO(writeback);
 static DEVICE_ATTR_RW(writeback_limit);
 static DEVICE_ATTR_RW(writeback_limit_enable);
 #endif
+static DEVICE_ATTR_RW(allocator);
 
 static struct attribute *zram_disk_attrs[] = {
 	&dev_attr_disksize.attr,
@@ -1857,6 +1896,7 @@ static struct attribute *zram_disk_attrs[] = {
 	&dev_attr_bd_stat.attr,
 #endif
 	&dev_attr_debug_stat.attr,
+	&dev_attr_allocator.attr,
 	NULL,
 };
 
@@ -1954,7 +1994,7 @@ static int zram_add(void)
 	device_add_disk(NULL, zram->disk, zram_disk_attr_groups);
 
 	strlcpy(zram->compressor, default_compressor, sizeof(zram->compressor));
-
+	strlcpy(zram->allocator, default_allocator, sizeof(zram->allocator));
 	zram_debugfs_register(zram);
 	pr_info("Added device: %s\n", zram->disk->disk_name);
 	return device_id;
diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
index f2fd46daa760..8a649a553a7a 100644
--- a/drivers/block/zram/zram_drv.h
+++ b/drivers/block/zram/zram_drv.h
@@ -16,7 +16,7 @@
 #define _ZRAM_DRV_H_
 
 #include <linux/rwsem.h>
-#include <linux/zsmalloc.h>
+#include <linux/zpool.h>
 #include <linux/crypto.h>
 
 #include "zcomp.h"
@@ -28,6 +28,7 @@
 #define ZRAM_SECTOR_PER_LOGICAL_BLOCK	\
 	(1 << (ZRAM_LOGICAL_BLOCK_SHIFT - SECTOR_SHIFT))
 
+#define ZRAM_MAX_ALLOCATOR_NAME	64
 
 /*
  * The lower ZRAM_FLAG_SHIFT bits of table.flags is for
@@ -91,7 +92,7 @@ struct zram_stats {
 
 struct zram {
 	struct zram_table_entry *table;
-	struct zs_pool *mem_pool;
+	struct zpool *mem_pool;
 	struct zcomp *comp;
 	struct gendisk *disk;
 	/* Prevent concurrent execution of device init */
@@ -108,6 +109,7 @@ struct zram {
 	 */
 	u64 disksize;	/* bytes */
 	char compressor[CRYPTO_MAX_ALG_NAME];
+	char allocator[ZRAM_MAX_ALLOCATOR_NAME];
 	/*
 	 * zram is claimed so open request will be failed
 	 */
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCHv2 0/3] Allow ZRAM to use any zpool-compatible backend
  2019-12-19 14:19 [PATCHv2 0/3] Allow ZRAM to use any zpool-compatible backend Vitaly Wool
                   ` (2 preceding siblings ...)
  2019-12-19 14:27 ` [PATCHv2 3/3] zram: use common zpool interface Vitaly Wool
@ 2019-12-20  3:13 ` Minchan Kim
  2019-12-20  6:04   ` Vitaly Wool
  3 siblings, 1 reply; 6+ messages in thread
From: Minchan Kim @ 2019-12-20  3:13 UTC (permalink / raw)
  To: Vitaly Wool
  Cc: Linux-MM, Andrew Morton, Dan Streetman, Sergey Senozhatsky, LKML,
	Vlastimil Babka, Shakeel Butt, Henry Burns, Theodore Ts'o

On Thu, Dec 19, 2019 at 03:19:28PM +0100, Vitaly Wool wrote:
> The coming patchset is a new take on the old issue: ZRAM can currently be
> used only with zsmalloc even though this may not be the optimal
> combination for some configurations. The previous (unsuccessful) attempt
> dates back to 2015 [1] and is notable for the heated discussions it has
> caused.
> 
> This patchset addresses the increasing demand to deploy ZRAM in systems
> where zsmalloc is not a perfect match or is not applicable at all. An
> example of a system of the first type is an embedded system using ZRAM
> block device as a swap where quick application launch is critical for
> good user experience since z3fold is substantially faster on read than
> zsmalloc [2].

Look https://lkml.org/lkml/2019/10/29/1169

z3fold read is 15% faster *only* when when compression ratio is bad below 50%
since zsmalloc involves copy operation if the object is located in the
boundary of discrete pages. It's never popular workload.
Even, once write is involved(write-only, mixed read-write), z3fold is always
slow. Think about that swap-in could cause swap out in the field because devices
are usually under memory pressure. Also, look at the memory usage.
zsmalloc saves bigger memory for all of compression ratios. 

You claims flexibility - some user want fast read. How do you guarantee
it is always fast? It depends on compression ratio. How can admin estimate
runtime data compression ratio in advance?  Their workload is read-only
if they use zram as swap? they are okay to lose write performance and memory
efficiency? Considering that, it's niche. I don't think it's worth to add
maintenance burden here for very limited usecase.

Think about why we replaced xvmalloc with zsmalloc instead of creating wrapper
to keep two allocators and why people push back so hard when we introduced even
zsmalloc. Why zbud was born at the cost of sacrificing memory efficiency was
concern about memory fragmenation of zsmalloc so freeing memory cannot make real
free memory so wanted to manage fragmentation limit(However, we introduced
object migration in zsmalloc afterward so the concern was gone now).

> 
> A system of the second type would, for instance, be the one with
> hardware on-the-fly RAM compression/decompression where the saved RAM
> space could be used for ZRAM but would require a special allocator.

I agree. For the special compressor, we would need other allocator, even
huge zram changes in future for best performance. However, I'm not sure
zpool is good fit for the case. We need discussion when that kinds of
requirments comes up.

Nacked-by: Minchan Kim <minchan@kernel.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv2 0/3] Allow ZRAM to use any zpool-compatible backend
  2019-12-20  3:13 ` [PATCHv2 0/3] Allow ZRAM to use any zpool-compatible backend Minchan Kim
@ 2019-12-20  6:04   ` Vitaly Wool
  0 siblings, 0 replies; 6+ messages in thread
From: Vitaly Wool @ 2019-12-20  6:04 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Linux-MM, Andrew Morton, Dan Streetman, Sergey Senozhatsky, LKML,
	Vlastimil Babka, Shakeel Butt, Henry Burns, Theodore Ts'o

On Fri, Dec 20, 2019 at 4:14 AM Minchan Kim <minchan@kernel.org> wrote:

<snip>
> Look https://lkml.org/lkml/2019/10/29/1169
>
> z3fold read is 15% faster *only* when when compression ratio is bad below 50%
> since zsmalloc involves copy operation if the object is located in the
> boundary of discrete pages. It's never popular workload.
> Even, once write is involved(write-only, mixed read-write), z3fold is always
> slow. Think about that swap-in could cause swap out in the field because devices
> are usually under memory pressure. Also, look at the memory usage.
> zsmalloc saves bigger memory for all of compression ratios.

Yes I remember that. Since your measurements were done on "an x86" without
providing any detail on the actual platform used, they are as good as none.

~Vitaly

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-12-20  6:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-19 14:19 [PATCHv2 0/3] Allow ZRAM to use any zpool-compatible backend Vitaly Wool
2019-12-19 14:21 ` [PATCHv2 1/3] zpool: add compaction api Vitaly Wool
2019-12-19 14:26 ` [PATCHv2 2/3] zsmalloc: add compaction and huge class callbacks Vitaly Wool
2019-12-19 14:27 ` [PATCHv2 3/3] zram: use common zpool interface Vitaly Wool
2019-12-20  3:13 ` [PATCHv2 0/3] Allow ZRAM to use any zpool-compatible backend Minchan Kim
2019-12-20  6:04   ` Vitaly Wool

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).