linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm/kasan: remove volatile keyword
@ 2021-02-22  5:22 Zhiyuan Dai
  2021-02-22  7:40 ` Dmitry Vyukov
  0 siblings, 1 reply; 2+ messages in thread
From: Zhiyuan Dai @ 2021-02-22  5:22 UTC (permalink / raw)
  To: ryabinin.a.a, glider, andreyknvl, dvyukov, akpm
  Cc: kasan-dev, linux-mm, linux-kernel, Zhiyuan Dai

Like volatile, the kernel primitives which make concurrent
access to data safe (spinlocks, mutexes, memory barriers,
etc.) are designed to prevent unwanted optimization.

If they are being used properly, there will be no need to
use volatile as well.  If volatile is still necessary,
there is almost certainly a bug in the code somewhere.
In properly-written kernel code, volatile can only serve
to slow things down.

see: Documentation/process/volatile-considered-harmful.rst

Signed-off-by: Zhiyuan Dai <daizhiyuan@phytium.com.cn>
---
 mm/kasan/shadow.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/kasan/shadow.c b/mm/kasan/shadow.c
index 7c2c08c..d5ff9ca 100644
--- a/mm/kasan/shadow.c
+++ b/mm/kasan/shadow.c
@@ -25,13 +25,13 @@
 
 #include "kasan.h"
 
-bool __kasan_check_read(const volatile void *p, unsigned int size)
+bool __kasan_check_read(const void *p, unsigned int size)
 {
 	return check_memory_region((unsigned long)p, size, false, _RET_IP_);
 }
 EXPORT_SYMBOL(__kasan_check_read);
 
-bool __kasan_check_write(const volatile void *p, unsigned int size)
+bool __kasan_check_write(const void *p, unsigned int size)
 {
 	return check_memory_region((unsigned long)p, size, true, _RET_IP_);
 }
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] mm/kasan: remove volatile keyword
  2021-02-22  5:22 [PATCH] mm/kasan: remove volatile keyword Zhiyuan Dai
@ 2021-02-22  7:40 ` Dmitry Vyukov
  0 siblings, 0 replies; 2+ messages in thread
From: Dmitry Vyukov @ 2021-02-22  7:40 UTC (permalink / raw)
  To: Zhiyuan Dai
  Cc: Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov,
	Andrew Morton, kasan-dev, Linux-MM, LKML

On Mon, Feb 22, 2021 at 6:22 AM Zhiyuan Dai <daizhiyuan@phytium.com.cn> wrote:
>
> Like volatile, the kernel primitives which make concurrent
> access to data safe (spinlocks, mutexes, memory barriers,
> etc.) are designed to prevent unwanted optimization.
>
> If they are being used properly, there will be no need to
> use volatile as well.  If volatile is still necessary,
> there is almost certainly a bug in the code somewhere.
> In properly-written kernel code, volatile can only serve
> to slow things down.
>
> see: Documentation/process/volatile-considered-harmful.rst

Nack.

This function does not require volatile variables. It uses volatile in
the same way as C/C++ atomic functions -- it only supports operating
on volatile variables. The same meaning as for const here. Such
functions need to use all possible type modifiers to support all
possible uses.

Anyway, the function is declared in kasan.h. So you would need to
change the signate there in the first place. But the kernel will
either not compile, or it won't compile in future when somebody adds
__kasan_check_read/write for a volatile variable. Such a change first
requires removing all volatile uses from the entire kernel and banning
volatile.


> Signed-off-by: Zhiyuan Dai <daizhiyuan@phytium.com.cn>
> ---
>  mm/kasan/shadow.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/kasan/shadow.c b/mm/kasan/shadow.c
> index 7c2c08c..d5ff9ca 100644
> --- a/mm/kasan/shadow.c
> +++ b/mm/kasan/shadow.c
> @@ -25,13 +25,13 @@
>
>  #include "kasan.h"
>
> -bool __kasan_check_read(const volatile void *p, unsigned int size)
> +bool __kasan_check_read(const void *p, unsigned int size)
>  {
>         return check_memory_region((unsigned long)p, size, false, _RET_IP_);
>  }
>  EXPORT_SYMBOL(__kasan_check_read);
>
> -bool __kasan_check_write(const volatile void *p, unsigned int size)
> +bool __kasan_check_write(const void *p, unsigned int size)
>  {
>         return check_memory_region((unsigned long)p, size, true, _RET_IP_);
>  }
> --
> 1.8.3.1
>


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-02-22  7:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-22  5:22 [PATCH] mm/kasan: remove volatile keyword Zhiyuan Dai
2021-02-22  7:40 ` Dmitry Vyukov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).