2022-06-15 05:14:21

by Fabio M. De Francesco

[permalink] [raw]
Subject: [PATCH v2] highmem: Make kunmap_{local,atomic}() take pointers to const void

kunmap_ {local,atomic}() currently take pointers to void. However, this
is semantically incorrect, since these functions do not change the memory
their arguments point to.

Therefore, make this semantics explicit by modifying the
kunmap_{local,atomic}() prototypes to take pointers to const void.

As side effects, compilers will likely produce more efficient code and
they won't any longer need casts to pointers to void where these functions
take arguments of type pointer to const void.

Suggested-by: David Sterba <[email protected]>
Suggested-by: Ira Weiny <[email protected]>
Signed-off-by: Fabio M. De Francesco <[email protected]>
---

v1->v2: Change the commit message to clearly explain why these functions
should require pointers to const void. The fundamental argument behind the
commit message changes is semantic correctness. Other bonuses come as side
effects. Obviously there are no changes to the code.

Many thanks to David Sterba and Ira Weiny for suggestions and reviews.

include/linux/highmem-internal.h | 10 +++++-----
mm/highmem.c | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/highmem-internal.h b/include/linux/highmem-internal.h
index cddb42ff0473..034b1106d022 100644
--- a/include/linux/highmem-internal.h
+++ b/include/linux/highmem-internal.h
@@ -8,7 +8,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 kunmap_local_indexed(void *vaddr);
+void kunmap_local_indexed(const void *vaddr);
void kmap_local_fork(struct task_struct *tsk);
void __kmap_local_sched_out(void);
void __kmap_local_sched_in(void);
@@ -89,7 +89,7 @@ static inline void *kmap_local_pfn(unsigned long pfn)
return __kmap_local_pfn_prot(pfn, kmap_prot);
}

-static inline void __kunmap_local(void *vaddr)
+static inline void __kunmap_local(const void *vaddr)
{
kunmap_local_indexed(vaddr);
}
@@ -121,7 +121,7 @@ static inline void *kmap_atomic_pfn(unsigned long pfn)
return __kmap_local_pfn_prot(pfn, kmap_prot);
}

-static inline void __kunmap_atomic(void *addr)
+static inline void __kunmap_atomic(const void *addr)
{
kunmap_local_indexed(addr);
pagefault_enable();
@@ -197,7 +197,7 @@ static inline void *kmap_local_pfn(unsigned long pfn)
return kmap_local_page(pfn_to_page(pfn));
}

-static inline void __kunmap_local(void *addr)
+static inline void __kunmap_local(const void *addr)
{
#ifdef ARCH_HAS_FLUSH_ON_KUNMAP
kunmap_flush_on_unmap(addr);
@@ -224,7 +224,7 @@ static inline void *kmap_atomic_pfn(unsigned long pfn)
return kmap_atomic(pfn_to_page(pfn));
}

-static inline void __kunmap_atomic(void *addr)
+static inline void __kunmap_atomic(const void *addr)
{
#ifdef ARCH_HAS_FLUSH_ON_KUNMAP
kunmap_flush_on_unmap(addr);
diff --git a/mm/highmem.c b/mm/highmem.c
index 1a692997fac4..e32083e4ce0d 100644
--- a/mm/highmem.c
+++ b/mm/highmem.c
@@ -561,7 +561,7 @@ void *__kmap_local_page_prot(struct page *page, pgprot_t prot)
}
EXPORT_SYMBOL(__kmap_local_page_prot);

-void kunmap_local_indexed(void *vaddr)
+void kunmap_local_indexed(const void *vaddr)
{
unsigned long addr = (unsigned long) vaddr & PAGE_MASK;
pte_t *kmap_pte;
--
2.36.1


2022-06-15 05:51:34

by Fabio M. De Francesco

[permalink] [raw]
Subject: Re: [PATCH v2] highmem: Make kunmap_{local,atomic}() take pointers to const void

On mercoledì 15 giugno 2022 07:12:56 CEST Fabio M. De Francesco wrote:
> kunmap_{local,atomic}() currently take pointers to void. However, this
> is semantically incorrect, since these functions do not change the memory
> their arguments point to.
>
> Therefore, make this semantics explicit by modifying the
> kunmap_{local,atomic}() prototypes to take pointers to const void.
>
> As side effects, compilers will likely produce more efficient code and
> they won't any longer need casts to pointers to void where these
> functions take arguments of type pointer to const void.
>
> Suggested-by: David Sterba <[email protected]>
> Suggested-by: Ira Weiny <[email protected]>
> Signed-off-by: Fabio M. De Francesco <[email protected]>
> ---
>
> v1->v2: Change the commit message to clearly explain why these functions
> should require pointers to const void. The fundamental argument behind
> the commit message changes is semantic correctness. Other bonuses come as
> side effects. Obviously there are no changes to the code.
>
> Many thanks to David Sterba and Ira Weiny for suggestions and reviews.
>
> include/linux/highmem-internal.h | 10 +++++-----
> mm/highmem.c | 2 +-
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
As said in another email, this is the second version of "[PATCH] highmem:
Make __kunmap_{local,atomic}() take "const void *"". You see that I changed
subject (involuntarily, that was just a mistake when formatting).

This is why I'm writing: make Maintainers find easily which patch this is
the second version of.

Sorry for that unwanted change to the subject.

Thanks,

Fabio