linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: SeongJae Park <sj@kernel.org>
To: Wei Yang <richard.weiyang@gmail.com>
Cc: SeongJae Park <sj@kernel.org>,
	akpm@linux-foundation.org, david@redhat.com,
	lorenzo.stoakes@oracle.com, ziy@nvidia.com,
	baolin.wang@linux.alibaba.com, Liam.Howlett@oracle.com,
	npache@redhat.com, ryan.roberts@arm.com, dev.jain@arm.com,
	baohua@kernel.org, lance.yang@linux.dev, xu.xin16@zte.com.cn,
	linux-mm@kvack.org, Kiryl Shutsemau <kirill@shutemov.name>
Subject: Re: [Patch v2 2/2] mm/khugepaged: remove definition of struct khugepaged_mm_slot
Date: Sat, 20 Sep 2025 04:52:33 -0700	[thread overview]
Message-ID: <20250920115233.81851-1-sj@kernel.org> (raw)
In-Reply-To: <20250919071244.17020-3-richard.weiyang@gmail.com>

Hello,

On Fri, 19 Sep 2025 07:12:44 +0000 Wei Yang <richard.weiyang@gmail.com> wrote:

> Current code is not correct to get struct khugepaged_mm_slot by
> mm_slot_entry() without checking mm_slot is !NULL. There is no problem
> reported since slot is the first element of struct khugepaged_mm_slot.
> 
> While struct khugepaged_mm_slot is just a wrapper of struct mm_slot,
> there is no need to define it.
> 
> Remove the definition of struct khugepaged_mm_slot, so there is not
> chance to miss use mm_slot_entry().
> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> Cc: Lance Yang <lance.yang@linux.dev>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Dev Jain <dev.jain@arm.com>
> Cc: Kiryl Shutsemau <kirill@shutemov.name>
> Cc: xu.xin16@zte.com.cn
> ---
>  mm/khugepaged.c | 57 ++++++++++++++++++-------------------------------
>  1 file changed, 21 insertions(+), 36 deletions(-)
> 
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index e019ea2cbab0..88ea92c64bf0 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
[...]
> @@ -2376,7 +2365,6 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result,
>  	__acquires(&khugepaged_mm_lock)
>  {
>  	struct vma_iterator vmi;
> -	struct khugepaged_mm_slot *mm_slot;
>  	struct mm_slot *slot;
>  	struct mm_struct *mm;
>  	struct vm_area_struct *vma;
> @@ -2387,14 +2375,12 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result,
>  	*result = SCAN_FAIL;
>  
>  	if (khugepaged_scan.mm_slot) {
> -		mm_slot = khugepaged_scan.mm_slot;
> -		slot = &mm_slot->slot;
> +		slot = khugepaged_scan.mm_slot;
>  	} else {
>  		slot = list_first_entry(&khugepaged_scan.mm_head,
>  				     struct mm_slot, mm_node);
> -		mm_slot = mm_slot_entry(slot, struct khugepaged_mm_slot, slot);
>  		khugepaged_scan.address = 0;
> -		khugepaged_scan.mm_slot = mm_slot;
> +		khugepaged_scan.mm_slot = slot;
>  	}
>  	spin_unlock(&khugepaged_mm_lock);
>  
> @@ -2492,7 +2478,7 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result,
>  breakouterloop_mmap_lock:
>  
>  	spin_lock(&khugepaged_mm_lock);
> -	VM_BUG_ON(khugepaged_scan.mm_slot != mm_slot);
> +	VM_BUG_ON(khugepaged_scan.mm_slot != slot);
>  	/*
>  	 * Release the current mm_slot if this mm is about to die, or
>  	 * if we scanned all vmas of this mm.
> @@ -2505,15 +2491,14 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result,
>  		 */
>  		if (!list_is_last(&slot->mm_node, &khugepaged_scan.mm_head)) {
>  			slot = list_next_entry(slot, mm_node);
> -			khugepaged_scan.mm_slot =
> -				mm_slot_entry(slot, struct khugepaged_mm_slot, slot);
> +			khugepaged_scan.mm_slot = slot;
>  			khugepaged_scan.address = 0;
>  		} else {
>  			khugepaged_scan.mm_slot = NULL;
>  			khugepaged_full_scans++;
>  		}
>  
> -		collect_mm_slot(mm_slot);
> +		collect_mm_slot(slot);
>  	}
>  
>  	return progress;
> @@ -2600,7 +2585,7 @@ static void khugepaged_wait_work(void)
>  
>  static int khugepaged(void *none)
>  {
> -	struct khugepaged_mm_slot *mm_slot;
> +	struct mm_slot *slot;
>  
>  	set_freezable();
>  	set_user_nice(current, MAX_NICE);
> @@ -2611,10 +2596,10 @@ static int khugepaged(void *none)
>  	}
>  
>  	spin_lock(&khugepaged_mm_lock);
> -	mm_slot = khugepaged_scan.mm_slot;
> +	slot = khugepaged_scan.mm_slot;
>  	khugepaged_scan.mm_slot = NULL;
> -	if (mm_slot)
> -		collect_mm_slot(mm_slot);
> +	if (slot)
> +		collect_mm_slot(slot);
>  	spin_unlock(&khugepaged_mm_lock);
>  	return 0;
>  }
> -- 
> 2.34.1
> 
> 
> 

On latest mm-new tree, I am getting below error while building UML mode kernel
for kunit.  And 'git bisect' points me this patch.  I'm not familiar with this
code and have no time to dive deep for now, so reporting first.

Oops: general protection fault, probably for non-canonical adI
[  356.456907] CPU: 34 UID: 0 PID: 309 Comm: khugepaged Not tainted 6.17.0-rc4+ #370 PREEMPT(voluntary)
[  356.457702] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-4.el9 04/01/2014
[  356.458484] RIP: 0010:collect_mm_slot (mm/khugepaged.c:1427)
[  356.458904] Code: 48 89 df 5b e9 1a 29 f3 ff 66 2e 0f 1f 84 00 00 00 00 00 90 90 90 90 90 90 90 90 90 90 908

Code starting with the faulting instruction
===========================================
   0:   48 89 df                mov    %rbx,%rdi
   3:   5b                      pop    %rbx
   4:   e9 1a 29 f3 ff          jmp    0xfffffffffff32923
   9:   66 2e 0f 1f 84 00 00    cs nopw 0x0(%rax,%rax,1)
  10:   00 00 00
  13:   90                      nop
  14:   90                      nop
  15:   90                      nop
  16:   90                      nop
  17:   90                      nop
  18:   90                      nop
  19:   90                      nop
  1a:   90                      nop
  1b:   90                      nop
  1c:   90                      nop
  1d:   08                      .byte 0x8
[  356.460685] RSP: 0018:ffffb61a46e37df8 EFLAGS: 00010286
[  356.461115] RAX: e1bca96613f6fe2b RBX: 0000000000000000 RCX: 8000000000000007
[  356.461692] RDX: 0000000000000001 RSI: ffffeba0443b2600 RDI: e1bca96613f6fe2b
[  356.462269] RBP: 00000000000000f2 R08: ffff8ea80ec9aa00 R09: 0000000080150001
[  356.462842] R10: 000000008015000e R11: 0000000000000000 R12: ffff8ea80ec9aa00
[  356.463574] R13: 00000000000001e5 R14: 0000000000000001 R15: ffffb61a46e37e60
[  356.464249] FS:  0000000000000000(0000) GS:ffff8eaf13dd1000(0000) knlGS:0000000000000000
[  356.465070] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  356.465578] CR2: 00007f8e913e2a1c CR3: 00000008cb022000 CR4: 00000000000006f0
[  356.466185] Call Trace:
[  356.466398]  <TASK>
[  356.466576]  khugepaged (mm/khugepaged.c:2519 mm/khugepaged.c:2556 mm/khugepaged.c:2612)
[  356.466869]  ? __pfx_khugepaged (mm/khugepaged.c:2605)
[  356.467284]  kthread (kernel/kthread.c:463)
[  356.467592]  ? finish_task_switch.isra.0 (arch/x86/include/asm/paravirt.h:671 kernel/sched/sched.h:1531 kernel/sched/core.c:5105 kernel/sched/core.c:5223)
[  356.468068]  ? __pfx_kthread (kernel/kthread.c:412)
[  356.468480]  ret_from_fork (arch/x86/kernel/process.c:154)
[  356.468849]  ? __pfx_kthread (kernel/kthread.c:412)
[  356.469223]  ret_from_fork_asm (arch/x86/entry/entry_64.S:258)
[  356.469591]  </TASK>
[  356.469778] Modules linked in: binfmt_misc ppdev parport_pc parport pcspkr evdev joydev button serio_raw sgn
[  356.473304] Dumping ftrace buffer:
[  356.473618]    (ftrace buffer empty)
[  356.473966] ---[ end trace 0000000000000000 ]---
[  356.474506] RIP: 0010:collect_mm_slot (mm/khugepaged.c:1427)
[  356.475142] Code: 48 89 df 5b e9 1a 29 f3 ff 66 2e 0f 1f 84 00 00 00 00 00 90 90 90 90 90 90 90 90 90 90 908

Code starting with the faulting instruction
===========================================
   0:   48 89 df                mov    %rbx,%rdi
   3:   5b                      pop    %rbx
   4:   e9 1a 29 f3 ff          jmp    0xfffffffffff32923
   9:   66 2e 0f 1f 84 00 00    cs nopw 0x0(%rax,%rax,1)
  10:   00 00 00
  13:   90                      nop
  14:   90                      nop
  15:   90                      nop
  16:   90                      nop
  17:   90                      nop
  18:   90                      nop
  19:   90                      nop
  1a:   90                      nop
  1b:   90                      nop
  1c:   90                      nop
  1d:   08                      .byte 0x8
[  356.478405] RSP: 0018:ffffb61a46e37df8 EFLAGS: 00010286
[  356.478935] RAX: e1bca96613f6fe2b RBX: 0000000000000000 RCX: 8000000000000007
[  356.479763] RDX: 0000000000000001 RSI: ffffeba0443b2600 RDI: e1bca96613f6fe2b
[  356.480722] RBP: 00000000000000f2 R08: ffff8ea80ec9aa00 R09: 0000000080150001
[  356.481703] R10: 000000008015000e R11: 0000000000000000 R12: ffff8ea80ec9aa00
[  356.482402] R13: 00000000000001e5 R14: 0000000000000001 R15: ffffb61a46e37e60
[  356.483060] FS:  0000000000000000(0000) GS:ffff8eaf13dd1000(0000) knlGS:0000000000000000
[  356.484027] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  356.484861] CR2: 00007f8e913e2a1c CR3: 00000008cb022000 CR4: 00000000000006f0
[  356.485559] note: khugepaged[309] exited with preempt_count 1


Thanks,
SJ


  parent reply	other threads:[~2025-09-20 11:52 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-19  7:12 [Patch v2 0/2] mm_slot: fix the usage of mm_slot_entry Wei Yang
2025-09-19  7:12 ` [Patch v2 1/2] mm/ksm: get mm_slot by mm_slot_entry() when slot is !NULL Wei Yang
2025-09-19  7:24   ` David Hildenbrand
2025-09-19  7:38   ` Dev Jain
2025-09-19  7:44   ` Lance Yang
2025-09-19  7:12 ` [Patch v2 2/2] mm/khugepaged: remove definition of struct khugepaged_mm_slot Wei Yang
2025-09-19  7:36   ` David Hildenbrand
2025-09-22 13:17     ` Nico Pache
2025-09-20 11:52   ` SeongJae Park [this message]
2025-09-20 12:29     ` Wei Yang
2025-09-20 13:41       ` SeongJae Park
2025-09-21 15:08         ` Wei Yang
2025-09-22  9:33           ` SeongJae Park
2025-09-21 16:07     ` Lance Yang
2025-09-22  0:28       ` Wei Yang
2025-09-22  9:37         ` SeongJae Park

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=20250920115233.81851-1-sj@kernel.org \
    --to=sj@kernel.org \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=baohua@kernel.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=david@redhat.com \
    --cc=dev.jain@arm.com \
    --cc=kirill@shutemov.name \
    --cc=lance.yang@linux.dev \
    --cc=linux-mm@kvack.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=npache@redhat.com \
    --cc=richard.weiyang@gmail.com \
    --cc=ryan.roberts@arm.com \
    --cc=xu.xin16@zte.com.cn \
    --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