| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAAhV-H5gtJD1Krz=EQ1T-=g9DpANkuuiV6wUcjrquYpU3hbrPA@mail.gmail.com>
Date: Sun, 17 Aug 2025 16:37:12 +0800
From: Huacai Chen <chenhuacai@...nel.org>
To: Huacai Chen <chenhuacai@...ngson.cn>
Cc: Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
David Hildenbrand <david@...hat.com>, Zi Yan <ziy@...dia.com>,
Lorenzo Stoakes <lorenzo.stoakes@...cle.com>, Harry Yoo <harry.yoo@...cle.com>,
Minchan Kim <minchan@...nel.org>, Sergey Senozhatsky <senozhatsky@...omium.org>,
"Michael S . Tsirkin" <mst@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V3] mm/migrate: Fix NULL movable_ops if CONFIG_ZSMALLOC=m
Sorry for my stupid, I forgot to update the commit message. :(
Huacai
On Sun, Aug 17, 2025 at 4:36 PM Huacai Chen <chenhuacai@...ngson.cn> 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")
> Acked-by: Zi Yan <ziy@...dia.com>
> Signed-off-by: Huacai Chen <chenhuacai@...ngson.cn>
> ---
> V2: Use EXPORT_SYMBOL_GPL_FOR_MODULES instead of EXPORT_SYMBOL and fix build.
> V3: Use register interface instead of set array directly (Thank David).
>
> include/linux/migrate.h | 5 +++++
> mm/balloon_compaction.c | 6 ++++++
> mm/migrate.c | 38 ++++++++++++++++++++++++++++++--------
> mm/zsmalloc.c | 10 ++++++++++
> 4 files changed, 51 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/migrate.h b/include/linux/migrate.h
> index acadd41e0b5c..9009e27b5f44 100644
> --- a/include/linux/migrate.h
> +++ b/include/linux/migrate.h
> @@ -79,6 +79,7 @@ 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 set_movable_ops(const struct movable_operations *ops, enum pagetype type);
>
> #else
>
> @@ -100,6 +101,10 @@ static inline int migrate_huge_page_move_mapping(struct address_space *mapping,
> {
> return -ENOSYS;
> }
> +static inline int set_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 2a4a649805c1..03c5dbabb156 100644
> --- a/mm/balloon_compaction.c
> +++ b/mm/balloon_compaction.c
> @@ -254,4 +254,10 @@ const struct movable_operations balloon_mops = {
> .putback_page = balloon_page_putback,
> };
>
> +static int __init balloon_init(void)
> +{
> + return set_movable_ops(&balloon_mops, PGTY_offline);
> +}
> +core_initcall(balloon_init);
> +
> #endif /* CONFIG_BALLOON_COMPACTION */
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 425401b2d4e1..9e5ef39ce73a 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,6 +51,33 @@
> #include "internal.h"
> #include "swap.h"
>
> +static const struct movable_operations *offline_movable_ops;
> +static const struct movable_operations *zsmalloc_movable_ops;
> +
> +int set_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 && ops)
> + return -EBUSY;
> + offline_movable_ops = ops;
> + break;
> + case PGTY_zsmalloc:
> + if (zsmalloc_movable_ops && ops)
> + return -EBUSY;
> + zsmalloc_movable_ops = ops;
> + break;
> + default:
> + return -EINVAL;
> + }
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(set_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 +87,12 @@ 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 (PageOffline(page))
> /* Only balloon compaction sets PageOffline pages movable. */
> - return &balloon_mops;
> -#endif /* CONFIG_BALLOON_COMPACTION */
> -#if defined(CONFIG_ZSMALLOC) && defined(CONFIG_COMPACTION)
> + return offline_movable_ops;
> if (PageZsmalloc(page))
> - return &zsmalloc_mops;
> -#endif /* defined(CONFIG_ZSMALLOC) && defined(CONFIG_COMPACTION) */
> + return zsmalloc_movable_ops;
> +
> return NULL;
> }
>
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index 2c5e56a65354..6c574ab8abff 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -2246,8 +2246,15 @@ EXPORT_SYMBOL_GPL(zs_destroy_pool);
>
> static int __init zs_init(void)
> {
> + int rc;
> +
> #ifdef CONFIG_ZPOOL
> zpool_register_driver(&zs_zpool_driver);
> +#endif
> +#ifdef CONFIG_COMPACTION
> + rc = set_movable_ops(&zsmalloc_mops, PGTY_zsmalloc);
> + if (rc)
> + return rc;
> #endif
> zs_stat_init();
> return 0;
> @@ -2257,6 +2264,9 @@ static void __exit zs_exit(void)
> {
> #ifdef CONFIG_ZPOOL
> zpool_unregister_driver(&zs_zpool_driver);
> +#endif
> +#ifdef CONFIG_COMPACTION
> + set_movable_ops(NULL, PGTY_zsmalloc);
> #endif
> zs_stat_exit();
> }
> --
> 2.47.3
>
Powered by blists - more mailing lists