linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Harry Yoo <harry.yoo@oracle.com>
To: "David Hildenbrand (Red Hat)" <david@kernel.org>
Cc: stable@vger.kernel.org, Liam.Howlett@oracle.com,
	akpm@linux-foundation.org, baohua@kernel.org,
	baolin.wang@linux.alibaba.com, dev.jain@arm.com,
	hughd@google.com, jane.chu@oracle.com, jannh@google.com,
	kas@kernel.org, lance.yang@linux.dev, linux-mm@kvack.org,
	lorenzo.stoakes@oracle.com, npache@redhat.com, pfalcato@suse.de,
	ryan.roberts@arm.com, vbabka@suse.cz, ziy@nvidia.com
Subject: Re: [PATCH V1 6.1.y 0/2] Fix bad pmd due to race between change_prot_numa() and THP migration
Date: Thu, 20 Nov 2025 09:38:00 +0900	[thread overview]
Message-ID: <aR5i6PdVskmgK-gL@hyeyoo> (raw)
In-Reply-To: <ac8d7137-3819-4a75-9dd3-fb3d2259ebe4@kernel.org>

On Fri, Nov 14, 2025 at 12:06:57PM +0100, David Hildenbrand (Red Hat) wrote:
> On 11.11.25 08:10, Harry Yoo wrote:
> > # TL;DR
> > 
> > previous discussion: https://lore.kernel.org/linux-mm/b41ea29e-6b48-4f64-859c-73be095453ae@redhat.com/
> > 
> > A "bad pmd" error occurs due to race condition between
> > change_prot_numa() and THP migration. The mainline kernel does not have
> > this bug as commit 670ddd8cdc fixes the race condition. 6.1.y, 5.15.y,
> > 5.10.y, 5.4.y are affected by this bug.
> > 
> > Fixing this in -stable kernels is tricky because pte_map_offset_lock()
> > has different semantics in pre-6.5 and post-6.5 kernels. I am trying to
> > backport the same mechanism we have in the mainline kernel.
> > Since the code looks bit different due to different semantics of
> > pte_map_offset_lock(), it'd be best to get this reviewed by MM folks.
> > 
> > # Testing
> > 
> > I verified that the bug described below is not reproduced anymore
> > (on a downstream kernel) after applying this patch series. It used to
> > trigger in few days of intensive numa balancing testing, but it survived
> > 2 weeks with this applied.
> > 
> > # Bug Description
> > 
> > It was reported that a bad pmd is seen when automatic NUMA
> > balancing is marking page table entries as prot_numa:
> >    [2437548.196018] mm/pgtable-generic.c:50: bad pmd 00000000af22fc02(dffffffe71fbfe02)
> >    [2437548.235022] Call Trace:
> >    [2437548.238234]  <TASK>
> >    [2437548.241060]  dump_stack_lvl+0x46/0x61
> >    [2437548.245689]  panic+0x106/0x2e5
> >    [2437548.249497]  pmd_clear_bad+0x3c/0x3c
> >    [2437548.253967]  change_pmd_range.isra.0+0x34d/0x3a7
> >    [2437548.259537]  change_p4d_range+0x156/0x20e
> >    [2437548.264392]  change_protection_range+0x116/0x1a9
> >    [2437548.269976]  change_prot_numa+0x15/0x37
> >    [2437548.274774]  task_numa_work+0x1b8/0x302
> >    [2437548.279512]  task_work_run+0x62/0x95
> >    [2437548.283882]  exit_to_user_mode_loop+0x1a4/0x1a9
> >    [2437548.289277]  exit_to_user_mode_prepare+0xf4/0xfc
> >    [2437548.294751]  ? sysvec_apic_timer_interrupt+0x34/0x81
> >    [2437548.300677]  irqentry_exit_to_user_mode+0x5/0x25
> >    [2437548.306153]  asm_sysvec_apic_timer_interrupt+0x16/0x1b
> > 
> > This is due to a race condition between change_prot_numa() and
> > THP migration because the kernel doesn't check is_swap_pmd() and
> > pmd_trans_huge() atomically:
> > 
> > change_prot_numa()                      THP migration
> > ======================================================================
> > - change_pmd_range()
> > -> is_swap_pmd() returns false,
> > meaning it's not a PMD migration
> > entry.
> > 				  - do_huge_pmd_numa_page()
> > 				  -> migrate_misplaced_page() sets
> > 				     migration entries for the THP.
> > - change_pmd_range()
> > -> pmd_none_or_clear_bad_unless_trans_huge()
> > -> pmd_none() and pmd_trans_huge() returns false
> > - pmd_none_or_clear_bad_unless_trans_huge()
> > -> pmd_bad() returns true for the migration entry!
> > 
> > The upstream commit 670ddd8cdcbd ("mm/mprotect: delete
> > pmd_none_or_clear_bad_unless_trans_huge()") closes this race condition
> > by checking is_swap_pmd() and pmd_trans_huge() atomically.
> > 
> > # Backporting note
> > 
> > commit a79390f5d6a7 ("mm/mprotect: use long for page accountings and retval")
> > is backported to return an error code (negative value) in
> > change_pte_range().
> > 
> > Unlike the mainline, pte_offset_map_lock() does not check if the pmd
> > entry is a migration entry or a hugepage; acquires PTL unconditionally
> > instead of returning failure. Therefore, it is necessary to keep the
> > !is_swap_pmd() && !pmd_trans_huge() && !pmd_devmap() checks in
> > change_pmd_range() before acquiring the PTL.
> > 
> > After acquiring the lock, open-code the semantics of
> > pte_offset_map_lock() in the mainline kernel; change_pte_range() fails
> > if the pmd value has changed. This requires adding pmd_old parameter
> > (pmd_t value that is read before calling the function) to
> > change_pte_range().
> 
> Looks reasonable to me, so I assume the backporting diff makes sense.
> 
> Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>

Hi David, just wanted to say that I really appreciate your
acknowledgement. Thanks!

While this bug doesn't appear to cause severe damage on the system
(only a "bad pmd" error printed to console due to race), that wasn't
really clear before closer investigation and I think it is worth
backporting to save others' time.

I'll send v5.15, v5.10, v5.4 fix soon that does the same thing.

-- 
Cheers,
Harry / Hyeonggon


      reply	other threads:[~2025-11-20  0:38 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-11  7:10 Harry Yoo
2025-11-11  7:11 ` [PATCH V1 6.1.y 1/2] mm/mprotect: use long for page accountings and retval Harry Yoo
2025-11-11  7:11 ` [PATCH V1 6.1.y 2/2] mm/mprotect: delete pmd_none_or_clear_bad_unless_trans_huge() Harry Yoo
2025-11-17 14:10   ` Sasha Levin
2025-11-14 11:06 ` [PATCH V1 6.1.y 0/2] Fix bad pmd due to race between change_prot_numa() and THP migration David Hildenbrand (Red Hat)
2025-11-20  0:38   ` Harry Yoo [this message]

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=aR5i6PdVskmgK-gL@hyeyoo \
    --to=harry.yoo@oracle.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=baohua@kernel.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=david@kernel.org \
    --cc=dev.jain@arm.com \
    --cc=hughd@google.com \
    --cc=jane.chu@oracle.com \
    --cc=jannh@google.com \
    --cc=kas@kernel.org \
    --cc=lance.yang@linux.dev \
    --cc=linux-mm@kvack.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=npache@redhat.com \
    --cc=pfalcato@suse.de \
    --cc=ryan.roberts@arm.com \
    --cc=stable@vger.kernel.org \
    --cc=vbabka@suse.cz \
    --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