[PATCH] mm/migrate: Fix NULL movable_ops if CONFIG_ZSMALLOC=m

[PATCH] mm/migrate: Fix NULL movable_ops if CONFIG_ZSMALLOC=m

[Huacai Chen]

After commit 84caf98838a3e5f4bdb34 ("mm: stop storing migration_ops in
page->mapping") we get such an error message if CONFIG_ZSMALLOC=m:

 WARNING: CPU: 3 PID: 42 at mm/migrate.c:142 isolate_movable_ops_page+0xa8/0x1c0
 CPU: 3 UID: 0 PID: 42 Comm: kcompactd0 Not tainted 6.16.0-rc5+ #2133 PREEMPT
 pc 9000000000540bd8 ra 9000000000540b84 tp 9000000100420000 sp 9000000100423a60
 a0 9000000100193a80 a1 000000000000000c a2 000000000000001b a3 ffffffffffffffff
 a4 ffffffffffffffff a5 0000000000000267 a6 0000000000000000 a7 9000000100423ae0
 t0 00000000000000f1 t1 00000000000000f6 t2 0000000000000000 t3 0000000000000001
 t4 ffffff00010eb834 t5 0000000000000040 t6 900000010c89d380 t7 90000000023fcc70
 t8 0000000000000018 u0 0000000000000000 s9 ffffff00010eb800 s0 ffffff00010eb800
 s1 000000000000000c s2 0000000000043ae0 s3 0000800000000000 s4 900000000219cc40
 s5 0000000000000000 s6 ffffff00010eb800 s7 0000000000000001 s8 90000000025b4000
    ra: 9000000000540b84 isolate_movable_ops_page+0x54/0x1c0
   ERA: 9000000000540bd8 isolate_movable_ops_page+0xa8/0x1c0
  CRMD: 000000b0 (PLV0 -IE -DA +PG DACF=CC DACM=CC -WE)
  PRMD: 00000004 (PPLV0 +PIE -PWE)
  EUEN: 00000000 (-FPE -SXE -ASXE -BTE)
  ECFG: 00071c1d (LIE=0,2-4,10-12 VS=7)
 ESTAT: 000c0000 [BRK] (IS= ECode=12 EsubCode=0)
  PRID: 0014c010 (Loongson-64bit, Loongson-3A5000)
 CPU: 3 UID: 0 PID: 42 Comm: kcompactd0 Not tainted 6.16.0-rc5+ #2133 PREEMPT
 Stack : 90000000021fd000 0000000000000000 9000000000247720 9000000100420000
         90000001004236a0 90000001004236a8 0000000000000000 90000001004237e8
         90000001004237e0 90000001004237e0 9000000100423550 0000000000000001
         0000000000000001 90000001004236a8 725a84864a19e2d9 90000000023fcc58
         9000000100420000 90000000024c6848 9000000002416848 0000000000000001
         0000000000000000 000000000000000a 0000000007fe0000 ffffff00010eb800
         0000000000000000 90000000021fd000 0000000000000000 900000000205cf30
         000000000000008e 0000000000000009 ffffff00010eb800 0000000000000001
         90000000025b4000 0000000000000000 900000000024773c 00007ffff103d748
         00000000000000b0 0000000000000004 0000000000000000 0000000000071c1d
         ...
 Call Trace:
 [<900000000024773c>] show_stack+0x5c/0x190
 [<90000000002415e0>] dump_stack_lvl+0x70/0x9c
 [<90000000004abe6c>] isolate_migratepages_block+0x3bc/0x16e0
 [<90000000004af408>] compact_zone+0x558/0x1000
 [<90000000004b0068>] compact_node+0xa8/0x1e0
 [<90000000004b0aa4>] kcompactd+0x394/0x410
 [<90000000002b3c98>] kthread+0x128/0x140
 [<9000000001779148>] ret_from_kernel_thread+0x28/0xc0
 [<9000000000245528>] ret_from_kernel_thread_asm+0x10/0x88

The reason is that defined(CONFIG_ZSMALLOC) evaluates to 1 only when
CONFIG_ZSMALLOC=y, we should use IS_ENABLED(CONFIG_ZSMALLOC) instead.
But when I use IS_ENABLED(CONFIG_ZSMALLOC), page_movable_ops() cannot
access zsmalloc_mops because zsmalloc_mops is in a module.

To solve this problem, we define a movable_ops[] array in mm/migrate.c,
initialise its elements at mm/balloon_compaction.c & mm/zsmalloc.c, and
let the page_movable_ops() function return elements from movable_ops[].

Fixes: 84caf98838a3e5f ("mm: stop storing migration_ops in page->mapping")
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
 include/linux/migrate.h |  6 ++++++
 mm/balloon_compaction.c |  7 +++++++
 mm/migrate.c            | 16 ++++++++++------
 mm/zsmalloc.c           |  6 ++++++
 4 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index acadd41e0b5c..58fac171e4d5 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -57,6 +57,12 @@ struct movable_operations {
 	void (*putback_page)(struct page *);
 };
 
+#define MOVABLE_BALLOON		0
+#define MOVABLE_ZSMALLOC	1
+#define MOVABLE_MAX		2
+
+extern const struct movable_operations *movable_ops[MOVABLE_MAX];
+
 /* Defined in mm/debug.c: */
 extern const char *migrate_reason_names[MR_TYPES];
 
diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
index 2a4a649805c1..a1d2625b5c39 100644
--- a/mm/balloon_compaction.c
+++ b/mm/balloon_compaction.c
@@ -254,4 +254,11 @@ const struct movable_operations balloon_mops = {
 	.putback_page = balloon_page_putback,
 };
 
+static int __init balloon_init(void)
+{
+	movable_ops[MOVABLE_BALLOON] = &balloon_mops;
+	return 0;
+}
+core_initcall(balloon_init);
+
 #endif /* CONFIG_BALLOON_COMPACTION */
diff --git a/mm/migrate.c b/mm/migrate.c
index 425401b2d4e1..92918fab3e1a 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -53,6 +53,10 @@
 #include "internal.h"
 #include "swap.h"
 
+const struct movable_operations *movable_ops[MOVABLE_MAX];
+
+EXPORT_SYMBOL(movable_ops);
+
 static const struct movable_operations *page_movable_ops(struct page *page)
 {
 	VM_WARN_ON_ONCE_PAGE(!page_has_movable_ops(page), page);
@@ -62,15 +66,15 @@ static const struct movable_operations *page_movable_ops(struct page *page)
 	 * it as movable, the page type must be sticky until the page gets freed
 	 * back to the buddy.
 	 */
-#ifdef CONFIG_BALLOON_COMPACTION
+#if IS_ENABLED(CONFIG_BALLOON_COMPACTION)
 	if (PageOffline(page))
 		/* Only balloon compaction sets PageOffline pages movable. */
-		return &balloon_mops;
-#endif /* CONFIG_BALLOON_COMPACTION */
-#if defined(CONFIG_ZSMALLOC) && defined(CONFIG_COMPACTION)
+		return movable_ops[MOVABLE_BALLOON];
+#endif /* IS_ENABLED(CONFIG_BALLOON_COMPACTION) */
+#if IS_ENABLED(CONFIG_ZSMALLOC) && IS_ENABLED(CONFIG_COMPACTION)
 	if (PageZsmalloc(page))
-		return &zsmalloc_mops;
-#endif /* defined(CONFIG_ZSMALLOC) && defined(CONFIG_COMPACTION) */
+		return movable_ops[MOVABLE_ZSMALLOC];
+#endif /* IS_ENABLED(CONFIG_ZSMALLOC) && IS_ENABLED(CONFIG_COMPACTION) */
 	return NULL;
 }
 
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 2c5e56a65354..052fcc0d29e5 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -2246,6 +2246,9 @@ EXPORT_SYMBOL_GPL(zs_destroy_pool);
 
 static int __init zs_init(void)
 {
+#ifdef CONFIG_MIGRATION
+	movable_ops[MOVABLE_ZSMALLOC] = &zsmalloc_mops;
+#endif
 #ifdef CONFIG_ZPOOL
 	zpool_register_driver(&zs_zpool_driver);
 #endif
@@ -2255,6 +2258,9 @@ static int __init zs_init(void)
 
 static void __exit zs_exit(void)
 {
+#ifdef CONFIG_MIGRATION
+	movable_ops[MOVABLE_ZSMALLOC] = NULL;
+#endif
 #ifdef CONFIG_ZPOOL
 	zpool_unregister_driver(&zs_zpool_driver);
 #endif
-- 
2.47.3

Re: [PATCH] mm/migrate: Fix NULL movable_ops if CONFIG_ZSMALLOC=m

[Zi Yan]

On 15 Aug 2025, at 5:05, Huacai Chen wrote:

> After commit 84caf98838a3e5f4bdb34 ("mm: stop storing migration_ops in
> page->mapping") we get such an error message if CONFIG_ZSMALLOC=m:
>
>  WARNING: CPU: 3 PID: 42 at mm/migrate.c:142 isolate_movable_ops_page+0xa8/0x1c0
>  CPU: 3 UID: 0 PID: 42 Comm: kcompactd0 Not tainted 6.16.0-rc5+ #2133 PREEMPT
>  pc 9000000000540bd8 ra 9000000000540b84 tp 9000000100420000 sp 9000000100423a60
>  a0 9000000100193a80 a1 000000000000000c a2 000000000000001b a3 ffffffffffffffff
>  a4 ffffffffffffffff a5 0000000000000267 a6 0000000000000000 a7 9000000100423ae0
>  t0 00000000000000f1 t1 00000000000000f6 t2 0000000000000000 t3 0000000000000001
>  t4 ffffff00010eb834 t5 0000000000000040 t6 900000010c89d380 t7 90000000023fcc70
>  t8 0000000000000018 u0 0000000000000000 s9 ffffff00010eb800 s0 ffffff00010eb800
>  s1 000000000000000c s2 0000000000043ae0 s3 0000800000000000 s4 900000000219cc40
>  s5 0000000000000000 s6 ffffff00010eb800 s7 0000000000000001 s8 90000000025b4000
>     ra: 9000000000540b84 isolate_movable_ops_page+0x54/0x1c0
>    ERA: 9000000000540bd8 isolate_movable_ops_page+0xa8/0x1c0
>   CRMD: 000000b0 (PLV0 -IE -DA +PG DACF=CC DACM=CC -WE)
>   PRMD: 00000004 (PPLV0 +PIE -PWE)
>   EUEN: 00000000 (-FPE -SXE -ASXE -BTE)
>   ECFG: 00071c1d (LIE=0,2-4,10-12 VS=7)
>  ESTAT: 000c0000 [BRK] (IS= ECode=12 EsubCode=0)
>   PRID: 0014c010 (Loongson-64bit, Loongson-3A5000)
>  CPU: 3 UID: 0 PID: 42 Comm: kcompactd0 Not tainted 6.16.0-rc5+ #2133 PREEMPT
>  Stack : 90000000021fd000 0000000000000000 9000000000247720 9000000100420000
>          90000001004236a0 90000001004236a8 0000000000000000 90000001004237e8
>          90000001004237e0 90000001004237e0 9000000100423550 0000000000000001
>          0000000000000001 90000001004236a8 725a84864a19e2d9 90000000023fcc58
>          9000000100420000 90000000024c6848 9000000002416848 0000000000000001
>          0000000000000000 000000000000000a 0000000007fe0000 ffffff00010eb800
>          0000000000000000 90000000021fd000 0000000000000000 900000000205cf30
>          000000000000008e 0000000000000009 ffffff00010eb800 0000000000000001
>          90000000025b4000 0000000000000000 900000000024773c 00007ffff103d748
>          00000000000000b0 0000000000000004 0000000000000000 0000000000071c1d
>          ...
>  Call Trace:
>  [<900000000024773c>] show_stack+0x5c/0x190
>  [<90000000002415e0>] dump_stack_lvl+0x70/0x9c
>  [<90000000004abe6c>] isolate_migratepages_block+0x3bc/0x16e0
>  [<90000000004af408>] compact_zone+0x558/0x1000
>  [<90000000004b0068>] compact_node+0xa8/0x1e0
>  [<90000000004b0aa4>] kcompactd+0x394/0x410
>  [<90000000002b3c98>] kthread+0x128/0x140
>  [<9000000001779148>] ret_from_kernel_thread+0x28/0xc0
>  [<9000000000245528>] ret_from_kernel_thread_asm+0x10/0x88
>
> The reason is that defined(CONFIG_ZSMALLOC) evaluates to 1 only when
> CONFIG_ZSMALLOC=y, we should use IS_ENABLED(CONFIG_ZSMALLOC) instead.
> But when I use IS_ENABLED(CONFIG_ZSMALLOC), page_movable_ops() cannot
> access zsmalloc_mops because zsmalloc_mops is in a module.
>
> To solve this problem, we define a movable_ops[] array in mm/migrate.c,
> initialise its elements at mm/balloon_compaction.c & mm/zsmalloc.c, and
> let the page_movable_ops() function return elements from movable_ops[].
>
> Fixes: 84caf98838a3e5f ("mm: stop storing migration_ops in page->mapping")
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> ---
>  include/linux/migrate.h |  6 ++++++
>  mm/balloon_compaction.c |  7 +++++++
>  mm/migrate.c            | 16 ++++++++++------
>  mm/zsmalloc.c           |  6 ++++++
>  4 files changed, 29 insertions(+), 6 deletions(-)

The fix looks reasonable to me. Just a comment on EXPORT_SYMBOL below.
Acked-by: Zi Yan <ziy@nvidia.com>

>
> diff --git a/include/linux/migrate.h b/include/linux/migrate.h
> index acadd41e0b5c..58fac171e4d5 100644
> --- a/include/linux/migrate.h
> +++ b/include/linux/migrate.h
> @@ -57,6 +57,12 @@ struct movable_operations {
>  	void (*putback_page)(struct page *);
>  };
>
> +#define MOVABLE_BALLOON		0
> +#define MOVABLE_ZSMALLOC	1
> +#define MOVABLE_MAX		2
> +
> +extern const struct movable_operations *movable_ops[MOVABLE_MAX];
> +
>  /* Defined in mm/debug.c: */
>  extern const char *migrate_reason_names[MR_TYPES];
>
> diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
> index 2a4a649805c1..a1d2625b5c39 100644
> --- a/mm/balloon_compaction.c
> +++ b/mm/balloon_compaction.c
> @@ -254,4 +254,11 @@ const struct movable_operations balloon_mops = {
>  	.putback_page = balloon_page_putback,
>  };
>
> +static int __init balloon_init(void)
> +{
> +	movable_ops[MOVABLE_BALLOON] = &balloon_mops;
> +	return 0;
> +}
> +core_initcall(balloon_init);
> +
>  #endif /* CONFIG_BALLOON_COMPACTION */
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 425401b2d4e1..92918fab3e1a 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -53,6 +53,10 @@
>  #include "internal.h"
>  #include "swap.h"
>
> +const struct movable_operations *movable_ops[MOVABLE_MAX];
> +
> +EXPORT_SYMBOL(movable_ops);

It is only used by zsmalloc and balloon_compaction, maybe
EXPORT_SYMBOL_GPL_FOR_MODULES is better?

> +
>  static const struct movable_operations *page_movable_ops(struct page *page)
>  {
>  	VM_WARN_ON_ONCE_PAGE(!page_has_movable_ops(page), page);
> @@ -62,15 +66,15 @@ static const struct movable_operations *page_movable_ops(struct page *page)
>  	 * it as movable, the page type must be sticky until the page gets freed
>  	 * back to the buddy.
>  	 */
> -#ifdef CONFIG_BALLOON_COMPACTION
> +#if IS_ENABLED(CONFIG_BALLOON_COMPACTION)
>  	if (PageOffline(page))
>  		/* Only balloon compaction sets PageOffline pages movable. */
> -		return &balloon_mops;
> -#endif /* CONFIG_BALLOON_COMPACTION */
> -#if defined(CONFIG_ZSMALLOC) && defined(CONFIG_COMPACTION)
> +		return movable_ops[MOVABLE_BALLOON];
> +#endif /* IS_ENABLED(CONFIG_BALLOON_COMPACTION) */
> +#if IS_ENABLED(CONFIG_ZSMALLOC) && IS_ENABLED(CONFIG_COMPACTION)
>  	if (PageZsmalloc(page))
> -		return &zsmalloc_mops;
> -#endif /* defined(CONFIG_ZSMALLOC) && defined(CONFIG_COMPACTION) */
> +		return movable_ops[MOVABLE_ZSMALLOC];
> +#endif /* IS_ENABLED(CONFIG_ZSMALLOC) && IS_ENABLED(CONFIG_COMPACTION) */
>  	return NULL;
>  }
>
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index 2c5e56a65354..052fcc0d29e5 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -2246,6 +2246,9 @@ EXPORT_SYMBOL_GPL(zs_destroy_pool);
>
>  static int __init zs_init(void)
>  {
> +#ifdef CONFIG_MIGRATION
> +	movable_ops[MOVABLE_ZSMALLOC] = &zsmalloc_mops;
> +#endif
>  #ifdef CONFIG_ZPOOL
>  	zpool_register_driver(&zs_zpool_driver);
>  #endif
> @@ -2255,6 +2258,9 @@ static int __init zs_init(void)
>
>  static void __exit zs_exit(void)
>  {
> +#ifdef CONFIG_MIGRATION
> +	movable_ops[MOVABLE_ZSMALLOC] = NULL;
> +#endif
>  #ifdef CONFIG_ZPOOL
>  	zpool_unregister_driver(&zs_zpool_driver);
>  #endif
> -- 
> 2.47.3


--
Best Regards,
Yan, Zi

Re: [PATCH] mm/migrate: Fix NULL movable_ops if CONFIG_ZSMALLOC=m

[kernel test robot]

Hi Huacai,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]

url:    https://github.com/intel-lab-lkp/linux/commits/Huacai-Chen/mm-migrate-Fix-NULL-movable_ops-if-CONFIG_ZSMALLOC-m/20250815-170745
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20250815090539.1578484-1-chenhuacai%40loongson.cn
patch subject: [PATCH] mm/migrate: Fix NULL movable_ops if CONFIG_ZSMALLOC=m
config: sparc-randconfig-001-20250816 (https://download.01.org/0day-ci/archive/20250816/202508160823.9dRNF1Rc-lkp@intel.com/config)
compiler: sparc-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250816/202508160823.9dRNF1Rc-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508160823.9dRNF1Rc-lkp@intel.com/

All errors (new ones prefixed by >>):

   sparc-linux-ld: mm/zsmalloc.o: in function `zs_init':
>> mm/zsmalloc.c:2250:(.init.text+0x8): undefined reference to `zsmalloc_mops'
>> sparc-linux-ld: mm/zsmalloc.c:2250:(.init.text+0xc): undefined reference to `zsmalloc_mops'


vim +2250 mm/zsmalloc.c

  2246	
  2247	static int __init zs_init(void)
  2248	{
  2249	#ifdef CONFIG_MIGRATION
> 2250		movable_ops[MOVABLE_ZSMALLOC] = &zsmalloc_mops;
  2251	#endif
  2252	#ifdef CONFIG_ZPOOL
  2253		zpool_register_driver(&zs_zpool_driver);
  2254	#endif
  2255		zs_stat_init();
  2256		return 0;
  2257	}
  2258	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] mm/migrate: Fix NULL movable_ops if CONFIG_ZSMALLOC=m

[David Hildenbrand]

On 15.08.25 11:05, Huacai Chen wrote:

Hi,

please CC the appropriate maintainers next time. You missed (some) 
balloon and zsmalloc maintainers.

> After commit 84caf98838a3e5f4bdb34 ("mm: stop storing migration_ops in
> page->mapping") we get such an error message if CONFIG_ZSMALLOC=m:
> 
>   WARNING: CPU: 3 PID: 42 at mm/migrate.c:142 isolate_movable_ops_page+0xa8/0x1c0
>   CPU: 3 UID: 0 PID: 42 Comm: kcompactd0 Not tainted 6.16.0-rc5+ #2133 PREEMPT
>   pc 9000000000540bd8 ra 9000000000540b84 tp 9000000100420000 sp 9000000100423a60
>   a0 9000000100193a80 a1 000000000000000c a2 000000000000001b a3 ffffffffffffffff
>   a4 ffffffffffffffff a5 0000000000000267 a6 0000000000000000 a7 9000000100423ae0
>   t0 00000000000000f1 t1 00000000000000f6 t2 0000000000000000 t3 0000000000000001
>   t4 ffffff00010eb834 t5 0000000000000040 t6 900000010c89d380 t7 90000000023fcc70
>   t8 0000000000000018 u0 0000000000000000 s9 ffffff00010eb800 s0 ffffff00010eb800
>   s1 000000000000000c s2 0000000000043ae0 s3 0000800000000000 s4 900000000219cc40
>   s5 0000000000000000 s6 ffffff00010eb800 s7 0000000000000001 s8 90000000025b4000
>      ra: 9000000000540b84 isolate_movable_ops_page+0x54/0x1c0
>     ERA: 9000000000540bd8 isolate_movable_ops_page+0xa8/0x1c0
>    CRMD: 000000b0 (PLV0 -IE -DA +PG DACF=CC DACM=CC -WE)
>    PRMD: 00000004 (PPLV0 +PIE -PWE)
>    EUEN: 00000000 (-FPE -SXE -ASXE -BTE)
>    ECFG: 00071c1d (LIE=0,2-4,10-12 VS=7)
>   ESTAT: 000c0000 [BRK] (IS= ECode=12 EsubCode=0)
>    PRID: 0014c010 (Loongson-64bit, Loongson-3A5000)
>   CPU: 3 UID: 0 PID: 42 Comm: kcompactd0 Not tainted 6.16.0-rc5+ #2133 PREEMPT
>   Stack : 90000000021fd000 0000000000000000 9000000000247720 9000000100420000
>           90000001004236a0 90000001004236a8 0000000000000000 90000001004237e8
>           90000001004237e0 90000001004237e0 9000000100423550 0000000000000001
>           0000000000000001 90000001004236a8 725a84864a19e2d9 90000000023fcc58
>           9000000100420000 90000000024c6848 9000000002416848 0000000000000001
>           0000000000000000 000000000000000a 0000000007fe0000 ffffff00010eb800
>           0000000000000000 90000000021fd000 0000000000000000 900000000205cf30
>           000000000000008e 0000000000000009 ffffff00010eb800 0000000000000001
>           90000000025b4000 0000000000000000 900000000024773c 00007ffff103d748
>           00000000000000b0 0000000000000004 0000000000000000 0000000000071c1d
>           ...
>   Call Trace:
>   [<900000000024773c>] show_stack+0x5c/0x190
>   [<90000000002415e0>] dump_stack_lvl+0x70/0x9c
>   [<90000000004abe6c>] isolate_migratepages_block+0x3bc/0x16e0
>   [<90000000004af408>] compact_zone+0x558/0x1000
>   [<90000000004b0068>] compact_node+0xa8/0x1e0
>   [<90000000004b0aa4>] kcompactd+0x394/0x410
>   [<90000000002b3c98>] kthread+0x128/0x140
>   [<9000000001779148>] ret_from_kernel_thread+0x28/0xc0
>   [<9000000000245528>] ret_from_kernel_thread_asm+0x10/0x88
> 
> The reason is that defined(CONFIG_ZSMALLOC) evaluates to 1 only when
> CONFIG_ZSMALLOC=y, we should use IS_ENABLED(CONFIG_ZSMALLOC) instead.

Ouch, I missed that CONFIG_ZSMALLOC can be configured like that. I 
thought it would always be builtin.

> But when I use IS_ENABLED(CONFIG_ZSMALLOC), page_movable_ops() cannot
> access zsmalloc_mops because zsmalloc_mops is in a module.
> 
> To solve this problem, we define a movable_ops[] array in mm/migrate.c,
> initialise its elements at mm/balloon_compaction.c & mm/zsmalloc.c, and
> let the page_movable_ops() function return elements from movable_ops[].

Before I took that easy route to just get it working quickly, I 
envisioned a proper registration interface. See below.

> 
> Fixes: 84caf98838a3e5f ("mm: stop storing migration_ops in page->mapping")
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> ---
>   include/linux/migrate.h |  6 ++++++
>   mm/balloon_compaction.c |  7 +++++++
>   mm/migrate.c            | 16 ++++++++++------
>   mm/zsmalloc.c           |  6 ++++++
>   4 files changed, 29 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/migrate.h b/include/linux/migrate.h
> index acadd41e0b5c..58fac171e4d5 100644
> --- a/include/linux/migrate.h
> +++ b/include/linux/migrate.h
> @@ -57,6 +57,12 @@ struct movable_operations {
>   	void (*putback_page)(struct page *);
>   };
>   
> +#define MOVABLE_BALLOON		0
> +#define MOVABLE_ZSMALLOC	1
> +#define MOVABLE_MAX		2
> +
> +extern const struct movable_operations *movable_ops[MOVABLE_MAX];
> +
>   /* Defined in mm/debug.c: */
>   extern const char *migrate_reason_names[MR_TYPES];
>   
> diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
> index 2a4a649805c1..a1d2625b5c39 100644
> --- a/mm/balloon_compaction.c
> +++ b/mm/balloon_compaction.c
> @@ -254,4 +254,11 @@ const struct movable_operations balloon_mops = {
>   	.putback_page = balloon_page_putback,
>   };
>   
> +static int __init balloon_init(void)
> +{
> +	movable_ops[MOVABLE_BALLOON] = &balloon_mops;
> +	return 0;
> +}
> +core_initcall(balloon_init);
> +
>   #endif /* CONFIG_BALLOON_COMPACTION */
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 425401b2d4e1..92918fab3e1a 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -53,6 +53,10 @@
>   #include "internal.h"
>   #include "swap.h"
>   
> +const struct movable_operations *movable_ops[MOVABLE_MAX];
> +
> +EXPORT_SYMBOL(movable_ops);
> +
>   static const struct movable_operations *page_movable_ops(struct page *page)
>   {
>   	VM_WARN_ON_ONCE_PAGE(!page_has_movable_ops(page), page);
> @@ -62,15 +66,15 @@ static const struct movable_operations *page_movable_ops(struct page *page)
>   	 * it as movable, the page type must be sticky until the page gets freed
>   	 * back to the buddy.
>   	 */
> -#ifdef CONFIG_BALLOON_COMPACTION
> +#if IS_ENABLED(CONFIG_BALLOON_COMPACTION)
>   	if (PageOffline(page))
>   		/* Only balloon compaction sets PageOffline pages movable. */
> -		return &balloon_mops;
> -#endif /* CONFIG_BALLOON_COMPACTION */
> -#if defined(CONFIG_ZSMALLOC) && defined(CONFIG_COMPACTION)
> +		return movable_ops[MOVABLE_BALLOON];
> +#endif /* IS_ENABLED(CONFIG_BALLOON_COMPACTION) */
> +#if IS_ENABLED(CONFIG_ZSMALLOC) && IS_ENABLED(CONFIG_COMPACTION)
>   	if (PageZsmalloc(page))
> -		return &zsmalloc_mops;
> -#endif /* defined(CONFIG_ZSMALLOC) && defined(CONFIG_COMPACTION) */
> +		return movable_ops[MOVABLE_ZSMALLOC];
> +#endif /* IS_ENABLED(CONFIG_ZSMALLOC) && IS_ENABLED(CONFIG_COMPACTION) */
>   	return NULL;
>   }
>   
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index 2c5e56a65354..052fcc0d29e5 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -2246,6 +2246,9 @@ EXPORT_SYMBOL_GPL(zs_destroy_pool);
>   
>   static int __init zs_init(void)
>   {
> +#ifdef CONFIG_MIGRATION
> +	movable_ops[MOVABLE_ZSMALLOC] = &zsmalloc_mops;
> +#endif

No, not like this.

Please add a proper registration function, and use the page type as an 
input.

int register_movable_ops(const struct movable_operations *, enum
			 pagetype);

Probably best to refuse any page_type except PGTY_offline and 
PGTY_zsmalloc for now, and let the function fail with -EBUSY if already 
registered. The function can have a stub that returns 0 without 
CONFIG_MIGRATION.


The you can do here

int rc;

if (IS_ENABLED(CONFIG_MIGRATION)) {
	rc = register_movable_ops(&zsmalloc_mops, PGTY_zsmalloc);
	if (WARN_ON_ONCE(rc))
		return rc;
}

-- 
Cheers

David / dhildenb

Re: [PATCH] mm/migrate: Fix NULL movable_ops if CONFIG_ZSMALLOC=m

[Huacai Chen]

Hi, David,

On Sat, Aug 16, 2025 at 3:22 PM David Hildenbrand <david@redhat.com> wrote:
>
> On 15.08.25 11:05, Huacai Chen wrote:
>
> Hi,
>
> please CC the appropriate maintainers next time. You missed (some)
> balloon and zsmalloc maintainers.
OK, thanks.

>
> > After commit 84caf98838a3e5f4bdb34 ("mm: stop storing migration_ops in
> > page->mapping") we get such an error message if CONFIG_ZSMALLOC=m:
> >
> >   WARNING: CPU: 3 PID: 42 at mm/migrate.c:142 isolate_movable_ops_page+0xa8/0x1c0
> >   CPU: 3 UID: 0 PID: 42 Comm: kcompactd0 Not tainted 6.16.0-rc5+ #2133 PREEMPT
> >   pc 9000000000540bd8 ra 9000000000540b84 tp 9000000100420000 sp 9000000100423a60
> >   a0 9000000100193a80 a1 000000000000000c a2 000000000000001b a3 ffffffffffffffff
> >   a4 ffffffffffffffff a5 0000000000000267 a6 0000000000000000 a7 9000000100423ae0
> >   t0 00000000000000f1 t1 00000000000000f6 t2 0000000000000000 t3 0000000000000001
> >   t4 ffffff00010eb834 t5 0000000000000040 t6 900000010c89d380 t7 90000000023fcc70
> >   t8 0000000000000018 u0 0000000000000000 s9 ffffff00010eb800 s0 ffffff00010eb800
> >   s1 000000000000000c s2 0000000000043ae0 s3 0000800000000000 s4 900000000219cc40
> >   s5 0000000000000000 s6 ffffff00010eb800 s7 0000000000000001 s8 90000000025b4000
> >      ra: 9000000000540b84 isolate_movable_ops_page+0x54/0x1c0
> >     ERA: 9000000000540bd8 isolate_movable_ops_page+0xa8/0x1c0
> >    CRMD: 000000b0 (PLV0 -IE -DA +PG DACF=CC DACM=CC -WE)
> >    PRMD: 00000004 (PPLV0 +PIE -PWE)
> >    EUEN: 00000000 (-FPE -SXE -ASXE -BTE)
> >    ECFG: 00071c1d (LIE=0,2-4,10-12 VS=7)
> >   ESTAT: 000c0000 [BRK] (IS= ECode=12 EsubCode=0)
> >    PRID: 0014c010 (Loongson-64bit, Loongson-3A5000)
> >   CPU: 3 UID: 0 PID: 42 Comm: kcompactd0 Not tainted 6.16.0-rc5+ #2133 PREEMPT
> >   Stack : 90000000021fd000 0000000000000000 9000000000247720 9000000100420000
> >           90000001004236a0 90000001004236a8 0000000000000000 90000001004237e8
> >           90000001004237e0 90000001004237e0 9000000100423550 0000000000000001
> >           0000000000000001 90000001004236a8 725a84864a19e2d9 90000000023fcc58
> >           9000000100420000 90000000024c6848 9000000002416848 0000000000000001
> >           0000000000000000 000000000000000a 0000000007fe0000 ffffff00010eb800
> >           0000000000000000 90000000021fd000 0000000000000000 900000000205cf30
> >           000000000000008e 0000000000000009 ffffff00010eb800 0000000000000001
> >           90000000025b4000 0000000000000000 900000000024773c 00007ffff103d748
> >           00000000000000b0 0000000000000004 0000000000000000 0000000000071c1d
> >           ...
> >   Call Trace:
> >   [<900000000024773c>] show_stack+0x5c/0x190
> >   [<90000000002415e0>] dump_stack_lvl+0x70/0x9c
> >   [<90000000004abe6c>] isolate_migratepages_block+0x3bc/0x16e0
> >   [<90000000004af408>] compact_zone+0x558/0x1000
> >   [<90000000004b0068>] compact_node+0xa8/0x1e0
> >   [<90000000004b0aa4>] kcompactd+0x394/0x410
> >   [<90000000002b3c98>] kthread+0x128/0x140
> >   [<9000000001779148>] ret_from_kernel_thread+0x28/0xc0
> >   [<9000000000245528>] ret_from_kernel_thread_asm+0x10/0x88
> >
> > The reason is that defined(CONFIG_ZSMALLOC) evaluates to 1 only when
> > CONFIG_ZSMALLOC=y, we should use IS_ENABLED(CONFIG_ZSMALLOC) instead.
>
> Ouch, I missed that CONFIG_ZSMALLOC can be configured like that. I
> thought it would always be builtin.
Make CONFIG_ZSMALLOC be bool can solve this, if you think it is reasonable.

>
> > But when I use IS_ENABLED(CONFIG_ZSMALLOC), page_movable_ops() cannot
> > access zsmalloc_mops because zsmalloc_mops is in a module.
> >
> > To solve this problem, we define a movable_ops[] array in mm/migrate.c,
> > initialise its elements at mm/balloon_compaction.c & mm/zsmalloc.c, and
> > let the page_movable_ops() function return elements from movable_ops[].
>
> Before I took that easy route to just get it working quickly, I
> envisioned a proper registration interface. See below.
When I found I cannot access zsmalloc_mops in a module I considered
the registration interface. But in this case I think that is an
over-design and not straight forward.

Moreover, a registration interface looks like a redesign and not
suitable for hot-fix.


Huacai

>
> >
> > Fixes: 84caf98838a3e5f ("mm: stop storing migration_ops in page->mapping")
> > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > ---
> >   include/linux/migrate.h |  6 ++++++
> >   mm/balloon_compaction.c |  7 +++++++
> >   mm/migrate.c            | 16 ++++++++++------
> >   mm/zsmalloc.c           |  6 ++++++
> >   4 files changed, 29 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/linux/migrate.h b/include/linux/migrate.h
> > index acadd41e0b5c..58fac171e4d5 100644
> > --- a/include/linux/migrate.h
> > +++ b/include/linux/migrate.h
> > @@ -57,6 +57,12 @@ struct movable_operations {
> >       void (*putback_page)(struct page *);
> >   };
> >
> > +#define MOVABLE_BALLOON              0
> > +#define MOVABLE_ZSMALLOC     1
> > +#define MOVABLE_MAX          2
> > +
> > +extern const struct movable_operations *movable_ops[MOVABLE_MAX];
> > +
> >   /* Defined in mm/debug.c: */
> >   extern const char *migrate_reason_names[MR_TYPES];
> >
> > diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
> > index 2a4a649805c1..a1d2625b5c39 100644
> > --- a/mm/balloon_compaction.c
> > +++ b/mm/balloon_compaction.c
> > @@ -254,4 +254,11 @@ const struct movable_operations balloon_mops = {
> >       .putback_page = balloon_page_putback,
> >   };
> >
> > +static int __init balloon_init(void)
> > +{
> > +     movable_ops[MOVABLE_BALLOON] = &balloon_mops;
> > +     return 0;
> > +}
> > +core_initcall(balloon_init);
> > +
> >   #endif /* CONFIG_BALLOON_COMPACTION */
> > diff --git a/mm/migrate.c b/mm/migrate.c
> > index 425401b2d4e1..92918fab3e1a 100644
> > --- a/mm/migrate.c
> > +++ b/mm/migrate.c
> > @@ -53,6 +53,10 @@
> >   #include "internal.h"
> >   #include "swap.h"
> >
> > +const struct movable_operations *movable_ops[MOVABLE_MAX];
> > +
> > +EXPORT_SYMBOL(movable_ops);
> > +
> >   static const struct movable_operations *page_movable_ops(struct page *page)
> >   {
> >       VM_WARN_ON_ONCE_PAGE(!page_has_movable_ops(page), page);
> > @@ -62,15 +66,15 @@ static const struct movable_operations *page_movable_ops(struct page *page)
> >        * it as movable, the page type must be sticky until the page gets freed
> >        * back to the buddy.
> >        */
> > -#ifdef CONFIG_BALLOON_COMPACTION
> > +#if IS_ENABLED(CONFIG_BALLOON_COMPACTION)
> >       if (PageOffline(page))
> >               /* Only balloon compaction sets PageOffline pages movable. */
> > -             return &balloon_mops;
> > -#endif /* CONFIG_BALLOON_COMPACTION */
> > -#if defined(CONFIG_ZSMALLOC) && defined(CONFIG_COMPACTION)
> > +             return movable_ops[MOVABLE_BALLOON];
> > +#endif /* IS_ENABLED(CONFIG_BALLOON_COMPACTION) */
> > +#if IS_ENABLED(CONFIG_ZSMALLOC) && IS_ENABLED(CONFIG_COMPACTION)
> >       if (PageZsmalloc(page))
> > -             return &zsmalloc_mops;
> > -#endif /* defined(CONFIG_ZSMALLOC) && defined(CONFIG_COMPACTION) */
> > +             return movable_ops[MOVABLE_ZSMALLOC];
> > +#endif /* IS_ENABLED(CONFIG_ZSMALLOC) && IS_ENABLED(CONFIG_COMPACTION) */
> >       return NULL;
> >   }
> >
> > diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> > index 2c5e56a65354..052fcc0d29e5 100644
> > --- a/mm/zsmalloc.c
> > +++ b/mm/zsmalloc.c
> > @@ -2246,6 +2246,9 @@ EXPORT_SYMBOL_GPL(zs_destroy_pool);
> >
> >   static int __init zs_init(void)
> >   {
> > +#ifdef CONFIG_MIGRATION
> > +     movable_ops[MOVABLE_ZSMALLOC] = &zsmalloc_mops;
> > +#endif
>
> No, not like this.
>
> Please add a proper registration function, and use the page type as an
> input.
>
> int register_movable_ops(const struct movable_operations *, enum
>                          pagetype);
>
> Probably best to refuse any page_type except PGTY_offline and
> PGTY_zsmalloc for now, and let the function fail with -EBUSY if already
> registered. The function can have a stub that returns 0 without
> CONFIG_MIGRATION.
>
>
> The you can do here
>
> int rc;
>
> if (IS_ENABLED(CONFIG_MIGRATION)) {
>         rc = register_movable_ops(&zsmalloc_mops, PGTY_zsmalloc);
>         if (WARN_ON_ONCE(rc))
>                 return rc;
> }
>
> --
> Cheers
>
> David / dhildenb
>

Re: [PATCH] mm/migrate: Fix NULL movable_ops if CONFIG_ZSMALLOC=m

[David Hildenbrand]

On 16.08.25 10:57, Huacai Chen wrote:
> Hi, David,
> 
> On Sat, Aug 16, 2025 at 3:22 PM David Hildenbrand <david@redhat.com> wrote:
>>
>> On 15.08.25 11:05, Huacai Chen wrote:
>>
>> Hi,
>>
>> please CC the appropriate maintainers next time. You missed (some)
>> balloon and zsmalloc maintainers.
> OK, thanks.
> 
>>
>>> After commit 84caf98838a3e5f4bdb34 ("mm: stop storing migration_ops in
>>> page->mapping") we get such an error message if CONFIG_ZSMALLOC=m:
>>>
>>>    WARNING: CPU: 3 PID: 42 at mm/migrate.c:142 isolate_movable_ops_page+0xa8/0x1c0
>>>    CPU: 3 UID: 0 PID: 42 Comm: kcompactd0 Not tainted 6.16.0-rc5+ #2133 PREEMPT
>>>    pc 9000000000540bd8 ra 9000000000540b84 tp 9000000100420000 sp 9000000100423a60
>>>    a0 9000000100193a80 a1 000000000000000c a2 000000000000001b a3 ffffffffffffffff
>>>    a4 ffffffffffffffff a5 0000000000000267 a6 0000000000000000 a7 9000000100423ae0
>>>    t0 00000000000000f1 t1 00000000000000f6 t2 0000000000000000 t3 0000000000000001
>>>    t4 ffffff00010eb834 t5 0000000000000040 t6 900000010c89d380 t7 90000000023fcc70
>>>    t8 0000000000000018 u0 0000000000000000 s9 ffffff00010eb800 s0 ffffff00010eb800
>>>    s1 000000000000000c s2 0000000000043ae0 s3 0000800000000000 s4 900000000219cc40
>>>    s5 0000000000000000 s6 ffffff00010eb800 s7 0000000000000001 s8 90000000025b4000
>>>       ra: 9000000000540b84 isolate_movable_ops_page+0x54/0x1c0
>>>      ERA: 9000000000540bd8 isolate_movable_ops_page+0xa8/0x1c0
>>>     CRMD: 000000b0 (PLV0 -IE -DA +PG DACF=CC DACM=CC -WE)
>>>     PRMD: 00000004 (PPLV0 +PIE -PWE)
>>>     EUEN: 00000000 (-FPE -SXE -ASXE -BTE)
>>>     ECFG: 00071c1d (LIE=0,2-4,10-12 VS=7)
>>>    ESTAT: 000c0000 [BRK] (IS= ECode=12 EsubCode=0)
>>>     PRID: 0014c010 (Loongson-64bit, Loongson-3A5000)
>>>    CPU: 3 UID: 0 PID: 42 Comm: kcompactd0 Not tainted 6.16.0-rc5+ #2133 PREEMPT
>>>    Stack : 90000000021fd000 0000000000000000 9000000000247720 9000000100420000
>>>            90000001004236a0 90000001004236a8 0000000000000000 90000001004237e8
>>>            90000001004237e0 90000001004237e0 9000000100423550 0000000000000001
>>>            0000000000000001 90000001004236a8 725a84864a19e2d9 90000000023fcc58
>>>            9000000100420000 90000000024c6848 9000000002416848 0000000000000001
>>>            0000000000000000 000000000000000a 0000000007fe0000 ffffff00010eb800
>>>            0000000000000000 90000000021fd000 0000000000000000 900000000205cf30
>>>            000000000000008e 0000000000000009 ffffff00010eb800 0000000000000001
>>>            90000000025b4000 0000000000000000 900000000024773c 00007ffff103d748
>>>            00000000000000b0 0000000000000004 0000000000000000 0000000000071c1d
>>>            ...
>>>    Call Trace:
>>>    [<900000000024773c>] show_stack+0x5c/0x190
>>>    [<90000000002415e0>] dump_stack_lvl+0x70/0x9c
>>>    [<90000000004abe6c>] isolate_migratepages_block+0x3bc/0x16e0
>>>    [<90000000004af408>] compact_zone+0x558/0x1000
>>>    [<90000000004b0068>] compact_node+0xa8/0x1e0
>>>    [<90000000004b0aa4>] kcompactd+0x394/0x410
>>>    [<90000000002b3c98>] kthread+0x128/0x140
>>>    [<9000000001779148>] ret_from_kernel_thread+0x28/0xc0
>>>    [<9000000000245528>] ret_from_kernel_thread_asm+0x10/0x88
>>>
>>> The reason is that defined(CONFIG_ZSMALLOC) evaluates to 1 only when
>>> CONFIG_ZSMALLOC=y, we should use IS_ENABLED(CONFIG_ZSMALLOC) instead.
>>
>> Ouch, I missed that CONFIG_ZSMALLOC can be configured like that. I
>> thought it would always be builtin.
> Make CONFIG_ZSMALLOC be bool can solve this, if you think it is reasonable.
> 
>>
>>> But when I use IS_ENABLED(CONFIG_ZSMALLOC), page_movable_ops() cannot
>>> access zsmalloc_mops because zsmalloc_mops is in a module.
>>>
>>> To solve this problem, we define a movable_ops[] array in mm/migrate.c,
>>> initialise its elements at mm/balloon_compaction.c & mm/zsmalloc.c, and
>>> let the page_movable_ops() function return elements from movable_ops[].
>>
>> Before I took that easy route to just get it working quickly, I
>> envisioned a proper registration interface. See below.
> When I found I cannot access zsmalloc_mops in a module I considered
> the registration interface. But in this case I think that is an
> over-design and not straight forward.
> 
> Moreover, a registration interface looks like a redesign and not
> suitable for hot-fix.

I think you misread my message: This is not debatable.

If you don't want to fix it properly, I can send a fix.

-- 
Cheers

David / dhildenb

Re: [PATCH] mm/migrate: Fix NULL movable_ops if CONFIG_ZSMALLOC=m

[David Hildenbrand]

On 16.08.25 11:01, David Hildenbrand wrote:
> On 16.08.25 10:57, Huacai Chen wrote:
>> Hi, David,
>>
>> On Sat, Aug 16, 2025 at 3:22 PM David Hildenbrand <david@redhat.com> wrote:
>>>
>>> On 15.08.25 11:05, Huacai Chen wrote:
>>>
>>> Hi,
>>>
>>> please CC the appropriate maintainers next time. You missed (some)
>>> balloon and zsmalloc maintainers.
>> OK, thanks.
>>
>>>
>>>> After commit 84caf98838a3e5f4bdb34 ("mm: stop storing migration_ops in
>>>> page->mapping") we get such an error message if CONFIG_ZSMALLOC=m:
>>>>
>>>>     WARNING: CPU: 3 PID: 42 at mm/migrate.c:142 isolate_movable_ops_page+0xa8/0x1c0
>>>>     CPU: 3 UID: 0 PID: 42 Comm: kcompactd0 Not tainted 6.16.0-rc5+ #2133 PREEMPT
>>>>     pc 9000000000540bd8 ra 9000000000540b84 tp 9000000100420000 sp 9000000100423a60
>>>>     a0 9000000100193a80 a1 000000000000000c a2 000000000000001b a3 ffffffffffffffff
>>>>     a4 ffffffffffffffff a5 0000000000000267 a6 0000000000000000 a7 9000000100423ae0
>>>>     t0 00000000000000f1 t1 00000000000000f6 t2 0000000000000000 t3 0000000000000001
>>>>     t4 ffffff00010eb834 t5 0000000000000040 t6 900000010c89d380 t7 90000000023fcc70
>>>>     t8 0000000000000018 u0 0000000000000000 s9 ffffff00010eb800 s0 ffffff00010eb800
>>>>     s1 000000000000000c s2 0000000000043ae0 s3 0000800000000000 s4 900000000219cc40
>>>>     s5 0000000000000000 s6 ffffff00010eb800 s7 0000000000000001 s8 90000000025b4000
>>>>        ra: 9000000000540b84 isolate_movable_ops_page+0x54/0x1c0
>>>>       ERA: 9000000000540bd8 isolate_movable_ops_page+0xa8/0x1c0
>>>>      CRMD: 000000b0 (PLV0 -IE -DA +PG DACF=CC DACM=CC -WE)
>>>>      PRMD: 00000004 (PPLV0 +PIE -PWE)
>>>>      EUEN: 00000000 (-FPE -SXE -ASXE -BTE)
>>>>      ECFG: 00071c1d (LIE=0,2-4,10-12 VS=7)
>>>>     ESTAT: 000c0000 [BRK] (IS= ECode=12 EsubCode=0)
>>>>      PRID: 0014c010 (Loongson-64bit, Loongson-3A5000)
>>>>     CPU: 3 UID: 0 PID: 42 Comm: kcompactd0 Not tainted 6.16.0-rc5+ #2133 PREEMPT
>>>>     Stack : 90000000021fd000 0000000000000000 9000000000247720 9000000100420000
>>>>             90000001004236a0 90000001004236a8 0000000000000000 90000001004237e8
>>>>             90000001004237e0 90000001004237e0 9000000100423550 0000000000000001
>>>>             0000000000000001 90000001004236a8 725a84864a19e2d9 90000000023fcc58
>>>>             9000000100420000 90000000024c6848 9000000002416848 0000000000000001
>>>>             0000000000000000 000000000000000a 0000000007fe0000 ffffff00010eb800
>>>>             0000000000000000 90000000021fd000 0000000000000000 900000000205cf30
>>>>             000000000000008e 0000000000000009 ffffff00010eb800 0000000000000001
>>>>             90000000025b4000 0000000000000000 900000000024773c 00007ffff103d748
>>>>             00000000000000b0 0000000000000004 0000000000000000 0000000000071c1d
>>>>             ...
>>>>     Call Trace:
>>>>     [<900000000024773c>] show_stack+0x5c/0x190
>>>>     [<90000000002415e0>] dump_stack_lvl+0x70/0x9c
>>>>     [<90000000004abe6c>] isolate_migratepages_block+0x3bc/0x16e0
>>>>     [<90000000004af408>] compact_zone+0x558/0x1000
>>>>     [<90000000004b0068>] compact_node+0xa8/0x1e0
>>>>     [<90000000004b0aa4>] kcompactd+0x394/0x410
>>>>     [<90000000002b3c98>] kthread+0x128/0x140
>>>>     [<9000000001779148>] ret_from_kernel_thread+0x28/0xc0
>>>>     [<9000000000245528>] ret_from_kernel_thread_asm+0x10/0x88
>>>>
>>>> The reason is that defined(CONFIG_ZSMALLOC) evaluates to 1 only when
>>>> CONFIG_ZSMALLOC=y, we should use IS_ENABLED(CONFIG_ZSMALLOC) instead.
>>>
>>> Ouch, I missed that CONFIG_ZSMALLOC can be configured like that. I
>>> thought it would always be builtin.
>> Make CONFIG_ZSMALLOC be bool can solve this, if you think it is reasonable.
>>
>>>
>>>> But when I use IS_ENABLED(CONFIG_ZSMALLOC), page_movable_ops() cannot
>>>> access zsmalloc_mops because zsmalloc_mops is in a module.
>>>>
>>>> To solve this problem, we define a movable_ops[] array in mm/migrate.c,
>>>> initialise its elements at mm/balloon_compaction.c & mm/zsmalloc.c, and
>>>> let the page_movable_ops() function return elements from movable_ops[].
>>>
>>> Before I took that easy route to just get it working quickly, I
>>> envisioned a proper registration interface. See below.
>> When I found I cannot access zsmalloc_mops in a module I considered
>> the registration interface. But in this case I think that is an
>> over-design and not straight forward.
>>
>> Moreover, a registration interface looks like a redesign and not
>> suitable for hot-fix.
> 
> I think you misread my message: This is not debatable.
> 
> If you don't want to fix it properly, I can send a fix.

FWIW, something like that on top of your patch.

diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index 58fac171e4d50..4c9b5bcb63ba9 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -57,12 +57,6 @@ struct movable_operations {
  	void (*putback_page)(struct page *);
  };
  
-#define MOVABLE_BALLOON		0
-#define MOVABLE_ZSMALLOC	1
-#define MOVABLE_MAX		2
-
-extern const struct movable_operations *movable_ops[MOVABLE_MAX];
-
  /* Defined in mm/debug.c: */
  extern const char *migrate_reason_names[MR_TYPES];
  
@@ -85,6 +79,10 @@ void migration_entry_wait_on_locked(swp_entry_t entry, spinlock_t *ptl)
  void folio_migrate_flags(struct folio *newfolio, struct folio *folio);
  int folio_migrate_mapping(struct address_space *mapping,
  		struct folio *newfolio, struct folio *folio, int extra_count);
+int register_movable_ops(const struct movable_operations *ops,
+		enum pagetype type);
+void unregister_movable_ops(const struct movable_operations *ops,
+		enum pagetype type);
  
  #else
  
@@ -106,6 +104,16 @@ static inline int migrate_huge_page_move_mapping(struct address_space *mapping,
  {
  	return -ENOSYS;
  }
+static inline int register_movable_ops(const struct movable_operations *ops,
+		enum pagetype type)
+{
+	return -ENOSYS;
+}
+static inline void unregister_movable_ops(const struct movable_operations *ops,
+		enum pagetype type)
+{
+	return -ENOSYS;
+}
  
  #endif /* CONFIG_MIGRATION */
  
diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
index a1d2625b5c391..c391435ce9942 100644
--- a/mm/balloon_compaction.c
+++ b/mm/balloon_compaction.c
@@ -256,8 +256,10 @@ const struct movable_operations balloon_mops = {
  
  static int __init balloon_init(void)
  {
-	movable_ops[MOVABLE_BALLOON] = &balloon_mops;
-	return 0;
+	int rc;
+
+	rc = register_movable_ops(&balloon_mops, PGTY_offline);
+	return rc;
  }
  core_initcall(balloon_init);
  
diff --git a/mm/migrate.c b/mm/migrate.c
index ee65d8787e95b..d69c9f7c27800 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -43,8 +43,6 @@
  #include <linux/sched/sysctl.h>
  #include <linux/memory-tiers.h>
  #include <linux/pagewalk.h>
-#include <linux/balloon_compaction.h>
-#include <linux/zsmalloc.h>
  
  #include <asm/tlbflush.h>
  
@@ -53,9 +51,49 @@
  #include "internal.h"
  #include "swap.h"
  
-const struct movable_operations *movable_ops[MOVABLE_MAX];
+static const struct movable_operations *offline_movable_ops;
+static const struct movable_operations *zsmalloc_movable_ops;
  
-EXPORT_SYMBOL_GPL_FOR_MODULES(movable_ops);
+int register_movable_ops(const struct movable_operations *ops, enum pagetype type)
+{
+	/*
+	 * We only allow for selected types and don't handle concurrent
+	 * registration attempts yet.
+	 */
+	switch (type) {
+	case PGTY_offline:
+		if (offline_movable_ops)
+			return -EBUSY;
+		offline_movable_ops = ops;
+		break;
+	case PGTY_zsmalloc:
+		if (zsmalloc_movable_ops)
+			return -EBUSY;
+		zsmalloc_movable_ops = ops;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(register_movable_ops);
+
+void unregister_movable_ops(const struct movable_operations *ops, enum pagetype type)
+{
+	switch (type) {
+	case PGTY_offline:
+		WARN_ON_ONCE(offline_movable_ops != ops);
+		offline_movable_ops = NULL;
+		break;
+	case PGTY_zsmalloc:
+		WARN_ON_ONCE(zsmalloc_movable_ops != ops);
+		zsmalloc_movable_ops = NULL;
+		break;
+	default:
+		break;
+	}
+}
+EXPORT_SYMBOL_GPL(unregister_movable_ops);
  
  static const struct movable_operations *page_movable_ops(struct page *page)
  {
@@ -66,15 +104,10 @@ static const struct movable_operations *page_movable_ops(struct page *page)
  	 * it as movable, the page type must be sticky until the page gets freed
  	 * back to the buddy.
  	 */
-#if IS_ENABLED(CONFIG_BALLOON_COMPACTION)
  	if (PageOffline(page))
-		/* Only balloon compaction sets PageOffline pages movable. */
-		return movable_ops[MOVABLE_BALLOON];
-#endif /* IS_ENABLED(CONFIG_BALLOON_COMPACTION) */
-#if IS_ENABLED(CONFIG_ZSMALLOC) && IS_ENABLED(CONFIG_COMPACTION)
+		return offline_movable_ops;
  	if (PageZsmalloc(page))
-		return movable_ops[MOVABLE_ZSMALLOC];
-#endif /* IS_ENABLED(CONFIG_ZSMALLOC) && IS_ENABLED(CONFIG_COMPACTION) */
+		return zsmalloc_movable_ops;
  	return NULL;
  }
  
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index ef95cf8c6af90..fcb4f2c599708 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -2246,8 +2246,12 @@ EXPORT_SYMBOL_GPL(zs_destroy_pool);
  
  static int __init zs_init(void)
  {
+	int rc;
+
  #ifdef CONFIG_COMPACTION
-	movable_ops[MOVABLE_ZSMALLOC] = &zsmalloc_mops;
+	rc = register_movable_ops(&zsmalloc_mops, PGTY_zsmalloc);
+	if (rc)
+		return rc;
  #endif
  #ifdef CONFIG_ZPOOL
  	zpool_register_driver(&zs_zpool_driver);
@@ -2259,7 +2263,7 @@ static int __init zs_init(void)
  static void __exit zs_exit(void)
  {
  #ifdef CONFIG_COMPACTION
-	movable_ops[MOVABLE_ZSMALLOC] = NULL;
+	unregister_movable_ops(&zsmalloc_mops, PGTY_zsmalloc);
  #endif
  #ifdef CONFIG_ZPOOL
  	zpool_unregister_driver(&zs_zpool_driver);
-- 
2.50.1


-- 
Cheers

David / dhildenb

Re: [PATCH] mm/migrate: Fix NULL movable_ops if CONFIG_ZSMALLOC=m

[Matthew Wilcox]

On Sat, Aug 16, 2025 at 12:54:52PM +0200, David Hildenbrand wrote:
> +++ b/mm/balloon_compaction.c
> @@ -256,8 +256,10 @@ const struct movable_operations balloon_mops = {
>  static int __init balloon_init(void)
>  {
> -	movable_ops[MOVABLE_BALLOON] = &balloon_mops;
> -	return 0;
> +	int rc;
> +
> +	rc = register_movable_ops(&balloon_mops, PGTY_offline);
> +	return rc;

Using 'rc' as the name of this variable is an anti-pattern.  All it
tells you is "this is the return value".  Calling it 'err' is far
better because now we know it's an error number (or zero for success,
of course).

It seems to be a particularly IBM derived antipattern ;-)
Some internal style guide, perhaps?

> +void unregister_movable_ops(const struct movable_operations *ops, enum pagetype type)
> +{
> +	switch (type) {
> +	case PGTY_offline:
> +		WARN_ON_ONCE(offline_movable_ops != ops);
> +		offline_movable_ops = NULL;
> +		break;
> +	case PGTY_zsmalloc:
> +		WARN_ON_ONCE(zsmalloc_movable_ops != ops);
> +		zsmalloc_movable_ops = NULL;
> +		break;

This might be a bit excessive ... just passing the pagetype and not
having the sanity checks should be enough for the tiny number of users
this interface will have.

Re: [PATCH] mm/migrate: Fix NULL movable_ops if CONFIG_ZSMALLOC=m

[David Hildenbrand]

On 16.08.25 18:23, Matthew Wilcox wrote:
> On Sat, Aug 16, 2025 at 12:54:52PM +0200, David Hildenbrand wrote:
>> +++ b/mm/balloon_compaction.c
>> @@ -256,8 +256,10 @@ const struct movable_operations balloon_mops = {
>>   static int __init balloon_init(void)
>>   {
>> -	movable_ops[MOVABLE_BALLOON] = &balloon_mops;
>> -	return 0;
>> +	int rc;
>> +
>> +	rc = register_movable_ops(&balloon_mops, PGTY_offline);
>> +	return rc;
> 
> Using 'rc' as the name of this variable is an anti-pattern.  All it
> tells you is "this is the return value".  Calling it 'err' is far
> better because now we know it's an error number (or zero for success,
> of course).

I know, we all have our things to complain about. Some about Cc: above 
--, others about the name of error variables :P

$ git grep "int rc" | wc -l
12730
$ git grep "int ret" | wc -l
80386
$ git grep "int error" | wc -l
4349
$ git grep "int err " | wc -l
6117

> 
> It seems to be a particularly IBM derived antipattern ;-)

Careful miser :D

> Some internal style guide, perhaps?

Kernel-internal style guide maybe ;)

> 
>> +void unregister_movable_ops(const struct movable_operations *ops, enum pagetype type)
>> +{
>> +	switch (type) {
>> +	case PGTY_offline:
>> +		WARN_ON_ONCE(offline_movable_ops != ops);
>> +		offline_movable_ops = NULL;
>> +		break;
>> +	case PGTY_zsmalloc:
>> +		WARN_ON_ONCE(zsmalloc_movable_ops != ops);
>> +		zsmalloc_movable_ops = NULL;
>> +		break;
> 
> This might be a bit excessive ... just passing the pagetype and not
> having the sanity checks should be enough for the tiny number of users
> this interface will have.

Yeah, no strong opinion, this was a 3 minute hack.

-- 
Cheers

David / dhildenb

Re: [PATCH] mm/migrate: Fix NULL movable_ops if CONFIG_ZSMALLOC=m

[David Hildenbrand]

On 16.08.25 19:02, David Hildenbrand wrote:
> On 16.08.25 18:23, Matthew Wilcox wrote:
>> On Sat, Aug 16, 2025 at 12:54:52PM +0200, David Hildenbrand wrote:
>>> +++ b/mm/balloon_compaction.c
>>> @@ -256,8 +256,10 @@ const struct movable_operations balloon_mops = {
>>>    static int __init balloon_init(void)
>>>    {
>>> -	movable_ops[MOVABLE_BALLOON] = &balloon_mops;
>>> -	return 0;
>>> +	int rc;
>>> +
>>> +	rc = register_movable_ops(&balloon_mops, PGTY_offline);
>>> +	return rc;
>>
>> Using 'rc' as the name of this variable is an anti-pattern.  All it
>> tells you is "this is the return value".  Calling it 'err' is far
>> better because now we know it's an error number (or zero for success,
>> of course).
> 
> I know, we all have our things to complain about. Some about Cc: above
> --, others about the name of error variables :P
> 
> $ git grep "int rc" | wc -l
> 12730
> $ git grep "int ret" | wc -l
> 80386
> $ git grep "int error" | wc -l
> 4349
> $ git grep "int err " | wc -l
> 6117
> 
>>
>> It seems to be a particularly IBM derived antipattern ;-)
> 
> Careful miser :D

"mister". No idea if "miser" exist and if so, whether it could be 
considered offensive ;)

-- 
Cheers

David / dhildenb

Re: [PATCH] mm/migrate: Fix NULL movable_ops if CONFIG_ZSMALLOC=m

[David Hildenbrand]

> diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
> index a1d2625b5c391..c391435ce9942 100644
> --- a/mm/balloon_compaction.c
> +++ b/mm/balloon_compaction.c
> @@ -256,8 +256,10 @@ const struct movable_operations balloon_mops = {
>    
>    static int __init balloon_init(void)
>    {
> -	movable_ops[MOVABLE_BALLOON] = &balloon_mops;
> -	return 0;
> +	int rc;
> +
> +	rc = register_movable_ops(&balloon_mops, PGTY_offline);
> +	return rc;

Staring again at this hunk, the temp variable is of course not required 
here.

-- 
Cheers

David / dhildenb

Re: [PATCH] mm/migrate: Fix NULL movable_ops if CONFIG_ZSMALLOC=m

[Huacai Chen]

On Sun, Aug 17, 2025 at 1:02 AM David Hildenbrand <david@redhat.com> wrote:
>
> On 16.08.25 18:23, Matthew Wilcox wrote:
> > On Sat, Aug 16, 2025 at 12:54:52PM +0200, David Hildenbrand wrote:
> >> +++ b/mm/balloon_compaction.c
> >> @@ -256,8 +256,10 @@ const struct movable_operations balloon_mops = {
> >>   static int __init balloon_init(void)
> >>   {
> >> -    movable_ops[MOVABLE_BALLOON] = &balloon_mops;
> >> -    return 0;
> >> +    int rc;
> >> +
> >> +    rc = register_movable_ops(&balloon_mops, PGTY_offline);
> >> +    return rc;
> >
> > Using 'rc' as the name of this variable is an anti-pattern.  All it
> > tells you is "this is the return value".  Calling it 'err' is far
> > better because now we know it's an error number (or zero for success,
> > of course).
>
> I know, we all have our things to complain about. Some about Cc: above
> --, others about the name of error variables :P
>
> $ git grep "int rc" | wc -l
> 12730
> $ git grep "int ret" | wc -l
> 80386
> $ git grep "int error" | wc -l
> 4349
> $ git grep "int err " | wc -l
> 6117
>
> >
> > It seems to be a particularly IBM derived antipattern ;-)
>
> Careful miser :D
>
> > Some internal style guide, perhaps?
>
> Kernel-internal style guide maybe ;)
>
> >
> >> +void unregister_movable_ops(const struct movable_operations *ops, enum pagetype type)
> >> +{
> >> +    switch (type) {
> >> +    case PGTY_offline:
> >> +            WARN_ON_ONCE(offline_movable_ops != ops);
> >> +            offline_movable_ops = NULL;
> >> +            break;
> >> +    case PGTY_zsmalloc:
> >> +            WARN_ON_ONCE(zsmalloc_movable_ops != ops);
> >> +            zsmalloc_movable_ops = NULL;
> >> +            break;
> >
> > This might be a bit excessive ... just passing the pagetype and not
> > having the sanity checks should be enough for the tiny number of users
> > this interface will have.
>
> Yeah, no strong opinion, this was a 3 minute hack.
I have tested your code, everything works well. But if the checking is
too excessive, can I use a set_movalbe_ops() to replace both
register_movable_ops() and unregister_movable_ops()? Because register
and unregister are very similar without excessive checking, then we
can pass a meaningful pointer for register and a NULL for unregister
via set_movalbe_ops().

Huacai

>
> --
> Cheers
>
> David / dhildenb
>

Re: [PATCH] mm/migrate: Fix NULL movable_ops if CONFIG_ZSMALLOC=m

[David Hildenbrand]

On 17.08.25 03:37, Huacai Chen wrote:
> On Sun, Aug 17, 2025 at 1:02 AM David Hildenbrand <david@redhat.com> wrote:
>>
>> On 16.08.25 18:23, Matthew Wilcox wrote:
>>> On Sat, Aug 16, 2025 at 12:54:52PM +0200, David Hildenbrand wrote:
>>>> +++ b/mm/balloon_compaction.c
>>>> @@ -256,8 +256,10 @@ const struct movable_operations balloon_mops = {
>>>>    static int __init balloon_init(void)
>>>>    {
>>>> -    movable_ops[MOVABLE_BALLOON] = &balloon_mops;
>>>> -    return 0;
>>>> +    int rc;
>>>> +
>>>> +    rc = register_movable_ops(&balloon_mops, PGTY_offline);
>>>> +    return rc;
>>>
>>> Using 'rc' as the name of this variable is an anti-pattern.  All it
>>> tells you is "this is the return value".  Calling it 'err' is far
>>> better because now we know it's an error number (or zero for success,
>>> of course).
>>
>> I know, we all have our things to complain about. Some about Cc: above
>> --, others about the name of error variables :P
>>
>> $ git grep "int rc" | wc -l
>> 12730
>> $ git grep "int ret" | wc -l
>> 80386
>> $ git grep "int error" | wc -l
>> 4349
>> $ git grep "int err " | wc -l
>> 6117
>>
>>>
>>> It seems to be a particularly IBM derived antipattern ;-)
>>
>> Careful miser :D
>>
>>> Some internal style guide, perhaps?
>>
>> Kernel-internal style guide maybe ;)
>>
>>>
>>>> +void unregister_movable_ops(const struct movable_operations *ops, enum pagetype type)
>>>> +{
>>>> +    switch (type) {
>>>> +    case PGTY_offline:
>>>> +            WARN_ON_ONCE(offline_movable_ops != ops);
>>>> +            offline_movable_ops = NULL;
>>>> +            break;
>>>> +    case PGTY_zsmalloc:
>>>> +            WARN_ON_ONCE(zsmalloc_movable_ops != ops);
>>>> +            zsmalloc_movable_ops = NULL;
>>>> +            break;
>>>
>>> This might be a bit excessive ... just passing the pagetype and not
>>> having the sanity checks should be enough for the tiny number of users
>>> this interface will have.
>>
>> Yeah, no strong opinion, this was a 3 minute hack.
> I have tested your code, everything works well. But if the checking is
> too excessive, can I use a set_movalbe_ops() to replace both
> register_movable_ops() and unregister_movable_ops()? 

Makes sense, given that I don't expect us to have multiple handlers for 
the same type in the near future.

Please do keep a sanity check for replacing valid pointers

if (old_ops && new_ops)
	return -EBUSY;

Thanks!

-- 
Cheers

David / dhildenb

Re: [PATCH] mm/migrate: Fix NULL movable_ops if CONFIG_ZSMALLOC=m

[Huacai Chen]

On Sun, Aug 17, 2025 at 4:05 PM David Hildenbrand <david@redhat.com> wrote:
>
> On 17.08.25 03:37, Huacai Chen wrote:
> > On Sun, Aug 17, 2025 at 1:02 AM David Hildenbrand <david@redhat.com> wrote:
> >>
> >> On 16.08.25 18:23, Matthew Wilcox wrote:
> >>> On Sat, Aug 16, 2025 at 12:54:52PM +0200, David Hildenbrand wrote:
> >>>> +++ b/mm/balloon_compaction.c
> >>>> @@ -256,8 +256,10 @@ const struct movable_operations balloon_mops = {
> >>>>    static int __init balloon_init(void)
> >>>>    {
> >>>> -    movable_ops[MOVABLE_BALLOON] = &balloon_mops;
> >>>> -    return 0;
> >>>> +    int rc;
> >>>> +
> >>>> +    rc = register_movable_ops(&balloon_mops, PGTY_offline);
> >>>> +    return rc;
> >>>
> >>> Using 'rc' as the name of this variable is an anti-pattern.  All it
> >>> tells you is "this is the return value".  Calling it 'err' is far
> >>> better because now we know it's an error number (or zero for success,
> >>> of course).
> >>
> >> I know, we all have our things to complain about. Some about Cc: above
> >> --, others about the name of error variables :P
> >>
> >> $ git grep "int rc" | wc -l
> >> 12730
> >> $ git grep "int ret" | wc -l
> >> 80386
> >> $ git grep "int error" | wc -l
> >> 4349
> >> $ git grep "int err " | wc -l
> >> 6117
> >>
> >>>
> >>> It seems to be a particularly IBM derived antipattern ;-)
> >>
> >> Careful miser :D
> >>
> >>> Some internal style guide, perhaps?
> >>
> >> Kernel-internal style guide maybe ;)
> >>
> >>>
> >>>> +void unregister_movable_ops(const struct movable_operations *ops, enum pagetype type)
> >>>> +{
> >>>> +    switch (type) {
> >>>> +    case PGTY_offline:
> >>>> +            WARN_ON_ONCE(offline_movable_ops != ops);
> >>>> +            offline_movable_ops = NULL;
> >>>> +            break;
> >>>> +    case PGTY_zsmalloc:
> >>>> +            WARN_ON_ONCE(zsmalloc_movable_ops != ops);
> >>>> +            zsmalloc_movable_ops = NULL;
> >>>> +            break;
> >>>
> >>> This might be a bit excessive ... just passing the pagetype and not
> >>> having the sanity checks should be enough for the tiny number of users
> >>> this interface will have.
> >>
> >> Yeah, no strong opinion, this was a 3 minute hack.
> > I have tested your code, everything works well. But if the checking is
> > too excessive, can I use a set_movalbe_ops() to replace both
> > register_movable_ops() and unregister_movable_ops()?
>
> Makes sense, given that I don't expect us to have multiple handlers for
> the same type in the near future.
>
> Please do keep a sanity check for replacing valid pointers
>
> if (old_ops && new_ops)
>         return -EBUSY;
OK, will done in V3 later.

Huacai

>
> Thanks!
>
> --
> Cheers
>
> David / dhildenb
>