All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marco Elver <elver@google.com>
To: Kefeng Wang <wangkefeng.wang@huawei.com>
Cc: catalin.marinas@arm.com, will@kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, mark.rutland@arm.com
Subject: Re: [PATCH] arm64: kcsan: Fix kcsan test_barrier fail and panic
Date: Tue, 26 Apr 2022 14:10:06 +0200	[thread overview]
Message-ID: <YmfhHuPDilwR/Wgp@elver.google.com> (raw)
In-Reply-To: <20220426081700.1376542-1-wangkefeng.wang@huawei.com>

On Tue, Apr 26, 2022 at 08:17AM +0000, Kefeng Wang wrote:
> As "kcsan: Support detecting a subset of missing memory barriers"
> introduced KCSAN_STRICT which make kcsan detects more missing memory
> barrier, but arm64 don't have KCSAN instrumentation for barriers, so
> the new selftest test_barrier() will fail, then panic.

Thanks for fixing this - did kcsan_test module pass as well?

> Let's prefix all barriers with __ on arm64, as asm-generic/barriers.h
> defined the final instrumented version of these barriers, which will
> fix the above issues.
> 
> Fixes: dd03762ab608 ("arm64: Enable KCSAN")
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  arch/arm64/include/asm/barrier.h | 12 ++++++------
>  include/asm-generic/barrier.h    |  4 ++++
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
> index 62217be36217..9760a8d4ed0a 100644
> --- a/arch/arm64/include/asm/barrier.h
> +++ b/arch/arm64/include/asm/barrier.h
> @@ -46,13 +46,13 @@
>  #define pmr_sync()	do {} while (0)
>  #endif
>  
> -#define mb()		dsb(sy)
> -#define rmb()		dsb(ld)
> -#define wmb()		dsb(st)
> +#define __mb()		dsb(sy)
> +#define __rmb()		dsb(ld)
> +#define __wmb()		dsb(st)
>  
> -#define dma_mb()	dmb(osh)
> -#define dma_rmb()	dmb(oshld)
> -#define dma_wmb()	dmb(oshst)
> +#define __dma_mb()	dmb(osh)
> +#define __dma_rmb()	dmb(oshld)
> +#define __dma_wmb()	dmb(oshst)
>  
>  #define io_stop_wc()	dgh()
>  
> diff --git a/include/asm-generic/barrier.h b/include/asm-generic/barrier.h
> index fd7e8fbaeef1..18863c50e9ce 100644
> --- a/include/asm-generic/barrier.h
> +++ b/include/asm-generic/barrier.h
> @@ -38,6 +38,10 @@
>  #define wmb()	do { kcsan_wmb(); __wmb(); } while (0)
>  #endif
>  
> +#ifdef __dma_mb
> +#define dma_mb()	do { kcsan_mb(); __dma_mb(); } while (0)
> +#endif
> +

So it looks like arm64 is the only arch that defines dma_mb(). By adding
it to asm-generic, we'd almost be encouraging other architectures to add
it, which I don't know we want.

Documentation/memory-barriers.txt doesn't mention dma_mb() either - so
perhaps dma_mb() doesn't belong in asm-generic/barrier.h, and you could
only change arm64's definition of dma_mb() to add the kcsan_mb().

Preferences? Maybe arch64 maintainers have more background on why arm64
is an anomaly here.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Marco Elver <elver@google.com>
To: Kefeng Wang <wangkefeng.wang@huawei.com>
Cc: catalin.marinas@arm.com, will@kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, mark.rutland@arm.com
Subject: Re: [PATCH] arm64: kcsan: Fix kcsan test_barrier fail and panic
Date: Tue, 26 Apr 2022 14:10:06 +0200	[thread overview]
Message-ID: <YmfhHuPDilwR/Wgp@elver.google.com> (raw)
In-Reply-To: <20220426081700.1376542-1-wangkefeng.wang@huawei.com>

On Tue, Apr 26, 2022 at 08:17AM +0000, Kefeng Wang wrote:
> As "kcsan: Support detecting a subset of missing memory barriers"
> introduced KCSAN_STRICT which make kcsan detects more missing memory
> barrier, but arm64 don't have KCSAN instrumentation for barriers, so
> the new selftest test_barrier() will fail, then panic.

Thanks for fixing this - did kcsan_test module pass as well?

> Let's prefix all barriers with __ on arm64, as asm-generic/barriers.h
> defined the final instrumented version of these barriers, which will
> fix the above issues.
> 
> Fixes: dd03762ab608 ("arm64: Enable KCSAN")
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  arch/arm64/include/asm/barrier.h | 12 ++++++------
>  include/asm-generic/barrier.h    |  4 ++++
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
> index 62217be36217..9760a8d4ed0a 100644
> --- a/arch/arm64/include/asm/barrier.h
> +++ b/arch/arm64/include/asm/barrier.h
> @@ -46,13 +46,13 @@
>  #define pmr_sync()	do {} while (0)
>  #endif
>  
> -#define mb()		dsb(sy)
> -#define rmb()		dsb(ld)
> -#define wmb()		dsb(st)
> +#define __mb()		dsb(sy)
> +#define __rmb()		dsb(ld)
> +#define __wmb()		dsb(st)
>  
> -#define dma_mb()	dmb(osh)
> -#define dma_rmb()	dmb(oshld)
> -#define dma_wmb()	dmb(oshst)
> +#define __dma_mb()	dmb(osh)
> +#define __dma_rmb()	dmb(oshld)
> +#define __dma_wmb()	dmb(oshst)
>  
>  #define io_stop_wc()	dgh()
>  
> diff --git a/include/asm-generic/barrier.h b/include/asm-generic/barrier.h
> index fd7e8fbaeef1..18863c50e9ce 100644
> --- a/include/asm-generic/barrier.h
> +++ b/include/asm-generic/barrier.h
> @@ -38,6 +38,10 @@
>  #define wmb()	do { kcsan_wmb(); __wmb(); } while (0)
>  #endif
>  
> +#ifdef __dma_mb
> +#define dma_mb()	do { kcsan_mb(); __dma_mb(); } while (0)
> +#endif
> +

So it looks like arm64 is the only arch that defines dma_mb(). By adding
it to asm-generic, we'd almost be encouraging other architectures to add
it, which I don't know we want.

Documentation/memory-barriers.txt doesn't mention dma_mb() either - so
perhaps dma_mb() doesn't belong in asm-generic/barrier.h, and you could
only change arm64's definition of dma_mb() to add the kcsan_mb().

Preferences? Maybe arch64 maintainers have more background on why arm64
is an anomaly here.

  reply	other threads:[~2022-04-26 12:11 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-26  8:17 [PATCH] arm64: kcsan: Fix kcsan test_barrier fail and panic Kefeng Wang
2022-04-26  8:17 ` Kefeng Wang
2022-04-26 12:10 ` Marco Elver [this message]
2022-04-26 12:10   ` Marco Elver
2022-04-26 12:50   ` Mark Rutland
2022-04-26 12:50     ` Mark Rutland
2022-04-28 10:49     ` Will Deacon
2022-04-28 10:49       ` Will Deacon
2022-04-26 15:13   ` Kefeng Wang
2022-04-26 15:13     ` Kefeng Wang
2022-04-26 12:42 ` Mark Rutland
2022-04-26 12:42   ` Mark Rutland
2022-04-26 15:39   ` Kefeng Wang
2022-04-26 15:39     ` Kefeng Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YmfhHuPDilwR/Wgp@elver.google.com \
    --to=elver@google.com \
    --cc=catalin.marinas@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=wangkefeng.wang@huawei.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.