Re: [RFC Patch 0/5] Make anon_vma operations testable

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

Wei,

NACK the whole series.

I'm really not sure how to get through to you. You were _explicitly_
advised not to send this series. And yet you've sent it anyway.

I mean, I appreciate your enthusiasm and the fact you've made tests here
etc. obviously. And you've clearly put a TON of work in. But I just don't
know why you would when explicitly told not to without at least discussing
it first?

This just isn't a great way of interacting with the community. We're all
human, please try to have some empathy for others here, as I really do try
to have with you as best I can.

This adds a ton of churn and LOCKS IN assumptions about how anon_vma works,
clashes with other series (most notably series I've been working on), takes
away from efforts I want to make to start to join file-backed and anon
reverse mapping logic, separates the two in such a way as to encourage this
to nonly grow and generally isn't conducive to where I want to go with
rmap.

This is part of why I explicitly told you please don't go down this road,
because you're likely to end up doing work that doesn't get used. It's not
a great use of your time either.

Since there's something useful here in tests, I may at a later date come
back to those.

So in order for it not to be an _entirely_ wasted effort, I will come back
to this later when the time is right and progress is made with rmap, and
see if we can extract some value from the testing.

Lorenzo

On Tue, Apr 29, 2025 at 09:06:34AM +0000, Wei Yang wrote:
> There are several anon_vma manipulation functions implemented in mm/rmap.c,
> those concerning anon_vma preparing, cloning and forking, which logically
> could be stand-alone.
>
> This patch series isolates anon_vma manipulation functionality into its own
> file, mm/anon_vma.c, and provides an API to the rest of the kernel in
> include/linux/anon_vma.h.
>
> It also introduce mm/anon_vma_internal.h, which specifies which headers need
> to be imported by anon_vma.c, leading to the very useful property that
> anon_vma.c depends only on include/linux/anon_vma.h and
> mm/anon_vma_internal.h.
>
> This means we can then re-implement anon_vma_internal.h in userland, adding
> shims for kernel mechanisms as required, allowing us to unit test internal
> anon_vma functionality.
>
> This patch series takes advantage of existing shim logic and full userland
> interval tree support contained in tools/testing/rbtree/ and
> tools/include/linux/.
>
> Kernel functionality is stubbed and shimmed as needed in
> tools/testing/anon_vma/ which contains a fully functional userland
> anon_vma_internal.h file and which imports mm/anon_vma.c to be directly tested
> from userland.
>
> Patch 1 split anon_vma related logic to mm/anon_vma.c
> Patch 2 add a simple skeleton testing on simple fault and fork
> Patch 3/4 add tests for mergeable and reusable anon_vma
> Patch 5 assert the anon_vma double-reuse is fixed
>
> Wei Yang (5):
>   mm: move anon_vma manipulation functions to own file
>   anon_vma: add skeleton code for userland testing of anon_vma logic
>   anon_vma: add test for mergeable anon_vma
>   anon_vma: add test for reusable anon_vma
>   anon_vma: add test to assert no double-reuse
>
>  MAINTAINERS                                |   3 +
>  include/linux/anon_vma.h                   | 163 +++++
>  include/linux/rmap.h                       | 147 +---
>  mm/Makefile                                |   2 +-
>  mm/anon_vma.c                              | 396 +++++++++++
>  mm/anon_vma_internal.h                     |  14 +
>  mm/rmap.c                                  | 391 -----------
>  tools/include/linux/rwsem.h                |  10 +
>  tools/include/linux/slab.h                 |   4 +
>  tools/testing/anon_vma/.gitignore          |   3 +
>  tools/testing/anon_vma/Makefile            |  25 +
>  tools/testing/anon_vma/anon_vma.c          | 773 +++++++++++++++++++++
>  tools/testing/anon_vma/anon_vma_internal.h |  88 +++
>  tools/testing/anon_vma/interval_tree.c     |  53 ++
>  tools/testing/anon_vma/linux/atomic.h      |  18 +
>  tools/testing/anon_vma/linux/fs.h          |   6 +
>  tools/testing/anon_vma/linux/mm.h          |  44 ++
>  tools/testing/anon_vma/linux/mm_types.h    |  57 ++
>  tools/testing/anon_vma/linux/mmzone.h      |   6 +
>  tools/testing/anon_vma/linux/rmap.h        |   8 +
>  tools/testing/shared/linux/anon_vma.h      |   7 +
>  21 files changed, 1680 insertions(+), 538 deletions(-)
>  create mode 100644 include/linux/anon_vma.h
>  create mode 100644 mm/anon_vma.c
>  create mode 100644 mm/anon_vma_internal.h
>  create mode 100644 tools/testing/anon_vma/.gitignore
>  create mode 100644 tools/testing/anon_vma/Makefile
>  create mode 100644 tools/testing/anon_vma/anon_vma.c
>  create mode 100644 tools/testing/anon_vma/anon_vma_internal.h
>  create mode 100644 tools/testing/anon_vma/interval_tree.c
>  create mode 100644 tools/testing/anon_vma/linux/atomic.h
>  create mode 100644 tools/testing/anon_vma/linux/fs.h
>  create mode 100644 tools/testing/anon_vma/linux/mm.h
>  create mode 100644 tools/testing/anon_vma/linux/mm_types.h
>  create mode 100644 tools/testing/anon_vma/linux/mmzone.h
>  create mode 100644 tools/testing/anon_vma/linux/rmap.h
>  create mode 100644 tools/testing/shared/linux/anon_vma.h
>
> --
> 2.34.1
>
>