linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH -next v4 0/6] Delay the initializaton of zswap
@ 2022-08-29 13:22 Liu Shixin
  2022-08-29 13:22 ` [PATCH -next v4 1/6] Revert "frontswap: remove support for multiple ops" Liu Shixin
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Liu Shixin @ 2022-08-29 13:22 UTC (permalink / raw)
  To: Seth Jennings, Dan Streetman, Vitaly Wool, Andrew Morton,
	Nathan Chancellor, Christoph Hellwig
  Cc: linux-mm, linux-kernel, Liu Shixin, Kefeng Wang

In the initialization of zswap, about 18MB memory will be allocated for       
zswap_pool. Since not all users use zswap, the memory may be wasted. Save  
the memory for these users by delaying the initialization of zswap to         
first enablement.                                                             

v3->v4: Revert three patches to replace previous patch[1-2] suggested by
Vitaly.
v2->v3: Fix frontswap_ops NULL reported by Nathan and add init for online
swap device in backend register.
v1->v2: Change init_zswap to zswap_init suggested by Andrew.

Liu Shixin (6):
  Revert "frontswap: remove support for multiple ops"
  Revert "mm: mark swap_lock and swap_active_head static"
  Revert "frontswap: simplify frontswap_register_ops"
  mm/zswap: replace zswap_init_{started/failed} with zswap_init_state
  mm/zswap: delay the initializaton of zswap until the first enablement
  mm/zswap: skip confusing print info

 include/linux/frontswap.h |  3 +-
 include/linux/swapfile.h  |  2 +
 mm/frontswap.c            | 91 +++++++++++++++++++++++++++++++++------
 mm/swapfile.c             |  4 +-
 mm/zswap.c                | 85 +++++++++++++++++++++++++-----------
 5 files changed, 145 insertions(+), 40 deletions(-)

-- 
2.25.1



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

* [PATCH -next v4 1/6] Revert "frontswap: remove support for multiple ops"
  2022-08-29 13:22 [PATCH -next v4 0/6] Delay the initializaton of zswap Liu Shixin
@ 2022-08-29 13:22 ` Liu Shixin
  2022-08-30  5:05   ` Christoph Hellwig
  2022-08-29 13:22 ` [PATCH -next v4 2/6] Revert "mm: mark swap_lock and swap_active_head static" Liu Shixin
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 7+ messages in thread
From: Liu Shixin @ 2022-08-29 13:22 UTC (permalink / raw)
  To: Seth Jennings, Dan Streetman, Vitaly Wool, Andrew Morton,
	Nathan Chancellor, Christoph Hellwig
  Cc: linux-mm, linux-kernel, Liu Shixin, Kefeng Wang

This reverts commit 1da0d94a3ec8c5f3793b7be8538b55e60ebeefe3.

If zswap initial failed or has not been initial, frontswap_ops will be
NULL. In such situation, swap device would enable failed with following
stack trace:

  Unable to handle kernel access to user memory outside uaccess routines at virtual address 0000000000000000
  Mem abort info:
    ESR = 0x0000000096000004
    EC = 0x25: DABT (current EL), IL = 32 bits
    SET = 0, FnV = 0
    EA = 0, S1PTW = 0
    FSC = 0x04: level 0 translation fault
  Data abort info:
    ISV = 0, ISS = 0x00000004
    CM = 0, WnR = 0
  user pgtable: 4k pages, 48-bit VAs, pgdp=00000020a4fab000
  [0000000000000000] pgd=0000000000000000, p4d=0000000000000000
  Internal error: Oops: 96000004 [#1] SMP
  Modules linked in: zram fsl_dpaa2_eth pcs_lynx phylink ahci_qoriq crct10dif_ce ghash_ce sbsa_gwdt fsl_mc_dpio nvme lm90 nvme_core at803x xhci_plat_hcd rtc_fsl_ftm_alarm xgmac_mdio ahci_platform i2c_imx ip6_tables ip_tables fuse
  Unloaded tainted modules: cppc_cpufreq():1
  CPU: 10 PID: 761 Comm: swapon Not tainted 6.0.0-rc2-00454-g22100432cf14 #1
  Hardware name: SolidRun Ltd. SolidRun CEX7 Platform, BIOS EDK II Jun 21 2022
  pstate: 00400005 (nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
  pc : frontswap_init+0x38/0x60
  lr : __do_sys_swapon+0x8a8/0x9f4
  sp : ffff80000969bcf0
  x29: ffff80000969bcf0 x28: ffff37bee0d8fc00 x27: ffff80000a7f5000
  x26: fffffcdefb971e80 x25: ffffaba797453b90 x24: 0000000000000064
  x23: ffff37c1f209d1a8 x22: ffff37bee880e000 x21: ffffaba797748560
  x20: ffff37bee0d8fce4 x19: ffffaba797748488 x18: 0000000000000014
  x17: 0000000030ec029a x16: ffffaba795a479b0 x15: 0000000000000000
  x14: 0000000000000000 x13: 0000000000000030 x12: 0000000000000001
  x11: ffff37c63c0aba18 x10: 0000000000000000 x9 : ffffaba7956b8c88
  x8 : ffff80000969bcd0 x7 : 0000000000000000 x6 : 0000000000000000
  x5 : 0000000000000001 x4 : 0000000000000000 x3 : ffffaba79730f000
  x2 : ffff37bee0d8fc00 x1 : 0000000000000000 x0 : 0000000000000000
  Call trace:
  frontswap_init+0x38/0x60
  __do_sys_swapon+0x8a8/0x9f4
  __arm64_sys_swapon+0x28/0x3c
  invoke_syscall+0x78/0x100
  el0_svc_common.constprop.0+0xd4/0xf4
  do_el0_svc+0x38/0x4c
  el0_svc+0x34/0x10c
  el0t_64_sync_handler+0x11c/0x150
  el0t_64_sync+0x190/0x194
  Code: d000e283 910003fd f9006c41 f946d461 (f9400021)
  ---[ end trace 0000000000000000 ]---

Reported-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
 include/linux/frontswap.h |  3 ++-
 mm/frontswap.c            | 50 +++++++++++++++++++++++++++++----------
 mm/zswap.c                |  8 ++-----
 3 files changed, 42 insertions(+), 19 deletions(-)

diff --git a/include/linux/frontswap.h b/include/linux/frontswap.h
index a631bac12220..c5b2848d2240 100644
--- a/include/linux/frontswap.h
+++ b/include/linux/frontswap.h
@@ -13,9 +13,10 @@ struct frontswap_ops {
 	int (*load)(unsigned, pgoff_t, struct page *); /* load a page */
 	void (*invalidate_page)(unsigned, pgoff_t); /* page no longer needed */
 	void (*invalidate_area)(unsigned); /* swap type just swapoff'ed */
+	struct frontswap_ops *next; /* private pointer to next ops */
 };
 
-int frontswap_register_ops(const struct frontswap_ops *ops);
+extern void frontswap_register_ops(struct frontswap_ops *ops);
 
 extern void frontswap_init(unsigned type, unsigned long *map);
 extern int __frontswap_store(struct page *page);
diff --git a/mm/frontswap.c b/mm/frontswap.c
index 1a97610308cb..8d644f56a1d2 100644
--- a/mm/frontswap.c
+++ b/mm/frontswap.c
@@ -27,7 +27,10 @@ DEFINE_STATIC_KEY_FALSE(frontswap_enabled_key);
  * may be registered, but implementations can never deregister.  This
  * is a simple singly-linked list of all registered implementations.
  */
-static const struct frontswap_ops *frontswap_ops __read_mostly;
+static struct frontswap_ops *frontswap_ops __read_mostly;
+
+#define for_each_frontswap_ops(ops)		\
+	for ((ops) = frontswap_ops; (ops); (ops) = (ops)->next)
 
 #ifdef CONFIG_DEBUG_FS
 /*
@@ -94,14 +97,18 @@ static inline void inc_frontswap_invalidates(void) { }
 /*
  * Register operations for frontswap
  */
-int frontswap_register_ops(const struct frontswap_ops *ops)
+void frontswap_register_ops(struct frontswap_ops *ops)
 {
-	if (frontswap_ops)
-		return -EINVAL;
+	/*
+	 * Setting frontswap_ops must happen after the ops->init() calls
+	 * above; cmpxchg implies smp_mb() which will ensure the init is
+	 * complete at this point.
+	 */
+	do {
+		ops->next = frontswap_ops;
+	} while (cmpxchg(&frontswap_ops, ops->next, ops) != ops->next);
 
-	frontswap_ops = ops;
 	static_branch_inc(&frontswap_enabled_key);
-	return 0;
 }
 
 /*
@@ -110,6 +117,7 @@ int frontswap_register_ops(const struct frontswap_ops *ops)
 void frontswap_init(unsigned type, unsigned long *map)
 {
 	struct swap_info_struct *sis = swap_info[type];
+	struct frontswap_ops *ops;
 
 	VM_BUG_ON(sis == NULL);
 
@@ -125,7 +133,9 @@ void frontswap_init(unsigned type, unsigned long *map)
 	 * p->frontswap set to something valid to work properly.
 	 */
 	frontswap_map_set(sis, map);
-	frontswap_ops->init(type);
+
+	for_each_frontswap_ops(ops)
+		ops->init(type);
 }
 
 static bool __frontswap_test(struct swap_info_struct *sis,
@@ -164,6 +174,7 @@ int __frontswap_store(struct page *page)
 	int type = swp_type(entry);
 	struct swap_info_struct *sis = swap_info[type];
 	pgoff_t offset = swp_offset(entry);
+	struct frontswap_ops *ops;
 
 	VM_BUG_ON(!frontswap_ops);
 	VM_BUG_ON(!PageLocked(page));
@@ -177,10 +188,16 @@ int __frontswap_store(struct page *page)
 	 */
 	if (__frontswap_test(sis, offset)) {
 		__frontswap_clear(sis, offset);
-		frontswap_ops->invalidate_page(type, offset);
+		for_each_frontswap_ops(ops)
+			ops->invalidate_page(type, offset);
 	}
 
-	ret = frontswap_ops->store(type, offset, page);
+	/* Try to store in each implementation, until one succeeds. */
+	for_each_frontswap_ops(ops) {
+		ret = ops->store(type, offset, page);
+		if (!ret) /* successful store */
+			break;
+	}
 	if (ret == 0) {
 		__frontswap_set(sis, offset);
 		inc_frontswap_succ_stores();
@@ -203,6 +220,7 @@ int __frontswap_load(struct page *page)
 	int type = swp_type(entry);
 	struct swap_info_struct *sis = swap_info[type];
 	pgoff_t offset = swp_offset(entry);
+	struct frontswap_ops *ops;
 
 	VM_BUG_ON(!frontswap_ops);
 	VM_BUG_ON(!PageLocked(page));
@@ -212,7 +230,11 @@ int __frontswap_load(struct page *page)
 		return -1;
 
 	/* Try loading from each implementation, until one succeeds. */
-	ret = frontswap_ops->load(type, offset, page);
+	for_each_frontswap_ops(ops) {
+		ret = ops->load(type, offset, page);
+		if (!ret) /* successful load */
+			break;
+	}
 	if (ret == 0)
 		inc_frontswap_loads();
 	return ret;
@@ -225,6 +247,7 @@ int __frontswap_load(struct page *page)
 void __frontswap_invalidate_page(unsigned type, pgoff_t offset)
 {
 	struct swap_info_struct *sis = swap_info[type];
+	struct frontswap_ops *ops;
 
 	VM_BUG_ON(!frontswap_ops);
 	VM_BUG_ON(sis == NULL);
@@ -232,7 +255,8 @@ void __frontswap_invalidate_page(unsigned type, pgoff_t offset)
 	if (!__frontswap_test(sis, offset))
 		return;
 
-	frontswap_ops->invalidate_page(type, offset);
+	for_each_frontswap_ops(ops)
+		ops->invalidate_page(type, offset);
 	__frontswap_clear(sis, offset);
 	inc_frontswap_invalidates();
 }
@@ -244,6 +268,7 @@ void __frontswap_invalidate_page(unsigned type, pgoff_t offset)
 void __frontswap_invalidate_area(unsigned type)
 {
 	struct swap_info_struct *sis = swap_info[type];
+	struct frontswap_ops *ops;
 
 	VM_BUG_ON(!frontswap_ops);
 	VM_BUG_ON(sis == NULL);
@@ -251,7 +276,8 @@ void __frontswap_invalidate_area(unsigned type)
 	if (sis->frontswap_map == NULL)
 		return;
 
-	frontswap_ops->invalidate_area(type);
+	for_each_frontswap_ops(ops)
+		ops->invalidate_area(type);
 	atomic_set(&sis->frontswap_pages, 0);
 	bitmap_zero(sis->frontswap_map, sis->max);
 }
diff --git a/mm/zswap.c b/mm/zswap.c
index 2d48fd59cc7a..c22208df292a 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -1415,7 +1415,7 @@ static void zswap_frontswap_init(unsigned type)
 	zswap_trees[type] = tree;
 }
 
-static const struct frontswap_ops zswap_frontswap_ops = {
+static struct frontswap_ops zswap_frontswap_ops = {
 	.store = zswap_frontswap_store,
 	.load = zswap_frontswap_load,
 	.invalidate_page = zswap_frontswap_invalidate_page,
@@ -1512,15 +1512,11 @@ static int __init init_zswap(void)
 	if (!shrink_wq)
 		goto fallback_fail;
 
-	ret = frontswap_register_ops(&zswap_frontswap_ops);
-	if (ret)
-		goto destroy_wq;
+	frontswap_register_ops(&zswap_frontswap_ops);
 	if (zswap_debugfs_init())
 		pr_warn("debugfs initialization failed\n");
 	return 0;
 
-destroy_wq:
-	destroy_workqueue(shrink_wq);
 fallback_fail:
 	if (pool)
 		zswap_pool_destroy(pool);
-- 
2.25.1



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

* [PATCH -next v4 2/6] Revert "mm: mark swap_lock and swap_active_head static"
  2022-08-29 13:22 [PATCH -next v4 0/6] Delay the initializaton of zswap Liu Shixin
  2022-08-29 13:22 ` [PATCH -next v4 1/6] Revert "frontswap: remove support for multiple ops" Liu Shixin
@ 2022-08-29 13:22 ` Liu Shixin
  2022-08-29 13:22 ` [PATCH -next v4 3/6] Revert "frontswap: simplify frontswap_register_ops" Liu Shixin
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Liu Shixin @ 2022-08-29 13:22 UTC (permalink / raw)
  To: Seth Jennings, Dan Streetman, Vitaly Wool, Andrew Morton,
	Nathan Chancellor, Christoph Hellwig
  Cc: linux-mm, linux-kernel, Liu Shixin, Kefeng Wang

This reverts commit 633423a09cb5cfe61438283e1ce49c23cf4a0611.

swap_lock and swap_active_head will be used in next patch, so export it
again.

Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
 include/linux/swapfile.h | 2 ++
 mm/swapfile.c            | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/swapfile.h b/include/linux/swapfile.h
index 2fbcc9afd814..75fc069594a5 100644
--- a/include/linux/swapfile.h
+++ b/include/linux/swapfile.h
@@ -6,6 +6,8 @@
  * these were static in swapfile.c but frontswap.c needs them and we don't
  * want to expose them to the dozens of source files that include swap.h
  */
+extern spinlock_t swap_lock;
+extern struct plist_head swap_active_head;
 extern struct swap_info_struct *swap_info[];
 extern unsigned long generic_max_swapfile_size(void);
 /* Maximum swapfile size supported for the arch (not inclusive). */
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 469d9af86be2..d383b282f269 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -51,7 +51,7 @@ static bool swap_count_continued(struct swap_info_struct *, pgoff_t,
 				 unsigned char);
 static void free_swap_count_continuations(struct swap_info_struct *);
 
-static DEFINE_SPINLOCK(swap_lock);
+DEFINE_SPINLOCK(swap_lock);
 static unsigned int nr_swapfiles;
 atomic_long_t nr_swap_pages;
 /*
@@ -77,7 +77,7 @@ static const char Unused_offset[] = "Unused swap offset entry ";
  * all active swap_info_structs
  * protected with swap_lock, and ordered by priority.
  */
-static PLIST_HEAD(swap_active_head);
+PLIST_HEAD(swap_active_head);
 
 /*
  * all available (active, not full) swap_info_structs
-- 
2.25.1



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

* [PATCH -next v4 3/6] Revert "frontswap: simplify frontswap_register_ops"
  2022-08-29 13:22 [PATCH -next v4 0/6] Delay the initializaton of zswap Liu Shixin
  2022-08-29 13:22 ` [PATCH -next v4 1/6] Revert "frontswap: remove support for multiple ops" Liu Shixin
  2022-08-29 13:22 ` [PATCH -next v4 2/6] Revert "mm: mark swap_lock and swap_active_head static" Liu Shixin
@ 2022-08-29 13:22 ` Liu Shixin
  2022-08-29 13:23 ` [PATCH -next v4 4/6] mm/zswap: replace zswap_init_{started/failed} with zswap_init_state Liu Shixin
  2022-08-29 13:23 ` [PATCH -next v4 5/6] mm/zswap: delay the initializaton of zswap until the first enablement Liu Shixin
  4 siblings, 0 replies; 7+ messages in thread
From: Liu Shixin @ 2022-08-29 13:22 UTC (permalink / raw)
  To: Seth Jennings, Dan Streetman, Vitaly Wool, Andrew Morton,
	Nathan Chancellor, Christoph Hellwig
  Cc: linux-mm, linux-kernel, Liu Shixin, Kefeng Wang

This reverts commit f328c1d16e4c764992895ac9c9425cea861b2ca0.

Since we are supported to delay zswap initializaton, we need to invoke
ops->init for the swap device which is already online when register
backend.

Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
 mm/frontswap.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/mm/frontswap.c b/mm/frontswap.c
index 8d644f56a1d2..555e78f9529d 100644
--- a/mm/frontswap.c
+++ b/mm/frontswap.c
@@ -99,6 +99,25 @@ static inline void inc_frontswap_invalidates(void) { }
  */
 void frontswap_register_ops(struct frontswap_ops *ops)
 {
+	DECLARE_BITMAP(a, MAX_SWAPFILES);
+	DECLARE_BITMAP(b, MAX_SWAPFILES);
+	struct swap_info_struct *si;
+	unsigned int i;
+
+	bitmap_zero(a, MAX_SWAPFILES);
+	bitmap_zero(b, MAX_SWAPFILES);
+
+	spin_lock(&swap_lock);
+	plist_for_each_entry(si, &swap_active_head, list) {
+		if (!WARN_ON(!si->frontswap_map))
+			__set_bit(si->type, a);
+	}
+	spin_unlock(&swap_lock);
+
+	/* the new ops needs to know the currently active swap devices */
+	for_each_set_bit(i, a, MAX_SWAPFILES)
+		ops->init(i);
+
 	/*
 	 * Setting frontswap_ops must happen after the ops->init() calls
 	 * above; cmpxchg implies smp_mb() which will ensure the init is
@@ -109,6 +128,28 @@ void frontswap_register_ops(struct frontswap_ops *ops)
 	} while (cmpxchg(&frontswap_ops, ops->next, ops) != ops->next);
 
 	static_branch_inc(&frontswap_enabled_key);
+
+	spin_lock(&swap_lock);
+	plist_for_each_entry(si, &swap_active_head, list) {
+		if (si->frontswap_map)
+			__set_bit(si->type, b);
+	}
+	spin_unlock(&swap_lock);
+
+	/*
+	 * On the very unlikely chance that a swap device was added or
+	 * removed between setting the "a" list bits and the ops init
+	 * calls, we re-check and do init or invalidate for any changed
+	 * bits.
+	 */
+	if (unlikely(!bitmap_equal(a, b, MAX_SWAPFILES))) {
+		for (i = 0; i < MAX_SWAPFILES; i++) {
+			if (!test_bit(i, a) && test_bit(i, b))
+				ops->init(i);
+			else if (test_bit(i, a) && !test_bit(i, b))
+				ops->invalidate_area(i);
+		}
+	}
 }
 
 /*
-- 
2.25.1



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

* [PATCH -next v4 4/6] mm/zswap: replace zswap_init_{started/failed} with zswap_init_state
  2022-08-29 13:22 [PATCH -next v4 0/6] Delay the initializaton of zswap Liu Shixin
                   ` (2 preceding siblings ...)
  2022-08-29 13:22 ` [PATCH -next v4 3/6] Revert "frontswap: simplify frontswap_register_ops" Liu Shixin
@ 2022-08-29 13:23 ` Liu Shixin
  2022-08-29 13:23 ` [PATCH -next v4 5/6] mm/zswap: delay the initializaton of zswap until the first enablement Liu Shixin
  4 siblings, 0 replies; 7+ messages in thread
From: Liu Shixin @ 2022-08-29 13:23 UTC (permalink / raw)
  To: Seth Jennings, Dan Streetman, Vitaly Wool, Andrew Morton,
	Nathan Chancellor, Christoph Hellwig
  Cc: linux-mm, linux-kernel, Liu Shixin, Kefeng Wang

zswap_init_started indicates that the initialization is started. And
zswap_init_failed indicates that the initialization is failed. As we will
support to init zswap after system startup, it's necessary to add a state
to indicate the initialization is complete and succeed to avoid
concurrency issues. Since we don't care about the difference between
init started with init completion. We only need three states:
uninitialized, initial failed, initial succeed.

Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
 mm/zswap.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/mm/zswap.c b/mm/zswap.c
index c22208df292a..155aee785a16 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -214,11 +214,12 @@ static DEFINE_SPINLOCK(zswap_pools_lock);
 /* pool counter to provide unique names to zpool */
 static atomic_t zswap_pools_count = ATOMIC_INIT(0);
 
-/* used by param callback function */
-static bool zswap_init_started;
+#define ZSWAP_UNINIT		0
+#define ZSWAP_INIT_SUCCEED	1
+#define ZSWAP_INIT_FAILED	2
 
-/* fatal error during init */
-static bool zswap_init_failed;
+/* init state */
+static int zswap_init_state;
 
 /* init completed, but couldn't create the initial pool */
 static bool zswap_has_pool;
@@ -772,7 +773,7 @@ static int __zswap_param_set(const char *val, const struct kernel_param *kp,
 	char *s = strstrip((char *)val);
 	int ret;
 
-	if (zswap_init_failed) {
+	if (zswap_init_state == ZSWAP_INIT_FAILED) {
 		pr_err("can't set param, initialization failed\n");
 		return -ENODEV;
 	}
@@ -784,7 +785,7 @@ static int __zswap_param_set(const char *val, const struct kernel_param *kp,
 	/* if this is load-time (pre-init) param setting,
 	 * don't create a pool; that's done during init.
 	 */
-	if (!zswap_init_started)
+	if (zswap_init_state == ZSWAP_UNINIT)
 		return param_set_charp(s, kp);
 
 	if (!type) {
@@ -875,11 +876,11 @@ static int zswap_zpool_param_set(const char *val,
 static int zswap_enabled_param_set(const char *val,
 				   const struct kernel_param *kp)
 {
-	if (zswap_init_failed) {
+	if (zswap_init_state == ZSWAP_INIT_FAILED) {
 		pr_err("can't enable, initialization failed\n");
 		return -ENODEV;
 	}
-	if (!zswap_has_pool && zswap_init_started) {
+	if (!zswap_has_pool && zswap_init_state == ZSWAP_INIT_SUCCEED) {
 		pr_err("can't enable, no pool configured\n");
 		return -ENODEV;
 	}
@@ -1476,8 +1477,6 @@ static int __init init_zswap(void)
 	struct zswap_pool *pool;
 	int ret;
 
-	zswap_init_started = true;
-
 	if (zswap_entry_cache_create()) {
 		pr_err("entry cache creation failed\n");
 		goto cache_fail;
@@ -1515,6 +1514,7 @@ static int __init init_zswap(void)
 	frontswap_register_ops(&zswap_frontswap_ops);
 	if (zswap_debugfs_init())
 		pr_warn("debugfs initialization failed\n");
+	zswap_init_state = ZSWAP_INIT_SUCCEED;
 	return 0;
 
 fallback_fail:
@@ -1526,7 +1526,7 @@ static int __init init_zswap(void)
 	zswap_entry_cache_destroy();
 cache_fail:
 	/* if built-in, we aren't unloaded on failure; don't allow use */
-	zswap_init_failed = true;
+	zswap_init_state = ZSWAP_INIT_FAILED;
 	zswap_enabled = false;
 	return -ENOMEM;
 }
-- 
2.25.1



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

* [PATCH -next v4 5/6] mm/zswap: delay the initializaton of zswap until the first enablement
  2022-08-29 13:22 [PATCH -next v4 0/6] Delay the initializaton of zswap Liu Shixin
                   ` (3 preceding siblings ...)
  2022-08-29 13:23 ` [PATCH -next v4 4/6] mm/zswap: replace zswap_init_{started/failed} with zswap_init_state Liu Shixin
@ 2022-08-29 13:23 ` Liu Shixin
  4 siblings, 0 replies; 7+ messages in thread
From: Liu Shixin @ 2022-08-29 13:23 UTC (permalink / raw)
  To: Seth Jennings, Dan Streetman, Vitaly Wool, Andrew Morton,
	Nathan Chancellor, Christoph Hellwig
  Cc: linux-mm, linux-kernel, Liu Shixin, Kefeng Wang

In the initialization of zswap, about 18MB memory will be allocated for
zswap_pool in my machine. Since not all users use zswap, the memory may be
wasted. Save the memory for these users by delaying the initialization of
zswap to first enablement.

Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
 mm/zswap.c | 50 ++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 40 insertions(+), 10 deletions(-)

diff --git a/mm/zswap.c b/mm/zswap.c
index 155aee785a16..dac1714c7eda 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -81,6 +81,8 @@ static bool zswap_pool_reached_full;
 
 #define ZSWAP_PARAM_UNSET ""
 
+static int zswap_setup(void);
+
 /* Enable/disable zswap */
 static bool zswap_enabled = IS_ENABLED(CONFIG_ZSWAP_DEFAULT_ON);
 static int zswap_enabled_param_set(const char *,
@@ -220,6 +222,8 @@ static atomic_t zswap_pools_count = ATOMIC_INIT(0);
 
 /* init state */
 static int zswap_init_state;
+/* used to ensure the integrity of initialization */
+static DEFINE_MUTEX(zswap_init_lock);
 
 /* init completed, but couldn't create the initial pool */
 static bool zswap_has_pool;
@@ -273,13 +277,13 @@ static void zswap_update_total_size(void)
 **********************************/
 static struct kmem_cache *zswap_entry_cache;
 
-static int __init zswap_entry_cache_create(void)
+static int zswap_entry_cache_create(void)
 {
 	zswap_entry_cache = KMEM_CACHE(zswap_entry, 0);
 	return zswap_entry_cache == NULL;
 }
 
-static void __init zswap_entry_cache_destroy(void)
+static void zswap_entry_cache_destroy(void)
 {
 	kmem_cache_destroy(zswap_entry_cache);
 }
@@ -664,7 +668,7 @@ static struct zswap_pool *zswap_pool_create(char *type, char *compressor)
 	return NULL;
 }
 
-static __init struct zswap_pool *__zswap_pool_create_fallback(void)
+static struct zswap_pool *__zswap_pool_create_fallback(void)
 {
 	bool has_comp, has_zpool;
 
@@ -782,11 +786,17 @@ static int __zswap_param_set(const char *val, const struct kernel_param *kp,
 	if (!strcmp(s, *(char **)kp->arg) && zswap_has_pool)
 		return 0;
 
-	/* if this is load-time (pre-init) param setting,
+	/*
+	 * if zswap has not been initialized,
 	 * don't create a pool; that's done during init.
 	 */
-	if (zswap_init_state == ZSWAP_UNINIT)
-		return param_set_charp(s, kp);
+	mutex_lock(&zswap_init_lock);
+	if (zswap_init_state == ZSWAP_UNINIT) {
+		ret = param_set_charp(s, kp);
+		mutex_unlock(&zswap_init_lock);
+		return ret;
+	}
+	mutex_unlock(&zswap_init_lock);
 
 	if (!type) {
 		if (!zpool_has_pool(s)) {
@@ -876,6 +886,14 @@ static int zswap_zpool_param_set(const char *val,
 static int zswap_enabled_param_set(const char *val,
 				   const struct kernel_param *kp)
 {
+	if (system_state == SYSTEM_RUNNING) {
+		mutex_lock(&zswap_init_lock);
+		if (zswap_setup()) {
+			mutex_unlock(&zswap_init_lock);
+			return -ENODEV;
+		}
+		mutex_unlock(&zswap_init_lock);
+	}
 	if (zswap_init_state == ZSWAP_INIT_FAILED) {
 		pr_err("can't enable, initialization failed\n");
 		return -ENODEV;
@@ -1432,7 +1450,7 @@ static struct frontswap_ops zswap_frontswap_ops = {
 
 static struct dentry *zswap_debugfs_root;
 
-static int __init zswap_debugfs_init(void)
+static int zswap_debugfs_init(void)
 {
 	if (!debugfs_initialized())
 		return -ENODEV;
@@ -1463,7 +1481,7 @@ static int __init zswap_debugfs_init(void)
 	return 0;
 }
 #else
-static int __init zswap_debugfs_init(void)
+static int zswap_debugfs_init(void)
 {
 	return 0;
 }
@@ -1472,11 +1490,14 @@ static int __init zswap_debugfs_init(void)
 /*********************************
 * module init and exit
 **********************************/
-static int __init init_zswap(void)
+static int zswap_setup(void)
 {
 	struct zswap_pool *pool;
 	int ret;
 
+	if (zswap_init_state != ZSWAP_UNINIT)
+		return 0;
+
 	if (zswap_entry_cache_create()) {
 		pr_err("entry cache creation failed\n");
 		goto cache_fail;
@@ -1530,8 +1551,17 @@ static int __init init_zswap(void)
 	zswap_enabled = false;
 	return -ENOMEM;
 }
+
+static int __init zswap_init(void)
+{
+	/* skip init if zswap is disabled when system startup */
+	if (!zswap_enabled)
+		return 0;
+	return zswap_setup();
+}
+
 /* must be late so crypto has time to come up */
-late_initcall(init_zswap);
+late_initcall(zswap_init);
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Seth Jennings <sjennings@variantweb.net>");
-- 
2.25.1



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

* Re: [PATCH -next v4 1/6] Revert "frontswap: remove support for multiple ops"
  2022-08-29 13:22 ` [PATCH -next v4 1/6] Revert "frontswap: remove support for multiple ops" Liu Shixin
@ 2022-08-30  5:05   ` Christoph Hellwig
  0 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2022-08-30  5:05 UTC (permalink / raw)
  To: Liu Shixin
  Cc: Seth Jennings, Dan Streetman, Vitaly Wool, Andrew Morton,
	Nathan Chancellor, Christoph Hellwig, linux-mm, linux-kernel,
	Kefeng Wang

On Mon, Aug 29, 2022 at 09:22:57PM +0800, Liu Shixin wrote:
> This reverts commit 1da0d94a3ec8c5f3793b7be8538b55e60ebeefe3.

Err, no, this is a way to big hammer and going in the wrong way.
If we can't have a sane way to fix it, the fix is just to remove
the now rather pointless fontswap abstraction entirely.

I'm on vacation this week, but I'll look into it ASAP once back.


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

end of thread, other threads:[~2022-08-30  5:06 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-29 13:22 [PATCH -next v4 0/6] Delay the initializaton of zswap Liu Shixin
2022-08-29 13:22 ` [PATCH -next v4 1/6] Revert "frontswap: remove support for multiple ops" Liu Shixin
2022-08-30  5:05   ` Christoph Hellwig
2022-08-29 13:22 ` [PATCH -next v4 2/6] Revert "mm: mark swap_lock and swap_active_head static" Liu Shixin
2022-08-29 13:22 ` [PATCH -next v4 3/6] Revert "frontswap: simplify frontswap_register_ops" Liu Shixin
2022-08-29 13:23 ` [PATCH -next v4 4/6] mm/zswap: replace zswap_init_{started/failed} with zswap_init_state Liu Shixin
2022-08-29 13:23 ` [PATCH -next v4 5/6] mm/zswap: delay the initializaton of zswap until the first enablement Liu Shixin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox