From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
To: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: akpm@linux-foundation.org, david@kernel.org, hughd@google.com,
ziy@nvidia.com, liam@infradead.org, nico.pache@linux.dev,
dev.jain@arm.com, ryan.roberts@arm.com, baohua@kernel.org,
lance.yang@linux.dev, usama.arif@linux.dev, linux-mm@kvack.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm: shmem: fix incorrect vm_flags usage when checking allowable orders
Date: Mon, 17 Aug 2026 08:40:52 +0100 [thread overview]
Message-ID: <aoK3JcBYe_JuydGm@lucifer> (raw)
In-Reply-To: <ed34ca03ae7d65e89467fb87bc961f5497049c00.1786948410.git.baolin.wang@linux.alibaba.com>
On Mon, Aug 17, 2026 at 03:16:43PM +0800, Baolin Wang wrote:
> Lance reported that when MADV_HUGEPAGE is set on a tmpfs file mounted with
> huge=advise option, khugepaged fails the allowable order check and does not
> scan the tmpfs file for collapse.
Ugh.
But really I think this isn't quite accurate - what you mean to say I think is
that when _nothing else_ causes the mm to be considered for khugepaged collapse,
an MADV_HUGEPAGE-advised tmpfs VMA alone does not cause scanning to commence.
>
> After commit 6beeab870e70 ("mm: shmem: move shmem_huge_global_enabled() into
> shmem_allowable_huge_orders()"), the shmem/tmpfs allowable order check reads
> vma->vm_flags directly. However, when MADV_HUGEPAGE is handled,
> khugepaged_enter_vma() is called before the VMA's vm_flags have been updated,
> so the check uses stale flags and incorrectly rejects the VMA for collapse.
> As a result, khugepaged does not collapse the tmpfs file into PMD order in time.
Could we at least refer to the non-deprecated field in the commit message?
i.e. vma->flags.
Probably worth mentioning VMA_HUGEPAGE_BIT also.
>
> Fix this by passing vm_flags as a parameter to shmem_allowable_huge_orders()
> instead of reading it from the vm_area_struct.
Ugh this is so disgusting.
I understand this is a fix for a bug to be backported but couldn't we just
achieve the same without having to add a deprecated field to be passed around?
As you say the khugepaged_enter_vma() isn't really so helpful in
hugepage_madvise().
But you could add this to the bottom of madvise_update_vma():
if (vma_flags_test(&new_vma_flags, VMA_HUGEPAGE_BIT))
khugepaged_enter_vma(vma, new_flags);
I don't think this is really egregious on this code path and could go some way
towards us eliminating the silly thing of passing around flags-to-be-set.
And yeah you'd endure a little backport pain on switching that new_vma_flags
vma_flags_t test to a new_flags vm_flags_t test but it's not so bad :)
I will be doing a series soon that improves the allowable orders vomit-a-thon
somewhat in tip :)
>
> Reported-by: Lance Yang <lance.yang@linux.dev>
> Closes: https://lore.kernel.org/all/20260815181632.21453-1-lance.yang@linux.dev/
> Fixes: 6beeab870e70 ("mm: shmem: move shmem_huge_global_enabled() into shmem_allowable_huge_orders()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> ---
> include/linux/shmem_fs.h | 8 ++++----
> mm/huge_memory.c | 2 +-
> mm/shmem.c | 9 +++++----
> 3 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/shmem_fs.h b/include/linux/shmem_fs.h
> index 5663dff53186..321017e0fd63 100644
> --- a/include/linux/shmem_fs.h
> +++ b/include/linux/shmem_fs.h
> @@ -127,13 +127,13 @@ int shmem_unuse(unsigned int type);
>
> #if defined(CONFIG_TRANSPARENT_HUGEPAGE) && defined(CONFIG_SHMEM)
> unsigned long shmem_allowable_huge_orders(struct inode *inode,
> - struct vm_area_struct *vma, pgoff_t index,
> - loff_t write_end, bool shmem_huge_force);
> + struct vm_area_struct *vma, vm_flags_t vm_flags,
> + pgoff_t index, loff_t write_end, bool shmem_huge_force);
> bool shmem_hpage_pmd_enabled(void);
> #else
> static inline unsigned long shmem_allowable_huge_orders(struct inode *inode,
> - struct vm_area_struct *vma, pgoff_t index,
> - loff_t write_end, bool shmem_huge_force)
> + struct vm_area_struct *vma, vm_flags_t vm_flags, pgoff_t index,
> + loff_t write_end, bool shmem_huge_force)
> {
> return 0;
> }
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index ced400f72d43..70f57d700739 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -187,7 +187,7 @@ unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
> */
> if (!in_pf && shmem_file(vma->vm_file))
> return orders & shmem_allowable_huge_orders(file_inode(vma->vm_file),
> - vma, vma_start_pgoff(vma), 0,
> + vma, vm_flags, vma_start_pgoff(vma), 0,
> forced_collapse);
>
> if (!vma_is_anonymous(vma)) {
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 599665a3d6e7..6f1ad5456aca 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1834,12 +1834,11 @@ bool shmem_hpage_pmd_enabled(void)
> }
>
> unsigned long shmem_allowable_huge_orders(struct inode *inode,
> - struct vm_area_struct *vma, pgoff_t index,
> - loff_t write_end, bool shmem_huge_force)
> + struct vm_area_struct *vma, vm_flags_t vm_flags,
> + pgoff_t index, loff_t write_end, bool shmem_huge_force)
> {
> unsigned long mask = READ_ONCE(huge_shmem_orders_always);
> unsigned long within_size_orders = READ_ONCE(huge_shmem_orders_within_size);
> - vm_flags_t vm_flags = vma ? vma->vm_flags : 0;
> unsigned int global_orders;
>
> if (thp_disabled_by_hw() || (vma && vma_thp_disabled(vma, vm_flags, shmem_huge_force)))
> @@ -2430,6 +2429,7 @@ static int shmem_get_folio_gfp(struct inode *inode, pgoff_t index,
> gfp_t gfp, struct vm_fault *vmf, vm_fault_t *fault_type)
> {
> struct vm_area_struct *vma = vmf ? vmf->vma : NULL;
> + vm_flags_t vm_flags = vma ? vma->vm_flags : 0;
> struct mm_struct *fault_mm;
> struct folio *folio;
> int error;
> @@ -2507,7 +2507,8 @@ static int shmem_get_folio_gfp(struct inode *inode, pgoff_t index,
> }
>
> /* Find hugepage orders that are allowed for anonymous shmem and tmpfs. */
> - orders = shmem_allowable_huge_orders(inode, vma, index, write_end, false);
> + orders = shmem_allowable_huge_orders(inode, vma, vm_flags, index,
> + write_end, false);
> if (orders > 0) {
> gfp_t huge_gfp;
>
> --
> 2.47.3
>
--
Cheers, Lorenzo
next prev parent reply other threads:[~2026-08-17 7:41 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-17 7:16 [PATCH] mm: shmem: fix incorrect vm_flags usage when checking allowable orders Baolin Wang
2026-08-17 7:39 ` Lance Yang
2026-08-17 7:40 ` Lorenzo Stoakes (ARM) [this message]
2026-08-17 9:34 ` Baolin Wang
2026-08-17 10:54 ` Baolin Wang
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=aoK3JcBYe_JuydGm@lucifer \
--to=ljs@kernel.org \
--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=lance.yang@linux.dev \
--cc=liam@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=nico.pache@linux.dev \
--cc=ryan.roberts@arm.com \
--cc=usama.arif@linux.dev \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.