| 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: <9db98f7f-b90f-464b-ae7f-e94ac523bc28@redhat.com>
Date: Sat, 16 Aug 2025 12:54:52 +0200
From: David Hildenbrand <david@...hat.com>
To: Huacai Chen <chenhuacai@...nel.org>
Cc: Huacai Chen <chenhuacai@...ngson.cn>,
Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
Zi Yan <ziy@...dia.com>, Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
Harry Yoo <harry.yoo@...cle.com>, linux-kernel@...r.kernel.org,
Minchan Kim <minchan@...nel.org>,
Sergey Senozhatsky <senozhatsky@...omium.org>,
"Michael S. Tsirkin" <mst@...hat.com>
Subject: Re: [PATCH] mm/migrate: Fix NULL movable_ops if CONFIG_ZSMALLOC=m
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@...hat.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
Powered by blists - more mailing lists