We select certain test functions which either invoke each other,
functions that are already const-ified, or no further functions.
It is therefore relatively trivial to const-ify them, which
provides a basis for further const-ification further up the call
stack.
(Even though seemingly unrelated, this also constifies the pointer
parameter of mmap_is_legacy() in arch/s390/mm/mmap.c because a copy of
the function exists in mm/util.c.)
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
Reviewed-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
---
arch/s390/mm/mmap.c | 2 +-
include/linux/mm.h | 6 +++---
include/linux/pagemap.h | 2 +-
mm/util.c | 11 ++++++-----
4 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/arch/s390/mm/mmap.c b/arch/s390/mm/mmap.c
index 547104ccc22a..c0f619fb9ab3 100644
--- a/arch/s390/mm/mmap.c
+++ b/arch/s390/mm/mmap.c
@@ -27,7 +27,7 @@ static unsigned long stack_maxrandom_size(void)
return STACK_RND_MASK << PAGE_SHIFT;
}
-static inline int mmap_is_legacy(struct rlimit *rlim_stack)
+static inline int mmap_is_legacy(const struct rlimit *const rlim_stack)
{
if (current->personality & ADDR_COMPAT_LAYOUT)
return 1;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index f70c6b4d5f80..23864c3519d6 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -986,7 +986,7 @@ static inline bool vma_is_shmem(const struct vm_area_struct *vma) { return false
static inline bool vma_is_anon_shmem(const struct vm_area_struct *vma) { return false; }
#endif
-int vma_is_stack_for_current(struct vm_area_struct *vma);
+int vma_is_stack_for_current(const struct vm_area_struct *vma);
/* flush_tlb_range() takes a vma, not a mm, and can care about flags */
#define TLB_FLUSH_VMA(mm,flags) { .vm_mm = (mm), .vm_flags = (flags) }
@@ -2585,7 +2585,7 @@ void folio_add_pin(struct folio *folio);
int account_locked_vm(struct mm_struct *mm, unsigned long pages, bool inc);
int __account_locked_vm(struct mm_struct *mm, unsigned long pages, bool inc,
- struct task_struct *task, bool bypass_rlim);
+ const struct task_struct *task, bool bypass_rlim);
struct kvec;
struct page *get_dump_page(unsigned long addr, int *locked);
@@ -3348,7 +3348,7 @@ void anon_vma_interval_tree_verify(struct anon_vma_chain *node);
avc; avc = anon_vma_interval_tree_iter_next(avc, start, last))
/* mmap.c */
-extern int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin);
+extern int __vm_enough_memory(const struct mm_struct *mm, long pages, int cap_sys_admin);
extern int insert_vm_struct(struct mm_struct *, struct vm_area_struct *);
extern void exit_mmap(struct mm_struct *);
bool mmap_read_lock_maybe_expand(struct mm_struct *mm, struct vm_area_struct *vma,
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 1d35f9e1416e..968b58a97236 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -551,7 +551,7 @@ static inline void filemap_nr_thps_dec(struct address_space *mapping)
#endif
}
-struct address_space *folio_mapping(struct folio *);
+struct address_space *folio_mapping(const struct folio *folio);
/**
* folio_flush_mapping - Find the file mapping this folio belongs to.
diff --git a/mm/util.c b/mm/util.c
index d235b74f7aff..f5a35efba7bf 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -315,7 +315,7 @@ void *memdup_user_nul(const void __user *src, size_t len)
EXPORT_SYMBOL(memdup_user_nul);
/* Check if the vma is being used as a stack by this task */
-int vma_is_stack_for_current(struct vm_area_struct *vma)
+int vma_is_stack_for_current(const struct vm_area_struct *const vma)
{
struct task_struct * __maybe_unused t = current;
@@ -410,7 +410,7 @@ unsigned long arch_mmap_rnd(void)
return rnd << PAGE_SHIFT;
}
-static int mmap_is_legacy(struct rlimit *rlim_stack)
+static int mmap_is_legacy(const struct rlimit *const rlim_stack)
{
if (current->personality & ADDR_COMPAT_LAYOUT)
return 1;
@@ -504,7 +504,7 @@ EXPORT_SYMBOL_IF_KUNIT(arch_pick_mmap_layout);
* * -ENOMEM if RLIMIT_MEMLOCK would be exceeded.
*/
int __account_locked_vm(struct mm_struct *mm, unsigned long pages, bool inc,
- struct task_struct *task, bool bypass_rlim)
+ const struct task_struct *const task, const bool bypass_rlim)
{
unsigned long locked_vm, limit;
int ret = 0;
@@ -688,7 +688,7 @@ struct anon_vma *folio_anon_vma(const struct folio *folio)
* You can call this for folios which aren't in the swap cache or page
* cache and it will return NULL.
*/
-struct address_space *folio_mapping(struct folio *folio)
+struct address_space *folio_mapping(const struct folio *const folio)
{
struct address_space *mapping;
@@ -926,7 +926,8 @@ EXPORT_SYMBOL_GPL(vm_memory_committed);
* Note this is a helper function intended to be used by LSMs which
* wish to use this logic.
*/
-int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin)
+int __vm_enough_memory(const struct mm_struct *const mm,
+ const long pages, const int cap_sys_admin)
{
long allowed;
unsigned long bytes_failed;
--
2.47.2On 01.09.25 14:30, Max Kellermann wrote:
> We select certain test functions which either invoke each other,
> functions that are already const-ified, or no further functions.
>
> It is therefore relatively trivial to const-ify them, which
> provides a basis for further const-ification further up the call
> stack.
>
> (Even though seemingly unrelated, this also constifies the pointer
> parameter of mmap_is_legacy() in arch/s390/mm/mmap.c because a copy of
> the function exists in mm/util.c.)
>
> Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
> Reviewed-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
Also here, some getters hiding.
> ---
> arch/s390/mm/mmap.c | 2 +-
> include/linux/mm.h | 6 +++---
> include/linux/pagemap.h | 2 +-
> mm/util.c | 11 ++++++-----
> 4 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/arch/s390/mm/mmap.c b/arch/s390/mm/mmap.c
> index 547104ccc22a..c0f619fb9ab3 100644
> --- a/arch/s390/mm/mmap.c
> +++ b/arch/s390/mm/mmap.c
> @@ -27,7 +27,7 @@ static unsigned long stack_maxrandom_size(void)
> return STACK_RND_MASK << PAGE_SHIFT;
> }
>
> -static inline int mmap_is_legacy(struct rlimit *rlim_stack)
> +static inline int mmap_is_legacy(const struct rlimit *const rlim_stack)
> {
> if (current->personality & ADDR_COMPAT_LAYOUT)
> return 1;
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index f70c6b4d5f80..23864c3519d6 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -986,7 +986,7 @@ static inline bool vma_is_shmem(const struct vm_area_struct *vma) { return false
> static inline bool vma_is_anon_shmem(const struct vm_area_struct *vma) { return false; }
> #endif
>
> -int vma_is_stack_for_current(struct vm_area_struct *vma);
> +int vma_is_stack_for_current(const struct vm_area_struct *vma);
Should this also be *const ?
>
> /* flush_tlb_range() takes a vma, not a mm, and can care about flags */
> #define TLB_FLUSH_VMA(mm,flags) { .vm_mm = (mm), .vm_flags = (flags) }
> @@ -2585,7 +2585,7 @@ void folio_add_pin(struct folio *folio);
>
> int account_locked_vm(struct mm_struct *mm, unsigned long pages, bool inc);
> int __account_locked_vm(struct mm_struct *mm, unsigned long pages, bool inc,
> - struct task_struct *task, bool bypass_rlim);
> + const struct task_struct *task, bool bypass_rlim);
>
Dito.
> struct kvec;
> struct page *get_dump_page(unsigned long addr, int *locked);
> @@ -3348,7 +3348,7 @@ void anon_vma_interval_tree_verify(struct anon_vma_chain *node);
> avc; avc = anon_vma_interval_tree_iter_next(avc, start, last))
>
> /* mmap.c */
> -extern int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin);
> +extern int __vm_enough_memory(const struct mm_struct *mm, long pages, int cap_sys_admin);
> extern int insert_vm_struct(struct mm_struct *, struct vm_area_struct *);
> extern void exit_mmap(struct mm_struct *);
> bool mmap_read_lock_maybe_expand(struct mm_struct *mm, struct vm_area_struct *vma,
> diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> index 1d35f9e1416e..968b58a97236 100644
> --- a/include/linux/pagemap.h
> +++ b/include/linux/pagemap.h
> @@ -551,7 +551,7 @@ static inline void filemap_nr_thps_dec(struct address_space *mapping)
> #endif
> }
>
> -struct address_space *folio_mapping(struct folio *);
> +struct address_space *folio_mapping(const struct folio *folio);
And this one?
--
Cheers
David / dhildenb
On Mon, Sep 1, 2025 at 3:54 PM David Hildenbrand <david@redhat.com> wrote: > > -int vma_is_stack_for_current(struct vm_area_struct *vma); > > +int vma_is_stack_for_current(const struct vm_area_struct *vma); > > Should this also be *const ? No. These are function protoypes. A "const" on a parameter value (pointer address, not pointed-to memory) makes no sense on a prototype.
On 01.09.25 17:02, Max Kellermann wrote: > On Mon, Sep 1, 2025 at 3:54 PM David Hildenbrand <david@redhat.com> wrote: >>> -int vma_is_stack_for_current(struct vm_area_struct *vma); >>> +int vma_is_stack_for_current(const struct vm_area_struct *vma); >> >> Should this also be *const ? > > No. These are function protoypes. A "const" on a parameter value > (pointer address, not pointed-to memory) makes no sense on a > prototype. But couldn't you argue the same about variable names? In most (not all :) ) we keep declaration + definition in sync. So thus my confusion. -- Cheers David / dhildenb
On Mon, Sep 1, 2025 at 5:11 PM David Hildenbrand <david@redhat.com> wrote: > >> Should this also be *const ? > > > > No. These are function protoypes. A "const" on a parameter value > > (pointer address, not pointed-to memory) makes no sense on a > > prototype. > > But couldn't you argue the same about variable names? In most (not all > :) ) we keep declaration + definition in sync. So thus my confusion. Variable names in the prototypes have no effect either, but they serve as useful documentation. Whereas the "const" on a parameter value documents nothing - it's an implementation detail whether the function would like to modify parameter values. That implementation detail has no effect for the caller. Of course, we could have "const" in the prototype as well. This boils down to personal taste. It's not my taste (has no use, has no effect, documents nothing, only adds noise for no gain), so I didn't add it. If you prefer to have that, I'll leave my taste and home and add it, but only after you guys make up your minds about whether you want to have const parameters at all.
On 01.09.25 17:22, Max Kellermann wrote: > On Mon, Sep 1, 2025 at 5:11 PM David Hildenbrand <david@redhat.com> wrote: >>>> Should this also be *const ? >>> >>> No. These are function protoypes. A "const" on a parameter value >>> (pointer address, not pointed-to memory) makes no sense on a >>> prototype. >> >> But couldn't you argue the same about variable names? In most (not all >> :) ) we keep declaration + definition in sync. So thus my confusion. > > Variable names in the prototypes have no effect either, but they serve > as useful documentation. > > Whereas the "const" on a parameter value documents nothing - it's an > implementation detail whether the function would like to modify > parameter values. That implementation detail has no effect for the > caller. > > Of course, we could have "const" in the prototype as well. This boils > down to personal taste. It's not my taste (has no use, has no effect, > documents nothing, only adds noise for no gain), so I didn't add it. > If you prefer to have that, I'll leave my taste and home and add it, > but only after you guys make up your minds about whether you want to > have const parameters at all. Valid points. The problem is that it could very soon become inconsistent. For example, when I write a new function I usually just copy what I have from the definition into the declaration. For example, checkpatch complains about missing variable names and I think it complains when "extern" is used for functions. If we were to decide to go that route (not keep them in perfect sync), I guess it would be reasonable to extend checkpatch to warn if "*const" is used in a declaration. (perl magic, no idea how hard that would be) I'm sure there are false positives in the following: $ git grep "\*const" *.h | grep -v inline | wc -l 403 -- Cheers David / dhildenb
Not going to add too much noise as same points as David here :)
I'm still definitely leaning towards us not doing the double-const unless
thre's a good reason to or perhaps for larger functions.
But I'm open to being convinced otherwise :)
On Mon, Sep 01, 2025 at 03:54:25PM +0200, David Hildenbrand wrote:
> On 01.09.25 14:30, Max Kellermann wrote:
> > We select certain test functions which either invoke each other,
> > functions that are already const-ified, or no further functions.
> >
> > It is therefore relatively trivial to const-ify them, which
> > provides a basis for further const-ification further up the call
> > stack.
> >
> > (Even though seemingly unrelated, this also constifies the pointer
> > parameter of mmap_is_legacy() in arch/s390/mm/mmap.c because a copy of
> > the function exists in mm/util.c.)
> >
> > Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
> > Reviewed-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
>
> Also here, some getters hiding.
>
> > ---
> > arch/s390/mm/mmap.c | 2 +-
> > include/linux/mm.h | 6 +++---
> > include/linux/pagemap.h | 2 +-
> > mm/util.c | 11 ++++++-----
> > 4 files changed, 11 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/s390/mm/mmap.c b/arch/s390/mm/mmap.c
> > index 547104ccc22a..c0f619fb9ab3 100644
> > --- a/arch/s390/mm/mmap.c
> > +++ b/arch/s390/mm/mmap.c
> > @@ -27,7 +27,7 @@ static unsigned long stack_maxrandom_size(void)
> > return STACK_RND_MASK << PAGE_SHIFT;
> > }
> > -static inline int mmap_is_legacy(struct rlimit *rlim_stack)
> > +static inline int mmap_is_legacy(const struct rlimit *const rlim_stack)
> > {
> > if (current->personality & ADDR_COMPAT_LAYOUT)
> > return 1;
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index f70c6b4d5f80..23864c3519d6 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -986,7 +986,7 @@ static inline bool vma_is_shmem(const struct vm_area_struct *vma) { return false
> > static inline bool vma_is_anon_shmem(const struct vm_area_struct *vma) { return false; }
> > #endif
> > -int vma_is_stack_for_current(struct vm_area_struct *vma);
> > +int vma_is_stack_for_current(const struct vm_area_struct *vma);
>
> Should this also be *const ?
>
> > /* flush_tlb_range() takes a vma, not a mm, and can care about flags */
> > #define TLB_FLUSH_VMA(mm,flags) { .vm_mm = (mm), .vm_flags = (flags) }
> > @@ -2585,7 +2585,7 @@ void folio_add_pin(struct folio *folio);
> > int account_locked_vm(struct mm_struct *mm, unsigned long pages, bool inc);
> > int __account_locked_vm(struct mm_struct *mm, unsigned long pages, bool inc,
> > - struct task_struct *task, bool bypass_rlim);
> > + const struct task_struct *task, bool bypass_rlim);
>
>
> Dito.
>
> > struct kvec;
> > struct page *get_dump_page(unsigned long addr, int *locked);
> > @@ -3348,7 +3348,7 @@ void anon_vma_interval_tree_verify(struct anon_vma_chain *node);
> > avc; avc = anon_vma_interval_tree_iter_next(avc, start, last))
> > /* mmap.c */
> > -extern int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin);
> > +extern int __vm_enough_memory(const struct mm_struct *mm, long pages, int cap_sys_admin);
> > extern int insert_vm_struct(struct mm_struct *, struct vm_area_struct *);
> > extern void exit_mmap(struct mm_struct *);
> > bool mmap_read_lock_maybe_expand(struct mm_struct *mm, struct vm_area_struct *vma,
> > diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> > index 1d35f9e1416e..968b58a97236 100644
> > --- a/include/linux/pagemap.h
> > +++ b/include/linux/pagemap.h
> > @@ -551,7 +551,7 @@ static inline void filemap_nr_thps_dec(struct address_space *mapping)
> > #endif
> > }
> > -struct address_space *folio_mapping(struct folio *);
> > +struct address_space *folio_mapping(const struct folio *folio);
>
> And this one?
>
> --
> Cheers
>
> David / dhildenb
>
© 2016 - 2025 Red Hat, Inc.