From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 305572EEE99 for ; Mon, 17 Aug 2026 07:41:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786952473; cv=none; b=Yrgxmvu+vGc18Rd40KIhJhYbT4TPJt645Wh8YM+kQlDnSfGcN4ZtIbIKvvxlzh1mdLlCG7+d7+Ts7/wGN4ivHEe8C7kcBORAGmocHp499zeHn7K+yXwtd0z4AopakD5qIqHA8ZMuGhqvoLM8/OTCN5E8YCyigrsn9IkISb1IKuw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786952473; c=relaxed/simple; bh=9tYaKuMV++R42E/fZvH2ARs6NPDYiVooHjUMnUw0M0s=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=REcs9jC9VH5GT3Wps1v/3cxStmlIhv9ZK7ssB4rpc2UC0EvZpohErKH7qNwR4qKz/U6lw8rrzmpDftLTKs0D4D4oYnaCTZc5Fs4aMljovRTnviubgEf9hiXKfXkFZnHkn/G7mHimwWGeiAfdWu51JVqGJY7y4Xo7OyPLRDIg/Ek= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=Z+tlLekJ; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="Z+tlLekJ" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6FB8B1F000E9; Mon, 17 Aug 2026 07:41:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1786952472; bh=EmziZaZv/cLbV513MDuzagnh1jExWrpF47K9zKIYMcU=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=Z+tlLekJzmSV2i7sOPawC9tGNESkdNn0F8lCQCJ/GJlbciVsvA21KbfypuVpB1N72 ayhXh9QG6klPR8NW4Y5OL2Xz9XEWfOFJiOijYK9M+7i6lpfxESIq8WlKnCTq2M+jXb uyZ5oKebW8nHMDeqnxsTt9MMz+BVIhpqsAFOhefkjlfGIAkpsxBBJZcfhkge/kva3i RuQF1l3io7no+1g6g7EvWMOpskVUkbr8SJl4ExQp8MjHDKoXAUrDqlkbZQpMl9ETBR N2HRD2hRBZiwjVjCcMp1MFo85Jo7ndRY7zFYrlGPqM9FGgFVEvOnbW4GHcQ6AglcQ8 q+Z6Fit2QeCsg== Date: Mon, 17 Aug 2026 08:40:52 +0100 From: "Lorenzo Stoakes (ARM)" To: Baolin Wang 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 Message-ID: References: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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 > 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 > --- > 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