[PATCH 0/2] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP

[PATCH 0/2] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP

[Lorenzo Stoakes]

Andrew -

I hope the explanation below resolves your query about the header include
(in [0]), let me know if doing this as a series like this works (we need to
enforce the ordering here).

Thanks!

[0]: 20250514153648.598bb031a2e498b1ac505b60@linux-foundation.org



Currently, when somebody attempts to set MADV_NOHUGEPAGE on a system that
does not enable CONFIG_TRANSPARENT_HUGEPAGE the confguration option, this
results in an -EINVAL error arising.

This doesn't really make sense, as to do so is essentially a no-op.

Additionally, the semantics of setting VM_[NO]HUGEPAGE in any case are such
that, should the attribute not apply, nothing will be done.

It therefore makes sense to simply make this operation a noop.

However, a fly in the ointment is that, in order to do so, we must check
against the MADV_NOHUGEPAGE constant. In doing so, we encounter two rather
annoying issues.

The first is that the usual include we would import to get hold of
MADV_NOHUGEPAGE, linux/mman.h, results in a circular dependency:

* If something includes linux/mman.h, we in turn include linux/mm.h prior
  to declaring MADV_NOHUGEPAGE.
* This then, in turn, includes linux/huge_mm.h.
* linux/huge_mm.h declares hugepage_madvise(), which then tries to
  reference MADV_NOHUGEPAGE, and the build fails.

This can be reached in other ways too.

So we work around this by including uapi/asm/mman.h instead, which allows
us to keep hugepage_madvise() inline.

The second issue is that the s390 arch declares PROT_NONE as a value in the
enum prot_type enumeration.

By updating the include in linux/huge_mm.h, we pull in the PROT_NONE
declaration (unavoidably, this is ultimately in
uapi/asm-generic/mman-common.h alongside MADV_NOHUGEPAGE), which collides
with the enumeration value.

To resolve this, we rename PROT_NONE to PROT_TYPE_DUMMY.

The ordering of these patches is critical, the s390 patch must be applied
prior to the MADV_NOHUGEPAGE patch, and therefore the two patches are sent
as a series.

v1:
* Place patches in series.
* Correct typo in comment as per James.

previous patches:
huge_mm.h patch - https://lore.kernel.org/all/20250508-madvise-nohugepage-noop-without-thp-v1-1-e7ceffb197f3@kuka.com/
s390 patch - https://lore.kernel.org/all/20250514163530.119582-1-lorenzo.stoakes@oracle.com/

Ignacio Moreno Gonzalez (1):
  mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP

Lorenzo Stoakes (1):
  KVM: s390: rename PROT_NONE to PROT_TYPE_DUMMY

 arch/s390/kvm/gaccess.c | 8 ++++----
 include/linux/huge_mm.h | 5 +++++
 2 files changed, 9 insertions(+), 4 deletions(-)

--
2.49.0

[PATCH 1/2] KVM: s390: rename PROT_NONE to PROT_TYPE_DUMMY

[Lorenzo Stoakes]

The enum type prot_type declared in arch/s390/kvm/gaccess.c declares an
unfortunate identifier within it - PROT_NONE.

This clashes with the protection bit define from the uapi for mmap()
declared in include/uapi/asm-generic/mman-common.h, which is indeed what
those casually reading this code would assume this to refer to.

This means that any changes which subsequently alter headers in any way
which results in the uapi header being imported here will cause build
errors.

Resolve the issue by renaming PROT_NONE to PROT_TYPE_DUMMY.

Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Suggested-by: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
Fixes: b3cefd6bf16e ("KVM: s390: Pass initialized arg even if unused")
Cc: stable@vger.kernel.org
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202505140943.IgHDa9s7-lkp@intel.com/
Acked-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Acked-by: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
Acked-by: Yang Shi <yang@os.amperecomputing.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
 arch/s390/kvm/gaccess.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
index f6fded15633a..4e5654ad1604 100644
--- a/arch/s390/kvm/gaccess.c
+++ b/arch/s390/kvm/gaccess.c
@@ -318,7 +318,7 @@ enum prot_type {
 	PROT_TYPE_DAT  = 3,
 	PROT_TYPE_IEP  = 4,
 	/* Dummy value for passing an initialized value when code != PGM_PROTECTION */
-	PROT_NONE,
+	PROT_TYPE_DUMMY,
 };
 
 static int trans_exc_ending(struct kvm_vcpu *vcpu, int code, unsigned long gva, u8 ar,
@@ -334,7 +334,7 @@ static int trans_exc_ending(struct kvm_vcpu *vcpu, int code, unsigned long gva,
 	switch (code) {
 	case PGM_PROTECTION:
 		switch (prot) {
-		case PROT_NONE:
+		case PROT_TYPE_DUMMY:
 			/* We should never get here, acts like termination */
 			WARN_ON_ONCE(1);
 			break;
@@ -804,7 +804,7 @@ static int guest_range_to_gpas(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
 			gpa = kvm_s390_real_to_abs(vcpu, ga);
 			if (!kvm_is_gpa_in_memslot(vcpu->kvm, gpa)) {
 				rc = PGM_ADDRESSING;
-				prot = PROT_NONE;
+				prot = PROT_TYPE_DUMMY;
 			}
 		}
 		if (rc)
@@ -962,7 +962,7 @@ int access_guest_with_key(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
 		if (rc == PGM_PROTECTION)
 			prot = PROT_TYPE_KEYC;
 		else
-			prot = PROT_NONE;
+			prot = PROT_TYPE_DUMMY;
 		rc = trans_exc_ending(vcpu, rc, ga, ar, mode, prot, terminate);
 	}
 out_unlock:
-- 
2.49.0

Re: [PATCH 1/2] KVM: s390: rename PROT_NONE to PROT_TYPE_DUMMY

[Liam R. Howlett]

* Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [250515 16:15]:
> The enum type prot_type declared in arch/s390/kvm/gaccess.c declares an
> unfortunate identifier within it - PROT_NONE.
> 
> This clashes with the protection bit define from the uapi for mmap()
> declared in include/uapi/asm-generic/mman-common.h, which is indeed what
> those casually reading this code would assume this to refer to.
> 
> This means that any changes which subsequently alter headers in any way
> which results in the uapi header being imported here will cause build
> errors.
> 
> Resolve the issue by renaming PROT_NONE to PROT_TYPE_DUMMY.
> 
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Suggested-by: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
> Fixes: b3cefd6bf16e ("KVM: s390: Pass initialized arg even if unused")
> Cc: stable@vger.kernel.org
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202505140943.IgHDa9s7-lkp@intel.com/
> Acked-by: Christian Borntraeger <borntraeger@linux.ibm.com>
> Acked-by: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
> Acked-by: Yang Shi <yang@os.amperecomputing.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>

Acked-by: Liam R. Howlett <Liam.Howlett@oracle.com>

> ---
>  arch/s390/kvm/gaccess.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
> index f6fded15633a..4e5654ad1604 100644
> --- a/arch/s390/kvm/gaccess.c
> +++ b/arch/s390/kvm/gaccess.c
> @@ -318,7 +318,7 @@ enum prot_type {
>  	PROT_TYPE_DAT  = 3,
>  	PROT_TYPE_IEP  = 4,
>  	/* Dummy value for passing an initialized value when code != PGM_PROTECTION */
> -	PROT_NONE,
> +	PROT_TYPE_DUMMY,
>  };
>  
>  static int trans_exc_ending(struct kvm_vcpu *vcpu, int code, unsigned long gva, u8 ar,
> @@ -334,7 +334,7 @@ static int trans_exc_ending(struct kvm_vcpu *vcpu, int code, unsigned long gva,
>  	switch (code) {
>  	case PGM_PROTECTION:
>  		switch (prot) {
> -		case PROT_NONE:
> +		case PROT_TYPE_DUMMY:
>  			/* We should never get here, acts like termination */
>  			WARN_ON_ONCE(1);
>  			break;
> @@ -804,7 +804,7 @@ static int guest_range_to_gpas(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
>  			gpa = kvm_s390_real_to_abs(vcpu, ga);
>  			if (!kvm_is_gpa_in_memslot(vcpu->kvm, gpa)) {
>  				rc = PGM_ADDRESSING;
> -				prot = PROT_NONE;
> +				prot = PROT_TYPE_DUMMY;
>  			}
>  		}
>  		if (rc)
> @@ -962,7 +962,7 @@ int access_guest_with_key(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
>  		if (rc == PGM_PROTECTION)
>  			prot = PROT_TYPE_KEYC;
>  		else
> -			prot = PROT_NONE;
> +			prot = PROT_TYPE_DUMMY;
>  		rc = trans_exc_ending(vcpu, rc, ga, ar, mode, prot, terminate);
>  	}
>  out_unlock:
> -- 
> 2.49.0
> 

Re: [PATCH 1/2] KVM: s390: rename PROT_NONE to PROT_TYPE_DUMMY

[Lorenzo Stoakes]

On Fri, May 16, 2025 at 11:39:19AM -0400, Liam R. Howlett wrote:
> * Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [250515 16:15]:
> > The enum type prot_type declared in arch/s390/kvm/gaccess.c declares an
> > unfortunate identifier within it - PROT_NONE.
> >
> > This clashes with the protection bit define from the uapi for mmap()
> > declared in include/uapi/asm-generic/mman-common.h, which is indeed what
> > those casually reading this code would assume this to refer to.
> >
> > This means that any changes which subsequently alter headers in any way
> > which results in the uapi header being imported here will cause build
> > errors.
> >
> > Resolve the issue by renaming PROT_NONE to PROT_TYPE_DUMMY.
> >
> > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > Suggested-by: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
> > Fixes: b3cefd6bf16e ("KVM: s390: Pass initialized arg even if unused")
> > Cc: stable@vger.kernel.org
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202505140943.IgHDa9s7-lkp@intel.com/
> > Acked-by: Christian Borntraeger <borntraeger@linux.ibm.com>
> > Acked-by: Ignacio Moreno Gonzalez <Ignacio.MorenoGonzalez@kuka.com>
> > Acked-by: Yang Shi <yang@os.amperecomputing.com>
> > Reviewed-by: David Hildenbrand <david@redhat.com>
>
> Acked-by: Liam R. Howlett <Liam.Howlett@oracle.com>

Thanks!

>
> > ---
> >  arch/s390/kvm/gaccess.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
> > index f6fded15633a..4e5654ad1604 100644
> > --- a/arch/s390/kvm/gaccess.c
> > +++ b/arch/s390/kvm/gaccess.c
> > @@ -318,7 +318,7 @@ enum prot_type {
> >  	PROT_TYPE_DAT  = 3,
> >  	PROT_TYPE_IEP  = 4,
> >  	/* Dummy value for passing an initialized value when code != PGM_PROTECTION */
> > -	PROT_NONE,
> > +	PROT_TYPE_DUMMY,
> >  };
> >
> >  static int trans_exc_ending(struct kvm_vcpu *vcpu, int code, unsigned long gva, u8 ar,
> > @@ -334,7 +334,7 @@ static int trans_exc_ending(struct kvm_vcpu *vcpu, int code, unsigned long gva,
> >  	switch (code) {
> >  	case PGM_PROTECTION:
> >  		switch (prot) {
> > -		case PROT_NONE:
> > +		case PROT_TYPE_DUMMY:
> >  			/* We should never get here, acts like termination */
> >  			WARN_ON_ONCE(1);
> >  			break;
> > @@ -804,7 +804,7 @@ static int guest_range_to_gpas(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
> >  			gpa = kvm_s390_real_to_abs(vcpu, ga);
> >  			if (!kvm_is_gpa_in_memslot(vcpu->kvm, gpa)) {
> >  				rc = PGM_ADDRESSING;
> > -				prot = PROT_NONE;
> > +				prot = PROT_TYPE_DUMMY;
> >  			}
> >  		}
> >  		if (rc)
> > @@ -962,7 +962,7 @@ int access_guest_with_key(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
> >  		if (rc == PGM_PROTECTION)
> >  			prot = PROT_TYPE_KEYC;
> >  		else
> > -			prot = PROT_NONE;
> > +			prot = PROT_TYPE_DUMMY;
> >  		rc = trans_exc_ending(vcpu, rc, ga, ar, mode, prot, terminate);
> >  	}
> >  out_unlock:
> > --
> > 2.49.0
> >

[PATCH 2/2] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP

[Lorenzo Stoakes]

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>
Reviewed-by: Yang Shi <yang@os.amperecomputing.com>
Acked-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
---
 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 2f190c90192d..1a8082c61e01 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -506,6 +506,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;
@@ -595,6 +597,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_HUGEPAGE is not supported */
+	if (advice == MADV_NOHUGEPAGE)
+		return 0;
 	return -EINVAL;
 }
 
-- 
2.49.0

Re: [PATCH 2/2] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP

[Liam R. Howlett]

* Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [250515 16:15]:
> 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>
> Reviewed-by: Yang Shi <yang@os.amperecomputing.com>
> Acked-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

Nice to see you review this for yourself :)

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

Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>

> ---
>  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 2f190c90192d..1a8082c61e01 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -506,6 +506,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;
> @@ -595,6 +597,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_HUGEPAGE is not supported */
> +	if (advice == MADV_NOHUGEPAGE)
> +		return 0;
>  	return -EINVAL;
>  }
>  
> -- 
> 2.49.0
> 

Re: [PATCH 2/2] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP

[Lorenzo Stoakes]

On Fri, May 16, 2025 at 11:40:23AM -0400, Liam R. Howlett wrote:
> * Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [250515 16:15]:
> > 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>
> > Reviewed-by: Yang Shi <yang@os.amperecomputing.com>
> > Acked-by: David Hildenbrand <david@redhat.com>
> > Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>
> Nice to see you review this for yourself :)

Haha yeah... this is a Lorenzo-getting-confused-by-kernel-process situation
again, this is 100% Ignacio's patch, I just bundled it up in this series to
enforce ordering.

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

...But of course I did format-patch -s so I signed it off as well :P

At any rate the From: field and Ignacio's S-o-b should make everything correct
in the wash. I think.

Andrew - this is Ignacio's patch for avoidance of doubt :P

>
> Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>
>
> > ---
> >  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 2f190c90192d..1a8082c61e01 100644
> > --- a/include/linux/huge_mm.h
> > +++ b/include/linux/huge_mm.h
> > @@ -506,6 +506,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;
> > @@ -595,6 +597,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_HUGEPAGE is not supported */
> > +	if (advice == MADV_NOHUGEPAGE)
> > +		return 0;
> >  	return -EINVAL;
> >  }
> >
> > --
> > 2.49.0
> >
>

Re: [PATCH 2/2] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP

[Liam R. Howlett]

* Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [250516 11:43]:
> On Fri, May 16, 2025 at 11:40:23AM -0400, Liam R. Howlett wrote:
> > * Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [250515 16:15]:
> > > 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>
> > > Reviewed-by: Yang Shi <yang@os.amperecomputing.com>
> > > Acked-by: David Hildenbrand <david@redhat.com>
> > > Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> >
> > Nice to see you review this for yourself :)
> 
> Haha yeah... this is a Lorenzo-getting-confused-by-kernel-process situation
> again, this is 100% Ignacio's patch, I just bundled it up in this series to
> enforce ordering.
> 
> >
> > > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> 
> ...But of course I did format-patch -s so I signed it off as well :P
> 
> At any rate the From: field and Ignacio's S-o-b should make everything correct
> in the wash. I think.
> 
> Andrew - this is Ignacio's patch for avoidance of doubt :P

Oh yes, I missed the initial sign off.  I think this is okay, but maybe
you didn't need your own RB?

> 
> >
> > Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>
> >
> > > ---
> > >  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 2f190c90192d..1a8082c61e01 100644
> > > --- a/include/linux/huge_mm.h
> > > +++ b/include/linux/huge_mm.h
> > > @@ -506,6 +506,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;
> > > @@ -595,6 +597,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_HUGEPAGE is not supported */
> > > +	if (advice == MADV_NOHUGEPAGE)
> > > +		return 0;
> > >  	return -EINVAL;
> > >  }
> > >
> > > --
> > > 2.49.0
> > >
> >

Re: [PATCH 0/2] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP

[Lorenzo Stoakes]

Andrew -

OK, I realise there's an issue here with patch 2/2. We're not accounting
for the fact that madvise() will reject this _anyway_ because
madvise_behavior_valid() will reject it.

I've tried to be especially helpful here to aid Ignacio in his early
contributions, but I think it's best now (if you don't mind Igancio) for me
to figure out a better solution after the merge window.

We're late in the cycle now so I will just resend the 1st patch (for s390)
separately if you're happy to take that for 6.16? It's a simple rename of
an entirely static identifier so should present no risk, and is approved by
the arch maintainers who have also agreed for it to come through the mm
tree.

Apologies for the mess!

Cheers, Lorenzo

On Thu, May 15, 2025 at 09:15:44PM +0100, Lorenzo Stoakes wrote:
> Andrew -
>
> I hope the explanation below resolves your query about the header include
> (in [0]), let me know if doing this as a series like this works (we need to
> enforce the ordering here).
>
> Thanks!
>
> [0]: 20250514153648.598bb031a2e498b1ac505b60@linux-foundation.org
>
>
>
> Currently, when somebody attempts to set MADV_NOHUGEPAGE on a system that
> does not enable CONFIG_TRANSPARENT_HUGEPAGE the confguration option, this
> results in an -EINVAL error arising.
>
> This doesn't really make sense, as to do so is essentially a no-op.
>
> Additionally, the semantics of setting VM_[NO]HUGEPAGE in any case are such
> that, should the attribute not apply, nothing will be done.
>
> It therefore makes sense to simply make this operation a noop.
>
> However, a fly in the ointment is that, in order to do so, we must check
> against the MADV_NOHUGEPAGE constant. In doing so, we encounter two rather
> annoying issues.
>
> The first is that the usual include we would import to get hold of
> MADV_NOHUGEPAGE, linux/mman.h, results in a circular dependency:
>
> * If something includes linux/mman.h, we in turn include linux/mm.h prior
>   to declaring MADV_NOHUGEPAGE.
> * This then, in turn, includes linux/huge_mm.h.
> * linux/huge_mm.h declares hugepage_madvise(), which then tries to
>   reference MADV_NOHUGEPAGE, and the build fails.
>
> This can be reached in other ways too.
>
> So we work around this by including uapi/asm/mman.h instead, which allows
> us to keep hugepage_madvise() inline.
>
> The second issue is that the s390 arch declares PROT_NONE as a value in the
> enum prot_type enumeration.
>
> By updating the include in linux/huge_mm.h, we pull in the PROT_NONE
> declaration (unavoidably, this is ultimately in
> uapi/asm-generic/mman-common.h alongside MADV_NOHUGEPAGE), which collides
> with the enumeration value.
>
> To resolve this, we rename PROT_NONE to PROT_TYPE_DUMMY.
>
> The ordering of these patches is critical, the s390 patch must be applied
> prior to the MADV_NOHUGEPAGE patch, and therefore the two patches are sent
> as a series.
>
> v1:
> * Place patches in series.
> * Correct typo in comment as per James.
>
> previous patches:
> huge_mm.h patch - https://lore.kernel.org/all/20250508-madvise-nohugepage-noop-without-thp-v1-1-e7ceffb197f3@kuka.com/
> s390 patch - https://lore.kernel.org/all/20250514163530.119582-1-lorenzo.stoakes@oracle.com/
>
> Ignacio Moreno Gonzalez (1):
>   mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP
>
> Lorenzo Stoakes (1):
>   KVM: s390: rename PROT_NONE to PROT_TYPE_DUMMY
>
>  arch/s390/kvm/gaccess.c | 8 ++++----
>  include/linux/huge_mm.h | 5 +++++
>  2 files changed, 9 insertions(+), 4 deletions(-)
>
> --
> 2.49.0

Re: [PATCH 0/2] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP

[Yang Shi]

On 5/19/25 7:43 AM, Lorenzo Stoakes wrote:
> Andrew -
>
> OK, I realise there's an issue here with patch 2/2. We're not accounting
> for the fact that madvise() will reject this _anyway_ because
> madvise_behavior_valid() will reject it.

Good catch. The purpose of this patch is to make MADV_NOHUGEPAGE a 
no-op, so we can just simply bail out early? The point of madvise 
behavior check is to avoid taking mmap_lock and walking vmas for invalid 
behavior, but it doesn't consider no-op (I treat op-op as valid but do 
nothing), so if we know this advise is a no-op, we just bail out by 
returning 0.

Maybe MADV_UNMERGEABLE should be no-op too, it returns 0 for !KSM anyway.

Thanks,
Yang


>
> I've tried to be especially helpful here to aid Ignacio in his early
> contributions, but I think it's best now (if you don't mind Igancio) for me
> to figure out a better solution after the merge window.
>
> We're late in the cycle now so I will just resend the 1st patch (for s390)
> separately if you're happy to take that for 6.16? It's a simple rename of
> an entirely static identifier so should present no risk, and is approved by
> the arch maintainers who have also agreed for it to come through the mm
> tree.
>
> Apologies for the mess!
>
> Cheers, Lorenzo
>
> On Thu, May 15, 2025 at 09:15:44PM +0100, Lorenzo Stoakes wrote:
>> Andrew -
>>
>> I hope the explanation below resolves your query about the header include
>> (in [0]), let me know if doing this as a series like this works (we need to
>> enforce the ordering here).
>>
>> Thanks!
>>
>> [0]: 20250514153648.598bb031a2e498b1ac505b60@linux-foundation.org
>>
>>
>>
>> Currently, when somebody attempts to set MADV_NOHUGEPAGE on a system that
>> does not enable CONFIG_TRANSPARENT_HUGEPAGE the confguration option, this
>> results in an -EINVAL error arising.
>>
>> This doesn't really make sense, as to do so is essentially a no-op.
>>
>> Additionally, the semantics of setting VM_[NO]HUGEPAGE in any case are such
>> that, should the attribute not apply, nothing will be done.
>>
>> It therefore makes sense to simply make this operation a noop.
>>
>> However, a fly in the ointment is that, in order to do so, we must check
>> against the MADV_NOHUGEPAGE constant. In doing so, we encounter two rather
>> annoying issues.
>>
>> The first is that the usual include we would import to get hold of
>> MADV_NOHUGEPAGE, linux/mman.h, results in a circular dependency:
>>
>> * If something includes linux/mman.h, we in turn include linux/mm.h prior
>>    to declaring MADV_NOHUGEPAGE.
>> * This then, in turn, includes linux/huge_mm.h.
>> * linux/huge_mm.h declares hugepage_madvise(), which then tries to
>>    reference MADV_NOHUGEPAGE, and the build fails.
>>
>> This can be reached in other ways too.
>>
>> So we work around this by including uapi/asm/mman.h instead, which allows
>> us to keep hugepage_madvise() inline.
>>
>> The second issue is that the s390 arch declares PROT_NONE as a value in the
>> enum prot_type enumeration.
>>
>> By updating the include in linux/huge_mm.h, we pull in the PROT_NONE
>> declaration (unavoidably, this is ultimately in
>> uapi/asm-generic/mman-common.h alongside MADV_NOHUGEPAGE), which collides
>> with the enumeration value.
>>
>> To resolve this, we rename PROT_NONE to PROT_TYPE_DUMMY.
>>
>> The ordering of these patches is critical, the s390 patch must be applied
>> prior to the MADV_NOHUGEPAGE patch, and therefore the two patches are sent
>> as a series.
>>
>> v1:
>> * Place patches in series.
>> * Correct typo in comment as per James.
>>
>> previous patches:
>> huge_mm.h patch - https://lore.kernel.org/all/20250508-madvise-nohugepage-noop-without-thp-v1-1-e7ceffb197f3@kuka.com/
>> s390 patch - https://lore.kernel.org/all/20250514163530.119582-1-lorenzo.stoakes@oracle.com/
>>
>> Ignacio Moreno Gonzalez (1):
>>    mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP
>>
>> Lorenzo Stoakes (1):
>>    KVM: s390: rename PROT_NONE to PROT_TYPE_DUMMY
>>
>>   arch/s390/kvm/gaccess.c | 8 ++++----
>>   include/linux/huge_mm.h | 5 +++++
>>   2 files changed, 9 insertions(+), 4 deletions(-)
>>
>> --
>> 2.49.0

Re: [PATCH 0/2] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP

[Lorenzo Stoakes]

On Mon, May 19, 2025 at 12:31:04PM -0700, Yang Shi wrote:
>
>
> On 5/19/25 7:43 AM, Lorenzo Stoakes wrote:
> > Andrew -
> >
> > OK, I realise there's an issue here with patch 2/2. We're not accounting
> > for the fact that madvise() will reject this _anyway_ because
> > madvise_behavior_valid() will reject it.
>
> Good catch. The purpose of this patch is to make MADV_NOHUGEPAGE a no-op, so
> we can just simply bail out early? The point of madvise behavior check is to
> avoid taking mmap_lock and walking vmas for invalid behavior, but it doesn't
> consider no-op (I treat op-op as valid but do nothing), so if we know this
> advise is a no-op, we just bail out by returning 0.

Yeah I was thinking better to just do it in mm/madvise.c to be honest.

>
> Maybe MADV_UNMERGEABLE should be no-op too, it returns 0 for !KSM anyway.

Ack, yes could maybe bundle up?

Anyway I didn't want to delay the s390 fix any longer and this is a mess now so
better to defer to next cycle I think :>)

>
> Thanks,
> Yang
>
>
> >
> > I've tried to be especially helpful here to aid Ignacio in his early
> > contributions, but I think it's best now (if you don't mind Igancio) for me
> > to figure out a better solution after the merge window.
> >
> > We're late in the cycle now so I will just resend the 1st patch (for s390)
> > separately if you're happy to take that for 6.16? It's a simple rename of
> > an entirely static identifier so should present no risk, and is approved by
> > the arch maintainers who have also agreed for it to come through the mm
> > tree.
> >
> > Apologies for the mess!
> >
> > Cheers, Lorenzo
> >
> > On Thu, May 15, 2025 at 09:15:44PM +0100, Lorenzo Stoakes wrote:
> > > Andrew -
> > >
> > > I hope the explanation below resolves your query about the header include
> > > (in [0]), let me know if doing this as a series like this works (we need to
> > > enforce the ordering here).
> > >
> > > Thanks!
> > >
> > > [0]: 20250514153648.598bb031a2e498b1ac505b60@linux-foundation.org
> > >
> > >
> > >
> > > Currently, when somebody attempts to set MADV_NOHUGEPAGE on a system that
> > > does not enable CONFIG_TRANSPARENT_HUGEPAGE the confguration option, this
> > > results in an -EINVAL error arising.
> > >
> > > This doesn't really make sense, as to do so is essentially a no-op.
> > >
> > > Additionally, the semantics of setting VM_[NO]HUGEPAGE in any case are such
> > > that, should the attribute not apply, nothing will be done.
> > >
> > > It therefore makes sense to simply make this operation a noop.
> > >
> > > However, a fly in the ointment is that, in order to do so, we must check
> > > against the MADV_NOHUGEPAGE constant. In doing so, we encounter two rather
> > > annoying issues.
> > >
> > > The first is that the usual include we would import to get hold of
> > > MADV_NOHUGEPAGE, linux/mman.h, results in a circular dependency:
> > >
> > > * If something includes linux/mman.h, we in turn include linux/mm.h prior
> > >    to declaring MADV_NOHUGEPAGE.
> > > * This then, in turn, includes linux/huge_mm.h.
> > > * linux/huge_mm.h declares hugepage_madvise(), which then tries to
> > >    reference MADV_NOHUGEPAGE, and the build fails.
> > >
> > > This can be reached in other ways too.
> > >
> > > So we work around this by including uapi/asm/mman.h instead, which allows
> > > us to keep hugepage_madvise() inline.
> > >
> > > The second issue is that the s390 arch declares PROT_NONE as a value in the
> > > enum prot_type enumeration.
> > >
> > > By updating the include in linux/huge_mm.h, we pull in the PROT_NONE
> > > declaration (unavoidably, this is ultimately in
> > > uapi/asm-generic/mman-common.h alongside MADV_NOHUGEPAGE), which collides
> > > with the enumeration value.
> > >
> > > To resolve this, we rename PROT_NONE to PROT_TYPE_DUMMY.
> > >
> > > The ordering of these patches is critical, the s390 patch must be applied
> > > prior to the MADV_NOHUGEPAGE patch, and therefore the two patches are sent
> > > as a series.
> > >
> > > v1:
> > > * Place patches in series.
> > > * Correct typo in comment as per James.
> > >
> > > previous patches:
> > > huge_mm.h patch - https://lore.kernel.org/all/20250508-madvise-nohugepage-noop-without-thp-v1-1-e7ceffb197f3@kuka.com/
> > > s390 patch - https://lore.kernel.org/all/20250514163530.119582-1-lorenzo.stoakes@oracle.com/
> > >
> > > Ignacio Moreno Gonzalez (1):
> > >    mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP
> > >
> > > Lorenzo Stoakes (1):
> > >    KVM: s390: rename PROT_NONE to PROT_TYPE_DUMMY
> > >
> > >   arch/s390/kvm/gaccess.c | 8 ++++----
> > >   include/linux/huge_mm.h | 5 +++++
> > >   2 files changed, 9 insertions(+), 4 deletions(-)
> > >
> > > --
> > > 2.49.0
>
>

Re: [PATCH 0/2] mm: madvise: make MADV_NOHUGEPAGE a no-op if !THP

[Ignacio Moreno González]

Internal
From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Sent: Monday, May 19, 2025 16:43

> I've tried to be especially helpful here to aid Ignacio in his early
> contributions, but I think it's best now (if you don't mind Igancio) for me
> to figure out a better solution after the merge window.

Sure, that's fine for me. Thanks!