linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP
@ 2025-05-08 10:20 Ignacio Moreno Gonzalez via B4 Relay
  2025-05-08 11:49 ` Lorenzo Stoakes
                   ` (5 more replies)
  0 siblings, 6 replies; 20+ messages in thread
From: Ignacio Moreno Gonzalez via B4 Relay @ 2025-05-08 10:20 UTC (permalink / raw)
  To: Andrew Morton, Lorenzo Stoakes, Liam R. Howlett
  Cc: yang, willy, linux-mm, linux-kernel, Ignacio Moreno Gonzalez

From: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>

VM_NOHUGEPAGE is a no-op if CONFIG_TRANSPARENT_HUGEPAGE is disabled. So
it makes no sense to return an error when calling madvise() with
MADV_NOHUGEPAGE in that case.

Suggested-by: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
---
https://lore.kernel.org/linux-mm/20250502-map-map_stack-to-vm_nohugepage-only-if-thp-is-enabled-v1-1-113cc634cd51@kuka.com

Here it is presented as a separate thread to avoid mixing stable and
non-stable patches.

This change makes calling madvise(addr, size, MADV_NOHUGEPAGE) on !THP
kernels to return 0 instead of -EINVAL.
---
 include/linux/huge_mm.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index e893d546a49f464f7586db639fe216231f03651a..5fca742dc5ba784ffccea055b07247707d16cc67 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -509,6 +509,8 @@ bool unmap_huge_pmd_locked(struct vm_area_struct *vma, unsigned long addr,
 
 #else /* CONFIG_TRANSPARENT_HUGEPAGE */
 
+#include <uapi/asm/mman.h>
+
 static inline bool folio_test_pmd_mappable(struct folio *folio)
 {
 	return false;
@@ -598,6 +600,9 @@ static inline bool unmap_huge_pmd_locked(struct vm_area_struct *vma,
 static inline int hugepage_madvise(struct vm_area_struct *vma,
 				   unsigned long *vm_flags, int advice)
 {
+	/* On a !THP kernel, MADV_NOHUGEPAGE is a no-op, but MADV_NOHUGEPAGE is not supported */
+	if (advice == MADV_NOHUGEPAGE)
+		return 0;
 	return -EINVAL;
 }
 

---
base-commit: fc96b232f8e7c0a6c282f47726b2ff6a5fb341d2
change-id: 20250508-madvise-nohugepage-noop-without-thp-e0721b973d82

Best regards,
-- 
Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>




^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP
  2025-05-08 10:20 [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP Ignacio Moreno Gonzalez via B4 Relay
@ 2025-05-08 11:49 ` Lorenzo Stoakes
  2025-05-08 15:06 ` David Hildenbrand
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: Lorenzo Stoakes @ 2025-05-08 11:49 UTC (permalink / raw)
  To: Ignacio.MorenoGonzalez
  Cc: Andrew Morton, Liam R. Howlett, yang, willy, linux-mm,
	linux-kernel

On Thu, May 08, 2025 at 12:20:27PM +0200, Ignacio Moreno Gonzalez via B4 Relay wrote:
> From: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
>
> VM_NOHUGEPAGE is a no-op if CONFIG_TRANSPARENT_HUGEPAGE is disabled. So
> it makes no sense to return an error when calling madvise() with
> MADV_NOHUGEPAGE in that case.
>
> Suggested-by: Matthew Wilcox <willy@infradead.org>
> Signed-off-by: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>

LGTM, so:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> ---
> https://lore.kernel.org/linux-mm/20250502-map-map_stack-to-vm_nohugepage-only-if-thp-is-enabled-v1-1-113cc634cd51@kuka.com
>
> Here it is presented as a separate thread to avoid mixing stable and
> non-stable patches.
>
> This change makes calling madvise(addr, size, MADV_NOHUGEPAGE) on !THP
> kernels to return 0 instead of -EINVAL.
> ---
>  include/linux/huge_mm.h | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index e893d546a49f464f7586db639fe216231f03651a..5fca742dc5ba784ffccea055b07247707d16cc67 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -509,6 +509,8 @@ bool unmap_huge_pmd_locked(struct vm_area_struct *vma, unsigned long addr,
>
>  #else /* CONFIG_TRANSPARENT_HUGEPAGE */
>
> +#include <uapi/asm/mman.h>
> +
>  static inline bool folio_test_pmd_mappable(struct folio *folio)
>  {
>  	return false;
> @@ -598,6 +600,9 @@ static inline bool unmap_huge_pmd_locked(struct vm_area_struct *vma,
>  static inline int hugepage_madvise(struct vm_area_struct *vma,
>  				   unsigned long *vm_flags, int advice)
>  {
> +	/* On a !THP kernel, MADV_NOHUGEPAGE is a no-op, but MADV_NOHUGEPAGE is not supported */
> +	if (advice == MADV_NOHUGEPAGE)
> +		return 0;
>  	return -EINVAL;
>  }
>
>
> ---
> base-commit: fc96b232f8e7c0a6c282f47726b2ff6a5fb341d2
> change-id: 20250508-madvise-nohugepage-noop-without-thp-e0721b973d82
>
> Best regards,
> --
> Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
>
>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP
  2025-05-08 10:20 [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP Ignacio Moreno Gonzalez via B4 Relay
  2025-05-08 11:49 ` Lorenzo Stoakes
@ 2025-05-08 15:06 ` David Hildenbrand
  2025-05-08 15:32 ` Yang Shi
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: David Hildenbrand @ 2025-05-08 15:06 UTC (permalink / raw)
  To: Ignacio.MorenoGonzalez, Andrew Morton, Lorenzo Stoakes,
	Liam R. Howlett
  Cc: yang, willy, linux-mm, linux-kernel

On 08.05.25 12:20, Ignacio Moreno Gonzalez via B4 Relay wrote:
> From: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
> 
> VM_NOHUGEPAGE is a no-op if CONFIG_TRANSPARENT_HUGEPAGE is disabled. So
> it makes no sense to return an error when calling madvise() with
> MADV_NOHUGEPAGE in that case.
> 
> Suggested-by: Matthew Wilcox <willy@infradead.org>
> Signed-off-by: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
> ---
> https://lore.kernel.org/linux-mm/20250502-map-map_stack-to-vm_nohugepage-only-if-thp-is-enabled-v1-1-113cc634cd51@kuka.com
> 
> Here it is presented as a separate thread to avoid mixing stable and
> non-stable patches.
> 
> This change makes calling madvise(addr, size, MADV_NOHUGEPAGE) on !THP
> kernels to return 0 instead of -EINVAL.
> ---
>   include/linux/huge_mm.h | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index e893d546a49f464f7586db639fe216231f03651a..5fca742dc5ba784ffccea055b07247707d16cc67 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -509,6 +509,8 @@ bool unmap_huge_pmd_locked(struct vm_area_struct *vma, unsigned long addr,
>   
>   #else /* CONFIG_TRANSPARENT_HUGEPAGE */
>   
> +#include <uapi/asm/mman.h>
> +
>   static inline bool folio_test_pmd_mappable(struct folio *folio)
>   {
>   	return false;
> @@ -598,6 +600,9 @@ static inline bool unmap_huge_pmd_locked(struct vm_area_struct *vma,
>   static inline int hugepage_madvise(struct vm_area_struct *vma,
>   				   unsigned long *vm_flags, int advice)
>   {
> +	/* On a !THP kernel, MADV_NOHUGEPAGE is a no-op, but MADV_NOHUGEPAGE is not supported */
> +	if (advice == MADV_NOHUGEPAGE)
> +		return 0;
>   	return -EINVAL;
>   }

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP
  2025-05-08 10:20 [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP Ignacio Moreno Gonzalez via B4 Relay
  2025-05-08 11:49 ` Lorenzo Stoakes
  2025-05-08 15:06 ` David Hildenbrand
@ 2025-05-08 15:32 ` Yang Shi
  2025-05-08 19:04 ` Liam R. Howlett
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: Yang Shi @ 2025-05-08 15:32 UTC (permalink / raw)
  To: Ignacio.MorenoGonzalez, Andrew Morton, Lorenzo Stoakes,
	Liam R. Howlett
  Cc: willy, linux-mm, linux-kernel



On 5/8/25 3:20 AM, Ignacio Moreno Gonzalez via B4 Relay wrote:
> From: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
>
> VM_NOHUGEPAGE is a no-op if CONFIG_TRANSPARENT_HUGEPAGE is disabled. So
> it makes no sense to return an error when calling madvise() with
> MADV_NOHUGEPAGE in that case.
>
> Suggested-by: Matthew Wilcox <willy@infradead.org>
> Signed-off-by: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
> ---
> https://lore.kernel.org/linux-mm/20250502-map-map_stack-to-vm_nohugepage-only-if-thp-is-enabled-v1-1-113cc634cd51@kuka.com
>
> Here it is presented as a separate thread to avoid mixing stable and
> non-stable patches.
>
> This change makes calling madvise(addr, size, MADV_NOHUGEPAGE) on !THP
> kernels to return 0 instead of -EINVAL.
> ---
>   include/linux/huge_mm.h | 5 +++++
>   1 file changed, 5 insertions(+)

Reviewed-by: Yang Shi <yang@os.amperecomputing.com>

>
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index e893d546a49f464f7586db639fe216231f03651a..5fca742dc5ba784ffccea055b07247707d16cc67 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -509,6 +509,8 @@ bool unmap_huge_pmd_locked(struct vm_area_struct *vma, unsigned long addr,
>   
>   #else /* CONFIG_TRANSPARENT_HUGEPAGE */
>   
> +#include <uapi/asm/mman.h>
> +
>   static inline bool folio_test_pmd_mappable(struct folio *folio)
>   {
>   	return false;
> @@ -598,6 +600,9 @@ static inline bool unmap_huge_pmd_locked(struct vm_area_struct *vma,
>   static inline int hugepage_madvise(struct vm_area_struct *vma,
>   				   unsigned long *vm_flags, int advice)
>   {
> +	/* On a !THP kernel, MADV_NOHUGEPAGE is a no-op, but MADV_NOHUGEPAGE is not supported */
> +	if (advice == MADV_NOHUGEPAGE)
> +		return 0;
>   	return -EINVAL;
>   }
>   
>
> ---
> base-commit: fc96b232f8e7c0a6c282f47726b2ff6a5fb341d2
> change-id: 20250508-madvise-nohugepage-noop-without-thp-e0721b973d82
>
> Best regards,



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP
  2025-05-08 10:20 [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP Ignacio Moreno Gonzalez via B4 Relay
                   ` (2 preceding siblings ...)
  2025-05-08 15:32 ` Yang Shi
@ 2025-05-08 19:04 ` Liam R. Howlett
  2025-05-09  5:47   ` Ignacio Moreno Gonzalez
  2025-05-14 13:52 ` Lorenzo Stoakes
  2025-05-14 20:15 ` James Houghton
  5 siblings, 1 reply; 20+ messages in thread
From: Liam R. Howlett @ 2025-05-08 19:04 UTC (permalink / raw)
  To: Ignacio.MorenoGonzalez
  Cc: Andrew Morton, Lorenzo Stoakes, yang, willy, linux-mm,
	linux-kernel

* Ignacio Moreno Gonzalez via B4 Relay <devnull+Ignacio.MorenoGonzalez.kuka.com@kernel.org> [250508 06:20]:
> From: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
> 
> VM_NOHUGEPAGE is a no-op if CONFIG_TRANSPARENT_HUGEPAGE is disabled. So
> it makes no sense to return an error when calling madvise() with
> MADV_NOHUGEPAGE in that case.
> 
> Suggested-by: Matthew Wilcox <willy@infradead.org>
> Signed-off-by: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
> ---
> https://lore.kernel.org/linux-mm/20250502-map-map_stack-to-vm_nohugepage-only-if-thp-is-enabled-v1-1-113cc634cd51@kuka.com
> 
> Here it is presented as a separate thread to avoid mixing stable and
> non-stable patches.
> 
> This change makes calling madvise(addr, size, MADV_NOHUGEPAGE) on !THP
> kernels to return 0 instead of -EINVAL.

Sorry, maybe I missed something here.

Which one should Cc stable, I don't see it on either patch?

Thanks,
Liam

> ---
>  include/linux/huge_mm.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index e893d546a49f464f7586db639fe216231f03651a..5fca742dc5ba784ffccea055b07247707d16cc67 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -509,6 +509,8 @@ bool unmap_huge_pmd_locked(struct vm_area_struct *vma, unsigned long addr,
>  
>  #else /* CONFIG_TRANSPARENT_HUGEPAGE */
>  
> +#include <uapi/asm/mman.h>
> +
>  static inline bool folio_test_pmd_mappable(struct folio *folio)
>  {
>  	return false;
> @@ -598,6 +600,9 @@ static inline bool unmap_huge_pmd_locked(struct vm_area_struct *vma,
>  static inline int hugepage_madvise(struct vm_area_struct *vma,
>  				   unsigned long *vm_flags, int advice)
>  {
> +	/* On a !THP kernel, MADV_NOHUGEPAGE is a no-op, but MADV_NOHUGEPAGE is not supported */
> +	if (advice == MADV_NOHUGEPAGE)
> +		return 0;
>  	return -EINVAL;
>  }
>  
> 
> ---
> base-commit: fc96b232f8e7c0a6c282f47726b2ff6a5fb341d2
> change-id: 20250508-madvise-nohugepage-noop-without-thp-e0721b973d82
> 
> Best regards,
> -- 
> Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
> 
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP
  2025-05-08 19:04 ` Liam R. Howlett
@ 2025-05-09  5:47   ` Ignacio Moreno Gonzalez
  2025-05-09 17:24     ` Liam R. Howlett
  0 siblings, 1 reply; 20+ messages in thread
From: Ignacio Moreno Gonzalez @ 2025-05-09  5:47 UTC (permalink / raw)
  To: Liam R. Howlett, Andrew Morton, Lorenzo Stoakes, yang, willy,
	linux-mm, linux-kernel

On 5/8/2025 9:04 PM, Liam R. Howlett wrote:
> Which one should Cc stable, I don't see it on either patch?

The patch in this thread is the one without CC to stable, and the one here:

https://lore.kernel.org/linux-mm/20250507-map-map_stack-to-vm_nohugepage-only-if-thp-is-enabled-v5-1-c6c38cfefd6e@kuka.com/

has CC to stable.

I had submitted both patches together in a series here:
https://lore.kernel.org/linux-mm/20250506-map-map_stack-to-vm_nohugepage-only-if-thp-is-enabled-v2-0-f11f0c794872@kuka.com/

but then Andrew explained that this is troublesome, that's why I then submitted them separately.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP
  2025-05-09  5:47   ` Ignacio Moreno Gonzalez
@ 2025-05-09 17:24     ` Liam R. Howlett
  0 siblings, 0 replies; 20+ messages in thread
From: Liam R. Howlett @ 2025-05-09 17:24 UTC (permalink / raw)
  To: Ignacio Moreno Gonzalez
  Cc: Andrew Morton, Lorenzo Stoakes, yang, willy, linux-mm,
	linux-kernel

* Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com> [250509 01:47]:
> On 5/8/2025 9:04 PM, Liam R. Howlett wrote:
> > Which one should Cc stable, I don't see it on either patch?
> 
> The patch in this thread is the one without CC to stable, and the one here:
> 
> https://lore.kernel.org/linux-mm/20250507-map-map_stack-to-vm_nohugepage-only-if-thp-is-enabled-v5-1-c6c38cfefd6e@kuka.com/
> 
> has CC to stable.
> 
> I had submitted both patches together in a series here:
> https://lore.kernel.org/linux-mm/20250506-map-map_stack-to-vm_nohugepage-only-if-thp-is-enabled-v2-0-f11f0c794872@kuka.com/
> 
> but then Andrew explained that this is troublesome, that's why I then submitted them separately.

Right, okay.  I felt like I was going crazy because I was sure I looked
at this before, but couldn't figure out what was going on.

Thanks,
Liam



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP
  2025-05-08 10:20 [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP Ignacio Moreno Gonzalez via B4 Relay
                   ` (3 preceding siblings ...)
  2025-05-08 19:04 ` Liam R. Howlett
@ 2025-05-14 13:52 ` Lorenzo Stoakes
  2025-05-14 15:07   ` Ignacio Moreno Gonzalez
  2025-05-14 20:15 ` James Houghton
  5 siblings, 1 reply; 20+ messages in thread
From: Lorenzo Stoakes @ 2025-05-14 13:52 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Ignacio.MorenoGonzalez, Liam R. Howlett, yang, willy, linux-mm,
	linux-kernel

Andrew - we should probably drop this patch for now given the report at [0].

It seems s390 (to risk sounding hyperbolic - fairly ludicriously) declares
PROT_NONE in an enum somewhere that blows this up.

I have pinged s390 people on there, but I don't think this is going to make this
cycle given we will probably need to coordinate with them to fix up this enum
name (which seems the best solution to me!...)

[0]: https://lore.kernel.org/all/202505140943.IgHDa9s7-lkp@intel.com/

Thanks, Lorenzo

On Thu, May 08, 2025 at 12:20:27PM +0200, Ignacio Moreno Gonzalez via B4 Relay wrote:
> From: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
>
> VM_NOHUGEPAGE is a no-op if CONFIG_TRANSPARENT_HUGEPAGE is disabled. So
> it makes no sense to return an error when calling madvise() with
> MADV_NOHUGEPAGE in that case.
>
> Suggested-by: Matthew Wilcox <willy@infradead.org>
> Signed-off-by: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
> ---
> https://lore.kernel.org/linux-mm/20250502-map-map_stack-to-vm_nohugepage-only-if-thp-is-enabled-v1-1-113cc634cd51@kuka.com
>
> Here it is presented as a separate thread to avoid mixing stable and
> non-stable patches.
>
> This change makes calling madvise(addr, size, MADV_NOHUGEPAGE) on !THP
> kernels to return 0 instead of -EINVAL.
> ---
>  include/linux/huge_mm.h | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index e893d546a49f464f7586db639fe216231f03651a..5fca742dc5ba784ffccea055b07247707d16cc67 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -509,6 +509,8 @@ bool unmap_huge_pmd_locked(struct vm_area_struct *vma, unsigned long addr,
>
>  #else /* CONFIG_TRANSPARENT_HUGEPAGE */
>
> +#include <uapi/asm/mman.h>

Also this should be linux/mman.h I believe, sorry for not catching first time round...!

> +
>  static inline bool folio_test_pmd_mappable(struct folio *folio)
>  {
>  	return false;
> @@ -598,6 +600,9 @@ static inline bool unmap_huge_pmd_locked(struct vm_area_struct *vma,
>  static inline int hugepage_madvise(struct vm_area_struct *vma,
>  				   unsigned long *vm_flags, int advice)
>  {
> +	/* On a !THP kernel, MADV_NOHUGEPAGE is a no-op, but MADV_NOHUGEPAGE is not supported */
> +	if (advice == MADV_NOHUGEPAGE)
> +		return 0;
>  	return -EINVAL;
>  }
>
>
> ---
> base-commit: fc96b232f8e7c0a6c282f47726b2ff6a5fb341d2
> change-id: 20250508-madvise-nohugepage-noop-without-thp-e0721b973d82
>
> Best regards,
> --
> Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
>
>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP
  2025-05-14 13:52 ` Lorenzo Stoakes
@ 2025-05-14 15:07   ` Ignacio Moreno Gonzalez
  2025-05-14 15:25     ` Yang Shi
  2025-05-14 15:27     ` Lorenzo Stoakes
  0 siblings, 2 replies; 20+ messages in thread
From: Ignacio Moreno Gonzalez @ 2025-05-14 15:07 UTC (permalink / raw)
  To: Lorenzo Stoakes, Andrew Morton
  Cc: Liam R. Howlett, yang, willy, linux-mm, linux-kernel

On 5/14/2025 3:52 PM, Lorenzo Stoakes wrote:

> I have pinged s390 people on there, but I don't think this is going to make this
> cycle given we will probably need to coordinate with them to fix up this enum
> name (which seems the best solution to me!...)

They answered that it would be ok for them to do a quick fix over the mm tree:

https://lore.kernel.org/linux-mm/6f8f3780-902b-49d4-a766-ea2e1a8f85ea@linux.ibm.com/


>> +#include <uapi/asm/mman.h>
> 
> Also this should be linux/mman.h I believe, sorry for not catching first time round...!
> 

Including linux/mman.h leads to a compilation error:

../include/linux/huge_mm.h:601:23: error: ‘MADV_NOHUGEPAGE’

Including uapi/linux/mman.h works, but I am not sure if that is correct.




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP
  2025-05-14 15:07   ` Ignacio Moreno Gonzalez
@ 2025-05-14 15:25     ` Yang Shi
  2025-05-14 15:29       ` Lorenzo Stoakes
  2025-05-14 15:27     ` Lorenzo Stoakes
  1 sibling, 1 reply; 20+ messages in thread
From: Yang Shi @ 2025-05-14 15:25 UTC (permalink / raw)
  To: Ignacio Moreno Gonzalez, Lorenzo Stoakes, Andrew Morton
  Cc: Liam R. Howlett, willy, linux-mm, linux-kernel



On 5/14/25 8:07 AM, Ignacio Moreno Gonzalez wrote:
> On 5/14/2025 3:52 PM, Lorenzo Stoakes wrote:
>
>> I have pinged s390 people on there, but I don't think this is going to make this
>> cycle given we will probably need to coordinate with them to fix up this enum
>> name (which seems the best solution to me!...)
> They answered that it would be ok for them to do a quick fix over the mm tree:
>
> https://lore.kernel.org/linux-mm/6f8f3780-902b-49d4-a766-ea2e1a8f85ea@linux.ibm.com/
>
>
>>> +#include <uapi/asm/mman.h>
>> Also this should be linux/mman.h I believe, sorry for not catching first time round...!
>>
> Including linux/mman.h leads to a compilation error:
>
> ../include/linux/huge_mm.h:601:23: error: ‘MADV_NOHUGEPAGE’
>
> Including uapi/linux/mman.h works, but I am not sure if that is correct.

Is this build on x86? I actually tried this on my arm64 build before I 
suggested uapi/asm/mman.h. But I saw a lot of compilation errors, like:

In file included from ./include/linux/memcontrol.h:22,
                  from ./include/linux/swap.h:9,
                  from ./include/linux/userfaultfd_k.h:18,
                  from ./include/linux/hugetlb.h:16,
                  from ./arch/arm64/include/asm/mman.h:10,
                  from ./include/uapi/linux/mman.h:5,
                  from ./include/linux/huge_mm.h:512,
                  from ./include/linux/mm.h:1238,
                  from ./include/linux/memblock.h:12,
                  from ./arch/arm64/include/asm/acpi.h:14,
                  from ./include/acpi/acpi_io.h:7,
                  from ./include/linux/acpi.h:39,
                  from ./include/acpi/apei.h:9,
                  from ./include/acpi/ghes.h:5,
                  from ./include/linux/arm_sdei.h:8,
                  from arch/arm64/kernel/asm-offsets.c:10:
./include/linux/vmstat.h: In function ‘__zone_stat_mod_folio’:
./include/linux/vmstat.h:414:31: error: implicit declaration of function 
‘folio_zone’; did you mean ‘folio_zonenum’? 
[-Wimplicit-function-declaration]
   414 |         __mod_zone_page_state(folio_zone(folio), item, nr);
       |                               ^~~~~~~~~~
       |                               folio_zonenum
./include/linux/vmstat.h:414:31: error: passing argument 1 of 
‘__mod_zone_page_state’ makes pointer from integer without a cast 
[-Wint-conversion]
   414 |         __mod_zone_page_state(folio_zone(folio), item, nr);
       |                               ^~~~~~~~~~~~~~~~~
       |                               |
       |                               int
./include/linux/vmstat.h:273:28: note: expected ‘struct zone *’ but 
argument is of type ‘int’
   273 | void __mod_zone_page_state(struct zone *, enum zone_stat_item 
item, long);
       |                            ^~~~~~~~~~~~~
./include/linux/vmstat.h: In function ‘__zone_stat_add_folio’:
./include/linux/vmstat.h:420:56: error: implicit declaration of function 
‘folio_nr_pages’; did you mean ‘folio_page’? 
[-Wimplicit-function-declaration]
   420 |         __mod_zone_page_state(folio_zone(folio), item, 
folio_nr_pages(folio));
       | ^~~~~~~~~~~~~~


The build used default Fedora kernel config with THP disabled and 
v6.15-rc6 kernel.

>
>



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP
  2025-05-14 15:07   ` Ignacio Moreno Gonzalez
  2025-05-14 15:25     ` Yang Shi
@ 2025-05-14 15:27     ` Lorenzo Stoakes
  2025-05-14 15:59       ` Ignacio Moreno Gonzalez
  1 sibling, 1 reply; 20+ messages in thread
From: Lorenzo Stoakes @ 2025-05-14 15:27 UTC (permalink / raw)
  To: Ignacio Moreno Gonzalez
  Cc: Andrew Morton, Liam R. Howlett, yang, willy, linux-mm,
	linux-kernel

Andrew - please hold fire given below. It seems we _can_ patch this after all :)

Missing context:

> Andrew - we should probably drop this patch for now given the report at [0].
>
> It seems s390 (to risk sounding hyperbolic - fairly ludicriously) declares
> PROT_NONE in an enum somewhere that blows this up.

On Wed, May 14, 2025 at 05:07:58PM +0200, Ignacio Moreno Gonzalez wrote:
> On 5/14/2025 3:52 PM, Lorenzo Stoakes wrote:

Igancio - you're deleting too much context here, you did it in the other email
too, it's really hard to follow what's going on. Please try to be careful about
what bits of an email you delete when quoting.

You're putting my comment here without the context of what I asked Andrew, so
now it's not clear what's going on.

>
> > I have pinged s390 people on there, but I don't think this is going to make this
> > cycle given we will probably need to coordinate with them to fix up this enum
> > name (which seems the best solution to me!...)
>
> They answered that it would be ok for them to do a quick fix over the mm tree:
>
> https://lore.kernel.org/linux-mm/6f8f3780-902b-49d4-a766-ea2e1a8f85ea@linux.ibm.com/

Yeah, we need to figure out who's going to do this patch though :)

Did you want to Iganacio?

I would suggest chasing up with them to see if they plan to do it or you ought
to do so.

I can also do it if you need it quick, but I don't want to deprive you of the
opportunity to patch that :)

They are happy for that to go in via the mm tree, so we should be able to do
this quick.

Thanks, Lorenzo

>
>
> >> +#include <uapi/asm/mman.h>
> >
> > Also this should be linux/mman.h I believe, sorry for not catching first time round...!
> >
>
> Including linux/mman.h leads to a compilation error:
>
> ../include/linux/huge_mm.h:601:23: error: ‘MADV_NOHUGEPAGE’
>
> Including uapi/linux/mman.h works, but I am not sure if that is correct.

OK this is because of a circular dependency (le sigh) as linux/mman.h itself
imports linux/mm.h (prior to including the headers that define this) that then
imports linux/huge_mm.h and welcome to hell.

Le sigh deux.

OK leave it as it is!

C headers are a pain.

>
>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP
  2025-05-14 15:25     ` Yang Shi
@ 2025-05-14 15:29       ` Lorenzo Stoakes
  2025-05-14 16:54         ` Yang Shi
  0 siblings, 1 reply; 20+ messages in thread
From: Lorenzo Stoakes @ 2025-05-14 15:29 UTC (permalink / raw)
  To: Yang Shi
  Cc: Ignacio Moreno Gonzalez, Andrew Morton, Liam R. Howlett, willy,
	linux-mm, linux-kernel

On Wed, May 14, 2025 at 08:25:05AM -0700, Yang Shi wrote:
>
>
> On 5/14/25 8:07 AM, Ignacio Moreno Gonzalez wrote:
> > On 5/14/2025 3:52 PM, Lorenzo Stoakes wrote:
> >
> > > I have pinged s390 people on there, but I don't think this is going to make this
> > > cycle given we will probably need to coordinate with them to fix up this enum
> > > name (which seems the best solution to me!...)
> > They answered that it would be ok for them to do a quick fix over the mm tree:
> >
> > https://lore.kernel.org/linux-mm/6f8f3780-902b-49d4-a766-ea2e1a8f85ea@linux.ibm.com/
> >
> >
> > > > +#include <uapi/asm/mman.h>
> > > Also this should be linux/mman.h I believe, sorry for not catching first time round...!
> > >
> > Including linux/mman.h leads to a compilation error:
> >
> > ../include/linux/huge_mm.h:601:23: error: ‘MADV_NOHUGEPAGE’
> >
> > Including uapi/linux/mman.h works, but I am not sure if that is correct.
>
> Is this build on x86? I actually tried this on my arm64 build before I
> suggested uapi/asm/mman.h. But I saw a lot of compilation errors, like:
>
> In file included from ./include/linux/memcontrol.h:22,
>                  from ./include/linux/swap.h:9,
>                  from ./include/linux/userfaultfd_k.h:18,
>                  from ./include/linux/hugetlb.h:16,
>                  from ./arch/arm64/include/asm/mman.h:10,
>                  from ./include/uapi/linux/mman.h:5,
>                  from ./include/linux/huge_mm.h:512,
>                  from ./include/linux/mm.h:1238,
>                  from ./include/linux/memblock.h:12,
>                  from ./arch/arm64/include/asm/acpi.h:14,
>                  from ./include/acpi/acpi_io.h:7,
>                  from ./include/linux/acpi.h:39,
>                  from ./include/acpi/apei.h:9,
>                  from ./include/acpi/ghes.h:5,
>                  from ./include/linux/arm_sdei.h:8,
>                  from arch/arm64/kernel/asm-offsets.c:10:
> ./include/linux/vmstat.h: In function ‘__zone_stat_mod_folio’:
> ./include/linux/vmstat.h:414:31: error: implicit declaration of function
> ‘folio_zone’; did you mean ‘folio_zonenum’?
> [-Wimplicit-function-declaration]
>   414 |         __mod_zone_page_state(folio_zone(folio), item, nr);
>       |                               ^~~~~~~~~~
>       |                               folio_zonenum
> ./include/linux/vmstat.h:414:31: error: passing argument 1 of
> ‘__mod_zone_page_state’ makes pointer from integer without a cast
> [-Wint-conversion]
>   414 |         __mod_zone_page_state(folio_zone(folio), item, nr);
>       |                               ^~~~~~~~~~~~~~~~~
>       |                               |
>       |                               int
> ./include/linux/vmstat.h:273:28: note: expected ‘struct zone *’ but argument
> is of type ‘int’
>   273 | void __mod_zone_page_state(struct zone *, enum zone_stat_item item,
> long);
>       |                            ^~~~~~~~~~~~~
> ./include/linux/vmstat.h: In function ‘__zone_stat_add_folio’:
> ./include/linux/vmstat.h:420:56: error: implicit declaration of function
> ‘folio_nr_pages’; did you mean ‘folio_page’?
> [-Wimplicit-function-declaration]
>   420 |         __mod_zone_page_state(folio_zone(folio), item,
> folio_nr_pages(folio));
>       | ^~~~~~~~~~~~~~
>
>
> The build used default Fedora kernel config with THP disabled and v6.15-rc6
> kernel.

Yeah I suspect this is because of circular dependencies as I mentioned in reply
to Ignacio :) Sorry I missed you'd suggested it Yang, you were right to do so :)

You can see mm.h -> huge_mm.h there so it does seem to be same thing.

C headers are a source of many sighs.

>
> >
> >
>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP
  2025-05-14 15:27     ` Lorenzo Stoakes
@ 2025-05-14 15:59       ` Ignacio Moreno Gonzalez
  2025-05-14 22:36         ` Andrew Morton
  0 siblings, 1 reply; 20+ messages in thread
From: Ignacio Moreno Gonzalez @ 2025-05-14 15:59 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: Andrew Morton, Liam R. Howlett, yang, willy, linux-mm,
	linux-kernel

On 5/14/2025 5:27 PM, Lorenzo Stoakes wrote:

>> On 5/14/2025 3:52 PM, Lorenzo Stoakes wrote:
> 
> Igancio - you're deleting too much context here, you did it in the other email
> too, it's really hard to follow what's going on. Please try to be careful about
> what bits of an email you delete when quoting.
> 
> You're putting my comment here without the context of what I asked Andrew, so
> now it's not clear what's going on.
> 

Hi Lorenzo,

Sorry about that and thanks for letting me know. I appreciate your advice, since I am new in the mailing list. I will try to include more context to avoid confusion in the future :)


>>> I have pinged s390 people on there, but I don't think this is going to make this
>>> cycle given we will probably need to coordinate with them to fix up this enum
>>> name (which seems the best solution to me!...)
>>
>> They answered that it would be ok for them to do a quick fix over the mm tree:
>>
>> https://lore.kernel.org/linux-mm/6f8f3780-902b-49d4-a766-ea2e1a8f85ea@linux.ibm.com/
> 
> Yeah, we need to figure out who's going to do this patch though :)
> 
> Did you want to Iganacio?
> 
> I would suggest chasing up with them to see if they plan to do it or you ought
> to do so.
> 
> I can also do it if you need it quick, but I don't want to deprive you of the
> opportunity to patch that :)

For me it's fine if you create the patch. That will probably be faster ;)


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP
  2025-05-14 15:29       ` Lorenzo Stoakes
@ 2025-05-14 16:54         ` Yang Shi
  0 siblings, 0 replies; 20+ messages in thread
From: Yang Shi @ 2025-05-14 16:54 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: Ignacio Moreno Gonzalez, Andrew Morton, Liam R. Howlett, willy,
	linux-mm, linux-kernel



On 5/14/25 8:29 AM, Lorenzo Stoakes wrote:
> On Wed, May 14, 2025 at 08:25:05AM -0700, Yang Shi wrote:
>>
>> On 5/14/25 8:07 AM, Ignacio Moreno Gonzalez wrote:
>>> On 5/14/2025 3:52 PM, Lorenzo Stoakes wrote:
>>>
>>>> I have pinged s390 people on there, but I don't think this is going to make this
>>>> cycle given we will probably need to coordinate with them to fix up this enum
>>>> name (which seems the best solution to me!...)
>>> They answered that it would be ok for them to do a quick fix over the mm tree:
>>>
>>> https://lore.kernel.org/linux-mm/6f8f3780-902b-49d4-a766-ea2e1a8f85ea@linux.ibm.com/
>>>
>>>
>>>>> +#include <uapi/asm/mman.h>
>>>> Also this should be linux/mman.h I believe, sorry for not catching first time round...!
>>>>
>>> Including linux/mman.h leads to a compilation error:
>>>
>>> ../include/linux/huge_mm.h:601:23: error: ‘MADV_NOHUGEPAGE’
>>>
>>> Including uapi/linux/mman.h works, but I am not sure if that is correct.
>> Is this build on x86? I actually tried this on my arm64 build before I
>> suggested uapi/asm/mman.h. But I saw a lot of compilation errors, like:
>>
>> In file included from ./include/linux/memcontrol.h:22,
>>                   from ./include/linux/swap.h:9,
>>                   from ./include/linux/userfaultfd_k.h:18,
>>                   from ./include/linux/hugetlb.h:16,
>>                   from ./arch/arm64/include/asm/mman.h:10,
>>                   from ./include/uapi/linux/mman.h:5,
>>                   from ./include/linux/huge_mm.h:512,
>>                   from ./include/linux/mm.h:1238,
>>                   from ./include/linux/memblock.h:12,
>>                   from ./arch/arm64/include/asm/acpi.h:14,
>>                   from ./include/acpi/acpi_io.h:7,
>>                   from ./include/linux/acpi.h:39,
>>                   from ./include/acpi/apei.h:9,
>>                   from ./include/acpi/ghes.h:5,
>>                   from ./include/linux/arm_sdei.h:8,
>>                   from arch/arm64/kernel/asm-offsets.c:10:
>> ./include/linux/vmstat.h: In function ‘__zone_stat_mod_folio’:
>> ./include/linux/vmstat.h:414:31: error: implicit declaration of function
>> ‘folio_zone’; did you mean ‘folio_zonenum’?
>> [-Wimplicit-function-declaration]
>>    414 |         __mod_zone_page_state(folio_zone(folio), item, nr);
>>        |                               ^~~~~~~~~~
>>        |                               folio_zonenum
>> ./include/linux/vmstat.h:414:31: error: passing argument 1 of
>> ‘__mod_zone_page_state’ makes pointer from integer without a cast
>> [-Wint-conversion]
>>    414 |         __mod_zone_page_state(folio_zone(folio), item, nr);
>>        |                               ^~~~~~~~~~~~~~~~~
>>        |                               |
>>        |                               int
>> ./include/linux/vmstat.h:273:28: note: expected ‘struct zone *’ but argument
>> is of type ‘int’
>>    273 | void __mod_zone_page_state(struct zone *, enum zone_stat_item item,
>> long);
>>        |                            ^~~~~~~~~~~~~
>> ./include/linux/vmstat.h: In function ‘__zone_stat_add_folio’:
>> ./include/linux/vmstat.h:420:56: error: implicit declaration of function
>> ‘folio_nr_pages’; did you mean ‘folio_page’?
>> [-Wimplicit-function-declaration]
>>    420 |         __mod_zone_page_state(folio_zone(folio), item,
>> folio_nr_pages(folio));
>>        | ^~~~~~~~~~~~~~
>>
>>
>> The build used default Fedora kernel config with THP disabled and v6.15-rc6
>> kernel.
> Yeah I suspect this is because of circular dependencies as I mentioned in reply
> to Ignacio :) Sorry I missed you'd suggested it Yang, you were right to do so :)

No problem :)

>
> You can see mm.h -> huge_mm.h there so it does seem to be same thing.

Yeah, it caused some circular dependencies.

>
> C headers are a source of many sighs.
>
>>>



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP
  2025-05-08 10:20 [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP Ignacio Moreno Gonzalez via B4 Relay
                   ` (4 preceding siblings ...)
  2025-05-14 13:52 ` Lorenzo Stoakes
@ 2025-05-14 20:15 ` James Houghton
  2025-05-15  7:03   ` Ignacio Moreno Gonzalez
  5 siblings, 1 reply; 20+ messages in thread
From: James Houghton @ 2025-05-14 20:15 UTC (permalink / raw)
  To: Ignacio.MorenoGonzalez
  Cc: Andrew Morton, Lorenzo Stoakes, Liam R. Howlett, yang, willy,
	linux-mm, linux-kernel

On Thu, May 8, 2025 at 3:20 AM Ignacio Moreno Gonzalez via B4 Relay
<devnull+Ignacio.MorenoGonzalez.kuka.com@kernel.org> wrote:
>
> From: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
>
> VM_NOHUGEPAGE is a no-op if CONFIG_TRANSPARENT_HUGEPAGE is disabled. So
> it makes no sense to return an error when calling madvise() with
> MADV_NOHUGEPAGE in that case.
>
> Suggested-by: Matthew Wilcox <willy@infradead.org>
> Signed-off-by: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
> ---
> https://lore.kernel.org/linux-mm/20250502-map-map_stack-to-vm_nohugepage-only-if-thp-is-enabled-v1-1-113cc634cd51@kuka.com
>
> Here it is presented as a separate thread to avoid mixing stable and
> non-stable patches.
>
> This change makes calling madvise(addr, size, MADV_NOHUGEPAGE) on !THP
> kernels to return 0 instead of -EINVAL.
> ---
>  include/linux/huge_mm.h | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index e893d546a49f464f7586db639fe216231f03651a..5fca742dc5ba784ffccea055b07247707d16cc67 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -509,6 +509,8 @@ bool unmap_huge_pmd_locked(struct vm_area_struct *vma, unsigned long addr,
>
>  #else /* CONFIG_TRANSPARENT_HUGEPAGE */
>
> +#include <uapi/asm/mman.h>
> +
>  static inline bool folio_test_pmd_mappable(struct folio *folio)
>  {
>         return false;
> @@ -598,6 +600,9 @@ static inline bool unmap_huge_pmd_locked(struct vm_area_struct *vma,
>  static inline int hugepage_madvise(struct vm_area_struct *vma,
>                                    unsigned long *vm_flags, int advice)
>  {
> +       /* On a !THP kernel, MADV_NOHUGEPAGE is a no-op, but MADV_NOHUGEPAGE is not supported */

Do you mean "but MADV_HUGEPAGE is not supported"? Just want to make
sure; it seems like no one else has asked about this yet.

> +       if (advice == MADV_NOHUGEPAGE)
> +               return 0;
>         return -EINVAL;
>  }


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP
  2025-05-14 15:59       ` Ignacio Moreno Gonzalez
@ 2025-05-14 22:36         ` Andrew Morton
  2025-05-15  5:30           ` Lorenzo Stoakes
  0 siblings, 1 reply; 20+ messages in thread
From: Andrew Morton @ 2025-05-14 22:36 UTC (permalink / raw)
  To: Ignacio Moreno Gonzalez
  Cc: Lorenzo Stoakes, Liam R. Howlett, yang, willy, linux-mm,
	linux-kernel

On Wed, 14 May 2025 17:59:23 +0200 Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com> wrote:

> > Did you want to Iganacio?
> > 
> > I would suggest chasing up with them to see if they plan to do it or you ought
> > to do so.
> > 
> > I can also do it if you need it quick, but I don't want to deprive you of the
> > opportunity to patch that :)
> 
> For me it's fine if you create the patch. That will probably be faster ;)

The original patch adds the only direct inclusion of uapi/asm/mman.h
under include/, which is a big red flag.

I'll drop this version of the patch, thanks.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP
  2025-05-14 22:36         ` Andrew Morton
@ 2025-05-15  5:30           ` Lorenzo Stoakes
  0 siblings, 0 replies; 20+ messages in thread
From: Lorenzo Stoakes @ 2025-05-15  5:30 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Ignacio Moreno Gonzalez, Liam R. Howlett, yang, willy, linux-mm,
	linux-kernel

On Wed, May 14, 2025 at 03:36:48PM -0700, Andrew Morton wrote:
> On Wed, 14 May 2025 17:59:23 +0200 Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com> wrote:
>
> > > Did you want to Iganacio?
> > >
> > > I would suggest chasing up with them to see if they plan to do it or you ought
> > > to do so.
> > >
> > > I can also do it if you need it quick, but I don't want to deprive you of the
> > > opportunity to patch that :)
> >
> > For me it's fine if you create the patch. That will probably be faster ;)
>
> The original patch adds the only direct inclusion of uapi/asm/mman.h
> under include/, which is a big red flag.
>
> I'll drop this version of the patch, thanks.

Yeah this is a case where it is justified, due to circular dependencies.

At any rate this whole thing is a bit of a mess now, so let me liaise with
Ignacio to resend this in a series alongside its now hard-dependency [0] to
ensure correct ordering.

Thanks, Lorenzo

[0]: https://lore.kernel.org/linux-mm/20250514163530.119582-1-lorenzo.stoakes@oracle.com/


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP
  2025-05-14 20:15 ` James Houghton
@ 2025-05-15  7:03   ` Ignacio Moreno Gonzalez
  2025-05-15 13:03     ` Lorenzo Stoakes
  0 siblings, 1 reply; 20+ messages in thread
From: Ignacio Moreno Gonzalez @ 2025-05-15  7:03 UTC (permalink / raw)
  To: James Houghton
  Cc: Andrew Morton, Lorenzo Stoakes, Liam R. Howlett, yang, willy,
	linux-mm, linux-kernel

On 5/14/2025 10:15 PM, James Houghton wrote:
> On Thu, May 8, 2025 at 3:20 AM Ignacio Moreno Gonzalez via B4 Relay
> <devnull+Ignacio.MorenoGonzalez.kuka.com@kernel.org> wrote:
>>
>> From: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
>>
>> VM_NOHUGEPAGE is a no-op if CONFIG_TRANSPARENT_HUGEPAGE is disabled. So
>> it makes no sense to return an error when calling madvise() with
>> MADV_NOHUGEPAGE in that case.
>>
>> Suggested-by: Matthew Wilcox <willy@infradead.org>
>> Signed-off-by: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
>> ---
>> https://lore.kernel.org/linux-mm/20250502-map-map_stack-to-vm_nohugepage-only-if-thp-is-enabled-v1-1-113cc634cd51@kuka.com
>>
>> Here it is presented as a separate thread to avoid mixing stable and
>> non-stable patches.
>>
>> This change makes calling madvise(addr, size, MADV_NOHUGEPAGE) on !THP
>> kernels to return 0 instead of -EINVAL.
>> ---
>>  include/linux/huge_mm.h | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
>> index e893d546a49f464f7586db639fe216231f03651a..5fca742dc5ba784ffccea055b07247707d16cc67 100644
>> --- a/include/linux/huge_mm.h
>> +++ b/include/linux/huge_mm.h
>> @@ -509,6 +509,8 @@ bool unmap_huge_pmd_locked(struct vm_area_struct *vma, unsigned long addr,
>>
>>  #else /* CONFIG_TRANSPARENT_HUGEPAGE */
>>
>> +#include <uapi/asm/mman.h>
>> +
>>  static inline bool folio_test_pmd_mappable(struct folio *folio)
>>  {
>>         return false;
>> @@ -598,6 +600,9 @@ static inline bool unmap_huge_pmd_locked(struct vm_area_struct *vma,
>>  static inline int hugepage_madvise(struct vm_area_struct *vma,
>>                                    unsigned long *vm_flags, int advice)
>>  {
>> +       /* On a !THP kernel, MADV_NOHUGEPAGE is a no-op, but MADV_NOHUGEPAGE is not supported */
> 
> Do you mean "but MADV_HUGEPAGE is not supported"? Just want to make
> sure; it seems like no one else has asked about this yet.
>

Yes, this is a typo. It should be MADV_HUGEPAGE. Thanks!

>> +       if (advice == MADV_NOHUGEPAGE)
>> +               return 0;
>>         return -EINVAL;
>>  }
> 



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP
  2025-05-15  7:03   ` Ignacio Moreno Gonzalez
@ 2025-05-15 13:03     ` Lorenzo Stoakes
  2025-05-15 14:42       ` Ignacio Moreno Gonzalez
  0 siblings, 1 reply; 20+ messages in thread
From: Lorenzo Stoakes @ 2025-05-15 13:03 UTC (permalink / raw)
  To: Ignacio Moreno Gonzalez
  Cc: James Houghton, Andrew Morton, Liam R. Howlett, yang, willy,
	linux-mm, linux-kernel

Hi Igancio,

I can package this up in a series with the fix below and with the
appropriate tags (signed-off by you) to enforce the correct ordering
between the s390 patch and this one if that works for you?

Let me know if my sending this revised patch in series with these changes
and your signed-off works for you?

Cheers, Lorenzo

On Thu, May 15, 2025 at 09:03:19AM +0200, Ignacio Moreno Gonzalez wrote:
> On 5/14/2025 10:15 PM, James Houghton wrote:
> > On Thu, May 8, 2025 at 3:20 AM Ignacio Moreno Gonzalez via B4 Relay
> > <devnull+Ignacio.MorenoGonzalez.kuka.com@kernel.org> wrote:
> >>
> >> From: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
> >>
> >> VM_NOHUGEPAGE is a no-op if CONFIG_TRANSPARENT_HUGEPAGE is disabled. So
> >> it makes no sense to return an error when calling madvise() with
> >> MADV_NOHUGEPAGE in that case.
> >>
> >> Suggested-by: Matthew Wilcox <willy@infradead.org>
> >> Signed-off-by: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
> >> ---
> >> https://lore.kernel.org/linux-mm/20250502-map-map_stack-to-vm_nohugepage-only-if-thp-is-enabled-v1-1-113cc634cd51@kuka.com
> >>
> >> Here it is presented as a separate thread to avoid mixing stable and
> >> non-stable patches.
> >>
> >> This change makes calling madvise(addr, size, MADV_NOHUGEPAGE) on !THP
> >> kernels to return 0 instead of -EINVAL.
> >> ---
> >>  include/linux/huge_mm.h | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> >> index e893d546a49f464f7586db639fe216231f03651a..5fca742dc5ba784ffccea055b07247707d16cc67 100644
> >> --- a/include/linux/huge_mm.h
> >> +++ b/include/linux/huge_mm.h
> >> @@ -509,6 +509,8 @@ bool unmap_huge_pmd_locked(struct vm_area_struct *vma, unsigned long addr,
> >>
> >>  #else /* CONFIG_TRANSPARENT_HUGEPAGE */
> >>
> >> +#include <uapi/asm/mman.h>
> >> +
> >>  static inline bool folio_test_pmd_mappable(struct folio *folio)
> >>  {
> >>         return false;
> >> @@ -598,6 +600,9 @@ static inline bool unmap_huge_pmd_locked(struct vm_area_struct *vma,
> >>  static inline int hugepage_madvise(struct vm_area_struct *vma,
> >>                                    unsigned long *vm_flags, int advice)
> >>  {
> >> +       /* On a !THP kernel, MADV_NOHUGEPAGE is a no-op, but MADV_NOHUGEPAGE is not supported */
> >
> > Do you mean "but MADV_HUGEPAGE is not supported"? Just want to make
> > sure; it seems like no one else has asked about this yet.
> >
>
> Yes, this is a typo. It should be MADV_HUGEPAGE. Thanks!
>
> >> +       if (advice == MADV_NOHUGEPAGE)
> >> +               return 0;
> >>         return -EINVAL;
> >>  }
> >
>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP
  2025-05-15 13:03     ` Lorenzo Stoakes
@ 2025-05-15 14:42       ` Ignacio Moreno Gonzalez
  0 siblings, 0 replies; 20+ messages in thread
From: Ignacio Moreno Gonzalez @ 2025-05-15 14:42 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: James Houghton, Andrew Morton, Liam R. Howlett, yang, willy,
	linux-mm, linux-kernel

On 5/15/2025 3:03 PM, Lorenzo Stoakes wrote:
> Hi Igancio,
> 
> I can package this up in a series with the fix below and with the
> appropriate tags (signed-off by you) to enforce the correct ordering
> between the s390 patch and this one if that works for you?
> 
> Let me know if my sending this revised patch in series with these changes
> and your signed-off works for you?

Hi Lorenzo,

Yes, that's fine :) Thanks!

> Cheers, Lorenzo
> 
> On Thu, May 15, 2025 at 09:03:19AM +0200, Ignacio Moreno Gonzalez wrote:
>> On 5/14/2025 10:15 PM, James Houghton wrote:
>>> On Thu, May 8, 2025 at 3:20 AM Ignacio Moreno Gonzalez via B4 Relay
>>> <devnull+Ignacio.MorenoGonzalez.kuka.com@kernel.org> wrote:
>>>>
>>>> From: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
>>>>
>>>> VM_NOHUGEPAGE is a no-op if CONFIG_TRANSPARENT_HUGEPAGE is disabled. So
>>>> it makes no sense to return an error when calling madvise() with
>>>> MADV_NOHUGEPAGE in that case.
>>>>
>>>> Suggested-by: Matthew Wilcox <willy@infradead.org>
>>>> Signed-off-by: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
>>>> ---
>>>> https://lore.kernel.org/linux-mm/20250502-map-map_stack-to-vm_nohugepage-only-if-thp-is-enabled-v1-1-113cc634cd51@kuka.com
>>>>
>>>> Here it is presented as a separate thread to avoid mixing stable and
>>>> non-stable patches.
>>>>
>>>> This change makes calling madvise(addr, size, MADV_NOHUGEPAGE) on !THP
>>>> kernels to return 0 instead of -EINVAL.
>>>> ---
>>>>  include/linux/huge_mm.h | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
>>>> index e893d546a49f464f7586db639fe216231f03651a..5fca742dc5ba784ffccea055b07247707d16cc67 100644
>>>> --- a/include/linux/huge_mm.h
>>>> +++ b/include/linux/huge_mm.h
>>>> @@ -509,6 +509,8 @@ bool unmap_huge_pmd_locked(struct vm_area_struct *vma, unsigned long addr,
>>>>
>>>>  #else /* CONFIG_TRANSPARENT_HUGEPAGE */
>>>>
>>>> +#include <uapi/asm/mman.h>
>>>> +
>>>>  static inline bool folio_test_pmd_mappable(struct folio *folio)
>>>>  {
>>>>         return false;
>>>> @@ -598,6 +600,9 @@ static inline bool unmap_huge_pmd_locked(struct vm_area_struct *vma,
>>>>  static inline int hugepage_madvise(struct vm_area_struct *vma,
>>>>                                    unsigned long *vm_flags, int advice)
>>>>  {
>>>> +       /* On a !THP kernel, MADV_NOHUGEPAGE is a no-op, but MADV_NOHUGEPAGE is not supported */
>>>
>>> Do you mean "but MADV_HUGEPAGE is not supported"? Just want to make
>>> sure; it seems like no one else has asked about this yet.
>>>
>>
>> Yes, this is a typo. It should be MADV_HUGEPAGE. Thanks!
>>
>>>> +       if (advice == MADV_NOHUGEPAGE)
>>>> +               return 0;
>>>>         return -EINVAL;
>>>>  }
>>>
>>
> 



^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2025-05-15 14:42 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-08 10:20 [PATCH] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP Ignacio Moreno Gonzalez via B4 Relay
2025-05-08 11:49 ` Lorenzo Stoakes
2025-05-08 15:06 ` David Hildenbrand
2025-05-08 15:32 ` Yang Shi
2025-05-08 19:04 ` Liam R. Howlett
2025-05-09  5:47   ` Ignacio Moreno Gonzalez
2025-05-09 17:24     ` Liam R. Howlett
2025-05-14 13:52 ` Lorenzo Stoakes
2025-05-14 15:07   ` Ignacio Moreno Gonzalez
2025-05-14 15:25     ` Yang Shi
2025-05-14 15:29       ` Lorenzo Stoakes
2025-05-14 16:54         ` Yang Shi
2025-05-14 15:27     ` Lorenzo Stoakes
2025-05-14 15:59       ` Ignacio Moreno Gonzalez
2025-05-14 22:36         ` Andrew Morton
2025-05-15  5:30           ` Lorenzo Stoakes
2025-05-14 20:15 ` James Houghton
2025-05-15  7:03   ` Ignacio Moreno Gonzalez
2025-05-15 13:03     ` Lorenzo Stoakes
2025-05-15 14:42       ` Ignacio Moreno Gonzalez

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).