linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: David Hildenbrand <david@redhat.com>
To: Huacai Chen <chenhuacai@kernel.org>
Cc: Huacai Chen <chenhuacai@loongson.cn>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, Zi Yan <ziy@nvidia.com>,
	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>,
	Harry Yoo <harry.yoo@oracle.com>,
	linux-kernel@vger.kernel.org, Minchan Kim <minchan@kernel.org>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	"Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [PATCH] mm/migrate: Fix NULL movable_ops if CONFIG_ZSMALLOC=m
Date: Sat, 16 Aug 2025 11:01:47 +0200	[thread overview]
Message-ID: <66ebc632-6704-4637-b62d-1cb11e5a4782@redhat.com> (raw)
In-Reply-To: <CAAhV-H470h2HDEN_NY2qNBxUqQrSRQhLzwqZe9PB8GjnNsZVFQ@mail.gmail.com>

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



  reply	other threads:[~2025-08-16  9:01 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-15  9:05 Huacai Chen
2025-08-15 20:57 ` Zi Yan
2025-08-16  0:57 ` kernel test robot
2025-08-16  7:22 ` David Hildenbrand
2025-08-16  8:57   ` Huacai Chen
2025-08-16  9:01     ` David Hildenbrand [this message]
2025-08-16 10:54       ` David Hildenbrand
2025-08-16 16:23         ` Matthew Wilcox
2025-08-16 17:02           ` David Hildenbrand
2025-08-16 17:04             ` David Hildenbrand
2025-08-17  1:37             ` Huacai Chen
2025-08-17  8:04               ` David Hildenbrand
2025-08-17  8:34                 ` Huacai Chen
2025-08-16 17:13         ` David Hildenbrand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=66ebc632-6704-4637-b62d-1cb11e5a4782@redhat.com \
    --to=david@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=chenhuacai@kernel.org \
    --cc=chenhuacai@loongson.cn \
    --cc=harry.yoo@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=minchan@kernel.org \
    --cc=mst@redhat.com \
    --cc=senozhatsky@chromium.org \
    --cc=ziy@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox