[PATCH v2 3/4] tools/testing/vma: eliminate dependency on vma->__vm_flags

Lorenzo Stoakes posted 4 patches 2 months, 3 weeks ago
There is a newer version of this series
[PATCH v2 3/4] tools/testing/vma: eliminate dependency on vma->__vm_flags
Posted by Lorenzo Stoakes 2 months, 3 weeks ago
The userland VMA test code relied on an internal implementation detail -
the existence of vma->__vm_flags to directly access VMA flags. There is no
need to do so when we have the vm_flags_*() helper functions available.

This is both ugly, but also a subsequent commit will eliminate this field
altogether so this will shortly become broken.

This patch has us utilise the helper functions instead.

Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
---
 tools/testing/vma/vma.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/tools/testing/vma/vma.c b/tools/testing/vma/vma.c
index ee9d3547c421..fc77fa3f66f0 100644
--- a/tools/testing/vma/vma.c
+++ b/tools/testing/vma/vma.c
@@ -69,18 +69,18 @@ static struct vm_area_struct *alloc_vma(struct mm_struct *mm,
 					pgoff_t pgoff,
 					vm_flags_t vm_flags)
 {
-	struct vm_area_struct *ret = vm_area_alloc(mm);
+	struct vm_area_struct *vma = vm_area_alloc(mm);
 
-	if (ret == NULL)
+	if (vma == NULL)
 		return NULL;
 
-	ret->vm_start = start;
-	ret->vm_end = end;
-	ret->vm_pgoff = pgoff;
-	ret->__vm_flags = vm_flags;
-	vma_assert_detached(ret);
+	vma->vm_start = start;
+	vma->vm_end = end;
+	vma->vm_pgoff = pgoff;
+	vm_flags_reset(vma, vm_flags);
+	vma_assert_detached(vma);
 
-	return ret;
+	return vma;
 }
 
 /* Helper function to allocate a VMA and link it to the tree. */
@@ -713,7 +713,7 @@ static bool test_vma_merge_special_flags(void)
 	for (i = 0; i < ARRAY_SIZE(special_flags); i++) {
 		vm_flags_t special_flag = special_flags[i];
 
-		vma_left->__vm_flags = vm_flags | special_flag;
+		vm_flags_reset(vma_left, vm_flags | special_flag);
 		vmg.vm_flags = vm_flags | special_flag;
 		vma = merge_new(&vmg);
 		ASSERT_EQ(vma, NULL);
@@ -735,7 +735,7 @@ static bool test_vma_merge_special_flags(void)
 	for (i = 0; i < ARRAY_SIZE(special_flags); i++) {
 		vm_flags_t special_flag = special_flags[i];
 
-		vma_left->__vm_flags = vm_flags | special_flag;
+		vm_flags_reset(vma_left, vm_flags | special_flag);
 		vmg.vm_flags = vm_flags | special_flag;
 		vma = merge_existing(&vmg);
 		ASSERT_EQ(vma, NULL);
-- 
2.51.0
Re: [PATCH v2 3/4] tools/testing/vma: eliminate dependency on vma->__vm_flags
Posted by Lorenzo Stoakes 2 months, 2 weeks ago
Hi Andrew,

Some small silly issue here, for some reason I seemed to have vm_flags_reset()
available from the VMA tests when I tested but err, that doesn't seem to be the
case at all.

Again I realise this is in mm-stable so this might be fiddly so we might have to
live with minor bisect pain :(

Cheers, Lorenzo

----8<----
From afe5af105e7d64e39a4280c7fc8b34ad67cf0db0 Mon Sep 17 00:00:00 2001
From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Date: Fri, 21 Nov 2025 17:25:18 +0000
Subject: [PATCH] tools/testing/vma: add missing stub

The vm_flags_reset() function is not available in the userland VMA tests,
so add a stub which const-casts vma->vm_flags and avoids the upcoming
removal of the vma->__vm_flags field.

Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
---
 tools/testing/vma/vma_internal.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/testing/vma/vma_internal.h b/tools/testing/vma/vma_internal.h
index 37fd479d49ce..f90a9b3c1880 100644
--- a/tools/testing/vma/vma_internal.h
+++ b/tools/testing/vma/vma_internal.h
@@ -1760,4 +1760,11 @@ static inline int do_munmap(struct mm_struct *, unsigned long, size_t,
 	return 0;
 }

+static inline void vm_flags_reset(struct vm_area_struct *vma, vm_flags_t flags)
+{
+	vm_flags_t *dst = (vm_flags_t *)(&vma->vm_flags);
+
+	*dst = flags;
+}
+
 #endif	/* __MM_VMA_INTERNAL_H */
--
2.51.2
Re: [PATCH v2 3/4] tools/testing/vma: eliminate dependency on vma->__vm_flags
Posted by Lorenzo Stoakes 2 months, 2 weeks ago
Hi Andrew,

It seems that the ordering of things has messed us up a bit here!

So this patch (3/4) currently introduces an issue where vm_flags_reset() is
missing from the VMA userland tests.

Then 4/4, now with the vma_internal.h fixes in place, puts vm_flags_reset()
back in place (this was my mistake in the series originally!).

But then this fix-patch, now applied as the latest patch from me in mm-new,
breaks the tests again by duplicating this function :)

So, I'm not sure if too late for rebases - if not, then just squash this
into 3/4.

If it is - then just drop this patch altogether and we'll live with this
being broken for a single commit!

Cheers, Lorenzo

On Fri, Nov 21, 2025 at 05:28:15PM +0000, Lorenzo Stoakes wrote:
> Hi Andrew,
>
> Some small silly issue here, for some reason I seemed to have vm_flags_reset()
> available from the VMA tests when I tested but err, that doesn't seem to be the
> case at all.
>
> Again I realise this is in mm-stable so this might be fiddly so we might have to
> live with minor bisect pain :(
>
> Cheers, Lorenzo
>
> ----8<----
> From afe5af105e7d64e39a4280c7fc8b34ad67cf0db0 Mon Sep 17 00:00:00 2001
> From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Date: Fri, 21 Nov 2025 17:25:18 +0000
> Subject: [PATCH] tools/testing/vma: add missing stub
>
> The vm_flags_reset() function is not available in the userland VMA tests,
> so add a stub which const-casts vma->vm_flags and avoids the upcoming
> removal of the vma->__vm_flags field.
>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> ---
>  tools/testing/vma/vma_internal.h | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/tools/testing/vma/vma_internal.h b/tools/testing/vma/vma_internal.h
> index 37fd479d49ce..f90a9b3c1880 100644
> --- a/tools/testing/vma/vma_internal.h
> +++ b/tools/testing/vma/vma_internal.h
> @@ -1760,4 +1760,11 @@ static inline int do_munmap(struct mm_struct *, unsigned long, size_t,
>  	return 0;
>  }
>
> +static inline void vm_flags_reset(struct vm_area_struct *vma, vm_flags_t flags)
> +{
> +	vm_flags_t *dst = (vm_flags_t *)(&vma->vm_flags);
> +
> +	*dst = flags;
> +}
> +
>  #endif	/* __MM_VMA_INTERNAL_H */
> --
> 2.51.2
Re: [PATCH v2 3/4] tools/testing/vma: eliminate dependency on vma->__vm_flags
Posted by Andrew Morton 2 months, 2 weeks ago
On Mon, 24 Nov 2025 12:43:18 +0000 Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:

> It seems that the ordering of things has messed us up a bit here!
> 
> So this patch (3/4) currently introduces an issue where vm_flags_reset() is
> missing from the VMA userland tests.
> 
> Then 4/4, now with the vma_internal.h fixes in place, puts vm_flags_reset()
> back in place (this was my mistake in the series originally!).
> 
> But then this fix-patch, now applied as the latest patch from me in mm-new,
> breaks the tests again by duplicating this function :)
> 
> So, I'm not sure if too late for rebases - if not, then just squash this
> into 3/4.
> 
> If it is - then just drop this patch altogether and we'll live with this
> being broken for a single commit!

All confused.  I dropped the whole series - please resend?
Re: [PATCH v2 3/4] tools/testing/vma: eliminate dependency on vma->__vm_flags
Posted by Lorenzo Stoakes 2 months, 2 weeks ago
On Mon, Nov 24, 2025 at 10:04:36AM -0800, Andrew Morton wrote:
> On Mon, 24 Nov 2025 12:43:18 +0000 Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:
>
> > It seems that the ordering of things has messed us up a bit here!
> >
> > So this patch (3/4) currently introduces an issue where vm_flags_reset() is
> > missing from the VMA userland tests.
> >
> > Then 4/4, now with the vma_internal.h fixes in place, puts vm_flags_reset()
> > back in place (this was my mistake in the series originally!).
> >
> > But then this fix-patch, now applied as the latest patch from me in mm-new,
> > breaks the tests again by duplicating this function :)
> >
> > So, I'm not sure if too late for rebases - if not, then just squash this
> > into 3/4.
> >
> > If it is - then just drop this patch altogether and we'll live with this
> > being broken for a single commit!
>
> All confused.  I dropped the whole series - please resend?

Ack will do shortly.