Re: [PATCH v3 00/12] mm: add `const` to lots of pointer parameters

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

+cc missing people/lists.

Max, this is the 3rd time I've asked (and others have asked too), and I'm
trying to be nice here, but you are not cc'ing the correct people.

For ease of reference:

$ scripts/get_maintainer.pl --nogit-blame arch/arm/include/asm/highmem.h \
	arch/parisc/include/asm/processor.h \
	arch/parisc/kernel/sys_parisc.c \
	arch/s390/mm/mmap.c \
	arch/sparc/kernel/sys_sparc_64.c \
	arch/x86/mm/mmap.c \
	arch/xtensa/include/asm/highmem.h \
	include/linux/fs.h \
	include/linux/highmem-internal.h \
	include/linux/highmem.h \
	include/linux/mm.h \
	include/linux/mm_inline.h \
	include/linux/mm_types.h \
	include/linux/mmzone.h \
	include/linux/pagemap.h \
	include/linux/sched/mm.h \
	include/linux/shmem_fs.h \
	mm/highmem.c \
	mm/oom_kill.c \
	mm/shmem.c \
	mm/util.c

Gives you what you need, or you can just generate the patch set and do

$ scripts/get_maintainer.pl --nogit-blame *.patch

And filter out anybody who's not listed as a maintainer or reviewer.

Then cc everybody on all mails in series.

On Mon, Sep 01, 2025 at 08:12:11AM +0200, Max Kellermann wrote:
> For improved const-correctness.

The cover letter is included upstream, so you need to expand upon this.

Something like 'In order to ensure better const correctness, we wish to
mark read-only paramaeters const within mm, but to do so we must start at
the bottom of the call graph and work our way up, this series lays the
foundation for this.'.

So I think drop all this stuff about review into the vX -> vY bit, and move:

	Establishing const-correctness in this low-level part of the kernel
	enables doing the same in higher-level parts, e.g. filesystems.

Up here.

>
> This work was initially posted here:
>  https://lore.kernel.org/lkml/20250827192233.447920-1-max.kellermann@ionos.com/
>
> .. but got rejected by Lorenzo Stoakes:
>  https://lore.kernel.org/lkml/d6bf808d-7d74-4e22-ac4b-a6d1f4892262@lucifer.local/

:)

This is the nature of review, it's iterative. Given this cover letter is
included upstream this isn't really hugely useful.

To be clear - I am actually _very much_ in favour of us trying to attack
this, it's just how we do it, how we structure it and how we proceed moving
forwards.

Hence why I reviewed your 'taster' change and agreed with David's
suggestion for a structured way forwards.

>
> David Hildenbrand and Lorenzo Stoakes suggested splitting the patch
> into smaller chunks.  My second attempt with one smaller patch was met
> with agreement:
>
>  https://lore.kernel.org/lkml/20250828130311.772993-1-max.kellermann@ionos.com/
>
> Now this is the rest of the initial patch in small pieces, plus some
> more.
>
> Establishing const-correctness in this low-level part of the kernel
> enables doing the same in higher-level parts, e.g. filesystems.
>
> Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
> ---
> v1 -> v2:
> - made several parameter values const (i.e. the pointer address, not
>   just the pointed-to memory), as suggested by Andrew Morton and
>   Yuanchu Xie
> - drop existing+obsolete "extern" keywords on lines modified by these
>   patches (suggested by Vishal Moola)
> - add missing parameter names on lines modified by these patches
>   (suggested by Vishal Moola)
> - more "const" pointers (e.g. the task_struct passed to
>   process_shares_mm())
> - add missing "const" to s390, fixing s390 build failure
> - moved the mmap_is_legacy() change in arch/s390/mm/mmap.c from 08/12
>   to 06/12 (suggested by Vishal Moola)
>
> v2 -> v3:
> - remove garbage from 06/12

Has this dealt with the build bot issues found in v2? I guess this is what
was 08/12 then right?

> - changed tags on subject line (suggested by Matthew Wilcox)

This is nice, but to be nitty could you do it in reverse order and ideally
:) put a lore link to previous versions? It's super useful.

>
> Max Kellermann (12):
>   mm/shmem: add `const` to lots of pointer parameters
>   mm/pagemap: add `const` to lots of pointer parameters
>   mm/mmzone: add `const` to lots of pointer parameters
>   fs: add `const` to several pointer parameters
>   mm/oom_kill: add `const` to pointer parameter
>   mm/util, s390: add `const` to several pointer parameters
>   parisc: add `const` to mmap_upper_limit() parameter
>   mm/util, s390, sparc, x86: add const to arch_pick_mmap_layout()
>     parameter
>   mm/mm_types: add `const` to several pointer parameters
>   mm/mm_inline: add `const` to lots of pointer parameters
>   mm: add `const` to lots of pointer parameters
>   mm/highmem: add `const` to lots of pointer parameters

Hm I wonder if per-file is the rigth approach, but let me read through the
series and see.

>
>  arch/arm/include/asm/highmem.h      |  6 +--
>  arch/parisc/include/asm/processor.h |  2 +-
>  arch/parisc/kernel/sys_parisc.c     |  2 +-
>  arch/s390/mm/mmap.c                 |  7 ++--
>  arch/sparc/kernel/sys_sparc_64.c    |  3 +-
>  arch/x86/mm/mmap.c                  |  7 ++--
>  arch/xtensa/include/asm/highmem.h   |  2 +-
>  include/linux/fs.h                  |  7 ++--
>  include/linux/highmem-internal.h    | 38 ++++++++++---------
>  include/linux/highmem.h             |  8 ++--
>  include/linux/mm.h                  | 48 +++++++++++------------
>  include/linux/mm_inline.h           | 26 +++++++------
>  include/linux/mm_types.h            |  4 +-
>  include/linux/mmzone.h              | 42 ++++++++++----------
>  include/linux/pagemap.h             | 59 +++++++++++++++--------------
>  include/linux/sched/mm.h            |  4 +-
>  include/linux/shmem_fs.h            |  4 +-
>  mm/highmem.c                        | 10 ++---
>  mm/oom_kill.c                       |  3 +-
>  mm/shmem.c                          |  6 +--
>  mm/util.c                           | 20 ++++++----
>  21 files changed, 162 insertions(+), 146 deletions(-)
>
> --
> 2.47.2
>

Thanks, Lorenzo