Re: [PATCH v5 12/12] mm: constify highmem related functions for improved const-correctness

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

On Mon, Sep 01, 2025 at 02:30:28PM +0200, Max Kellermann wrote:
> Lots of functions in mm/highmem.c do not write to the given pointers
> and do not call functions that take non-const pointers and can
> therefore be constified.
>
> This includes functions like kunmap() which might be implemented in a
> way that writes to the pointer (e.g. to update reference counters or
> mapping fields), but currently are not.
>
> kmap() on the other hand cannot be made const because it calls
> set_page_address() which is non-const in some
> architectures/configurations.
>
> Signed-off-by: Max Kellermann <max.kellermann@ionos.com>

Some inconsitencies on const vs. double-const here.

We need to figure this out :)

Since const <type> *val will pretty well work in all circumstances, I again
think we ought to just do that instead of double-const.

Thanks

> ---
>  arch/arm/include/asm/highmem.h    |  6 ++---
>  arch/xtensa/include/asm/highmem.h |  2 +-
>  include/linux/highmem-internal.h  | 44 +++++++++++++++++--------------
>  include/linux/highmem.h           |  8 +++---
>  mm/highmem.c                      | 10 +++----
>  5 files changed, 37 insertions(+), 33 deletions(-)
>
> diff --git a/arch/arm/include/asm/highmem.h b/arch/arm/include/asm/highmem.h
> index b4b66220952d..023be74298f3 100644
> --- a/arch/arm/include/asm/highmem.h
> +++ b/arch/arm/include/asm/highmem.h
> @@ -46,9 +46,9 @@ extern pte_t *pkmap_page_table;
>  #endif
>
>  #ifdef ARCH_NEEDS_KMAP_HIGH_GET
> -extern void *kmap_high_get(struct page *page);
> +extern void *kmap_high_get(const struct page *page);
>
> -static inline void *arch_kmap_local_high_get(struct page *page)
> +static inline void *arch_kmap_local_high_get(const struct page *page)
>  {
>  	if (IS_ENABLED(CONFIG_DEBUG_HIGHMEM) && !cache_is_vivt())
>  		return NULL;
> @@ -57,7 +57,7 @@ static inline void *arch_kmap_local_high_get(struct page *page)
>  #define arch_kmap_local_high_get arch_kmap_local_high_get
>
>  #else /* ARCH_NEEDS_KMAP_HIGH_GET */
> -static inline void *kmap_high_get(struct page *page)
> +static inline void *kmap_high_get(const struct page *const page)
>  {
>  	return NULL;
>  }
> diff --git a/arch/xtensa/include/asm/highmem.h b/arch/xtensa/include/asm/highmem.h
> index 34b8b620e7f1..473b622b863b 100644
> --- a/arch/xtensa/include/asm/highmem.h
> +++ b/arch/xtensa/include/asm/highmem.h
> @@ -29,7 +29,7 @@
>
>  #if DCACHE_WAY_SIZE > PAGE_SIZE
>  #define get_pkmap_color get_pkmap_color
> -static inline int get_pkmap_color(struct page *page)
> +static inline int get_pkmap_color(const struct page *const page)
>  {
>  	return DCACHE_ALIAS(page_to_phys(page));
>  }
> diff --git a/include/linux/highmem-internal.h b/include/linux/highmem-internal.h
> index 36053c3d6d64..442d0efea5c7 100644
> --- a/include/linux/highmem-internal.h
> +++ b/include/linux/highmem-internal.h
> @@ -7,7 +7,7 @@
>   */
>  #ifdef CONFIG_KMAP_LOCAL
>  void *__kmap_local_pfn_prot(unsigned long pfn, pgprot_t prot);
> -void *__kmap_local_page_prot(struct page *page, pgprot_t prot);
> +void *__kmap_local_page_prot(const struct page *page, pgprot_t prot);
>  void kunmap_local_indexed(const void *vaddr);
>  void kmap_local_fork(struct task_struct *tsk);
>  void __kmap_local_sched_out(void);
> @@ -33,11 +33,11 @@ static inline void kmap_flush_tlb(unsigned long addr) { }
>  #endif
>
>  void *kmap_high(struct page *page);
> -void kunmap_high(struct page *page);
> +void kunmap_high(const struct page *page);
>  void __kmap_flush_unused(void);
>  struct page *__kmap_to_page(void *addr);
>
> -static inline void *kmap(struct page *page)
> +static inline void *kmap(struct page *const page)
>  {
>  	void *addr;
>
> @@ -50,7 +50,7 @@ static inline void *kmap(struct page *page)
>  	return addr;
>  }
>
> -static inline void kunmap(struct page *page)
> +static inline void kunmap(const struct page *const page)
>  {
>  	might_sleep();
>  	if (!PageHighMem(page))
> @@ -68,12 +68,12 @@ static inline void kmap_flush_unused(void)
>  	__kmap_flush_unused();
>  }
>
> -static inline void *kmap_local_page(struct page *page)
> +static inline void *kmap_local_page(const struct page *const page)
>  {
>  	return __kmap_local_page_prot(page, kmap_prot);
>  }
>
> -static inline void *kmap_local_page_try_from_panic(struct page *page)
> +static inline void *kmap_local_page_try_from_panic(const struct page *const page)
>  {
>  	if (!PageHighMem(page))
>  		return page_address(page);
> @@ -81,13 +81,15 @@ static inline void *kmap_local_page_try_from_panic(struct page *page)
>  	return NULL;
>  }
>
> -static inline void *kmap_local_folio(struct folio *folio, size_t offset)
> +static inline void *kmap_local_folio(const struct folio *const folio,
> +				     const size_t offset)
>  {
> -	struct page *page = folio_page(folio, offset / PAGE_SIZE);
> +	const struct page *page = folio_page(folio, offset / PAGE_SIZE);
>  	return __kmap_local_page_prot(page, kmap_prot) + offset % PAGE_SIZE;
>  }
>
> -static inline void *kmap_local_page_prot(struct page *page, pgprot_t prot)
> +static inline void *kmap_local_page_prot(const struct page *const page,
> +					 const pgprot_t prot)
>  {
>  	return __kmap_local_page_prot(page, prot);
>  }
> @@ -102,7 +104,7 @@ static inline void __kunmap_local(const void *vaddr)
>  	kunmap_local_indexed(vaddr);
>  }
>
> -static inline void *kmap_atomic_prot(struct page *page, pgprot_t prot)
> +static inline void *kmap_atomic_prot(const struct page *const page, const pgprot_t prot)
>  {
>  	if (IS_ENABLED(CONFIG_PREEMPT_RT))
>  		migrate_disable();
> @@ -113,7 +115,7 @@ static inline void *kmap_atomic_prot(struct page *page, pgprot_t prot)
>  	return __kmap_local_page_prot(page, prot);
>  }
>
> -static inline void *kmap_atomic(struct page *page)
> +static inline void *kmap_atomic(const struct page *const page)
>  {
>  	return kmap_atomic_prot(page, kmap_prot);
>  }
> @@ -167,38 +169,40 @@ static inline struct page *kmap_to_page(void *addr)
>  	return virt_to_page(addr);
>  }
>
> -static inline void *kmap(struct page *page)
> +static inline void *kmap(struct page *const page)
>  {
>  	might_sleep();
>  	return page_address(page);
>  }
>
> -static inline void kunmap_high(struct page *page) { }
> +static inline void kunmap_high(const struct page *const page) { }
>  static inline void kmap_flush_unused(void) { }
>
> -static inline void kunmap(struct page *page)
> +static inline void kunmap(const struct page *const page)
>  {
>  #ifdef ARCH_HAS_FLUSH_ON_KUNMAP
>  	kunmap_flush_on_unmap(page_address(page));
>  #endif
>  }
>
> -static inline void *kmap_local_page(struct page *page)
> +static inline void *kmap_local_page(const struct page *const page)
>  {
>  	return page_address(page);
>  }
>
> -static inline void *kmap_local_page_try_from_panic(struct page *page)
> +static inline void *kmap_local_page_try_from_panic(const struct page *const page)
>  {
>  	return page_address(page);
>  }
>
> -static inline void *kmap_local_folio(struct folio *folio, size_t offset)
> +static inline void *kmap_local_folio(const struct folio *const folio,
> +				     const size_t offset)
>  {
>  	return folio_address(folio) + offset;
>  }
>
> -static inline void *kmap_local_page_prot(struct page *page, pgprot_t prot)
> +static inline void *kmap_local_page_prot(const struct page *const page,
> +					 const pgprot_t prot)
>  {
>  	return kmap_local_page(page);
>  }
> @@ -215,7 +219,7 @@ static inline void __kunmap_local(const void *addr)
>  #endif
>  }
>
> -static inline void *kmap_atomic(struct page *page)
> +static inline void *kmap_atomic(const struct page *const page)
>  {
>  	if (IS_ENABLED(CONFIG_PREEMPT_RT))
>  		migrate_disable();
> @@ -225,7 +229,7 @@ static inline void *kmap_atomic(struct page *page)
>  	return page_address(page);
>  }
>
> -static inline void *kmap_atomic_prot(struct page *page, pgprot_t prot)
> +static inline void *kmap_atomic_prot(const struct page *const page, const pgprot_t prot)
>  {
>  	return kmap_atomic(page);
>  }
> diff --git a/include/linux/highmem.h b/include/linux/highmem.h
> index 6234f316468c..105cc4c00cc3 100644
> --- a/include/linux/highmem.h
> +++ b/include/linux/highmem.h
> @@ -43,7 +43,7 @@ static inline void *kmap(struct page *page);
>   * Counterpart to kmap(). A NOOP for CONFIG_HIGHMEM=n and for mappings of
>   * pages in the low memory area.
>   */
> -static inline void kunmap(struct page *page);
> +static inline void kunmap(const struct page *page);
>
>  /**
>   * kmap_to_page - Get the page for a kmap'ed address
> @@ -93,7 +93,7 @@ static inline void kmap_flush_unused(void);
>   * disabling migration in order to keep the virtual address stable across
>   * preemption. No caller of kmap_local_page() can rely on this side effect.
>   */
> -static inline void *kmap_local_page(struct page *page);
> +static inline void *kmap_local_page(const struct page *page);
>
>  /**
>   * kmap_local_folio - Map a page in this folio for temporary usage
> @@ -129,7 +129,7 @@ static inline void *kmap_local_page(struct page *page);
>   * Context: Can be invoked from any context.
>   * Return: The virtual address of @offset.
>   */
> -static inline void *kmap_local_folio(struct folio *folio, size_t offset);
> +static inline void *kmap_local_folio(const struct folio *folio, size_t offset);
>
>  /**
>   * kmap_atomic - Atomically map a page for temporary usage - Deprecated!
> @@ -176,7 +176,7 @@ static inline void *kmap_local_folio(struct folio *folio, size_t offset);
>   * kunmap_atomic(vaddr2);
>   * kunmap_atomic(vaddr1);
>   */
> -static inline void *kmap_atomic(struct page *page);
> +static inline void *kmap_atomic(const struct page *page);
>
>  /* Highmem related interfaces for management code */
>  static inline unsigned long nr_free_highpages(void);
> diff --git a/mm/highmem.c b/mm/highmem.c
> index ef3189b36cad..93fa505fcb98 100644
> --- a/mm/highmem.c
> +++ b/mm/highmem.c
> @@ -61,7 +61,7 @@ static inline int kmap_local_calc_idx(int idx)
>  /*
>   * Determine color of virtual address where the page should be mapped.
>   */
> -static inline unsigned int get_pkmap_color(struct page *page)
> +static inline unsigned int get_pkmap_color(const struct page *const page)
>  {
>  	return 0;
>  }
> @@ -334,7 +334,7 @@ EXPORT_SYMBOL(kmap_high);
>   *
>   * This can be called from any context.
>   */
> -void *kmap_high_get(struct page *page)
> +void *kmap_high_get(const struct page *const page)
>  {
>  	unsigned long vaddr, flags;
>
> @@ -356,7 +356,7 @@ void *kmap_high_get(struct page *page)
>   * If ARCH_NEEDS_KMAP_HIGH_GET is not defined then this may be called
>   * only from user context.
>   */
> -void kunmap_high(struct page *page)
> +void kunmap_high(const struct page *const page)
>  {
>  	unsigned long vaddr;
>  	unsigned long nr;
> @@ -508,7 +508,7 @@ static inline void kmap_local_idx_pop(void)
>  #endif
>
>  #ifndef arch_kmap_local_high_get
> -static inline void *arch_kmap_local_high_get(struct page *page)
> +static inline void *arch_kmap_local_high_get(const struct page *const page)
>  {
>  	return NULL;
>  }
> @@ -572,7 +572,7 @@ void *__kmap_local_pfn_prot(unsigned long pfn, pgprot_t prot)
>  }
>  EXPORT_SYMBOL_GPL(__kmap_local_pfn_prot);
>
> -void *__kmap_local_page_prot(struct page *page, pgprot_t prot)
> +void *__kmap_local_page_prot(const struct page *const page, const pgprot_t prot)
>  {
>  	void *kmap;
>
> --
> 2.47.2
>