From: Mark Rutland <mark.rutland@arm.com>
To: Uros Bizjak <ubizjak@gmail.com>
Cc: linux-alpha@vger.kernel.org, linux-kernel@vger.kernel.org,
loongarch@lists.linux.dev, linux-mips@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, linux-arch@vger.kernel.org,
linux-perf-users@vger.kernel.org, Will Deacon <will@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Boqun Feng <boqun.feng@gmail.com>
Subject: Re: [PATCH 01/10] locking/atomic: Add missing cast to try_cmpxchg() fallbacks
Date: Fri, 24 Mar 2023 14:13:13 +0000 [thread overview]
Message-ID: <ZB2v+avNt52ac/+w@FVFF77S0Q05N> (raw)
In-Reply-To: <20230305205628.27385-2-ubizjak@gmail.com>
On Sun, Mar 05, 2023 at 09:56:19PM +0100, Uros Bizjak wrote:
> Cast _oldp to the type of _ptr to avoid incompatible-pointer-types warning.
Can you give an example of where we are passing an incompatible pointer?
That sounds indicative of a bug in the caller, but maybe I'm missing some
reason this is necessary due to some indirection.
> Fixes: 29f006fdefe6 ("asm-generic/atomic: Add try_cmpxchg() fallbacks")
I'm not sure that this needs a fixes tag. Does anything go wrong today, or only
later in this series?
Thanks,
Mark.
> Cc: Will Deacon <will@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> ---
> include/linux/atomic/atomic-arch-fallback.h | 18 +++++++++---------
> scripts/atomic/gen-atomic-fallback.sh | 2 +-
> 2 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/atomic/atomic-arch-fallback.h b/include/linux/atomic/atomic-arch-fallback.h
> index 77bc5522e61c..19debd501ee7 100644
> --- a/include/linux/atomic/atomic-arch-fallback.h
> +++ b/include/linux/atomic/atomic-arch-fallback.h
> @@ -87,7 +87,7 @@
> #ifndef arch_try_cmpxchg
> #define arch_try_cmpxchg(_ptr, _oldp, _new) \
> ({ \
> - typeof(*(_ptr)) *___op = (_oldp), ___o = *___op, ___r; \
> + typeof(*(_ptr)) *___op = (typeof(_ptr))(_oldp), ___o = *___op, ___r; \
> ___r = arch_cmpxchg((_ptr), ___o, (_new)); \
> if (unlikely(___r != ___o)) \
> *___op = ___r; \
> @@ -98,7 +98,7 @@
> #ifndef arch_try_cmpxchg_acquire
> #define arch_try_cmpxchg_acquire(_ptr, _oldp, _new) \
> ({ \
> - typeof(*(_ptr)) *___op = (_oldp), ___o = *___op, ___r; \
> + typeof(*(_ptr)) *___op = (typeof(_ptr))(_oldp), ___o = *___op, ___r; \
> ___r = arch_cmpxchg_acquire((_ptr), ___o, (_new)); \
> if (unlikely(___r != ___o)) \
> *___op = ___r; \
> @@ -109,7 +109,7 @@
> #ifndef arch_try_cmpxchg_release
> #define arch_try_cmpxchg_release(_ptr, _oldp, _new) \
> ({ \
> - typeof(*(_ptr)) *___op = (_oldp), ___o = *___op, ___r; \
> + typeof(*(_ptr)) *___op = (typeof(_ptr))(_oldp), ___o = *___op, ___r; \
> ___r = arch_cmpxchg_release((_ptr), ___o, (_new)); \
> if (unlikely(___r != ___o)) \
> *___op = ___r; \
> @@ -120,7 +120,7 @@
> #ifndef arch_try_cmpxchg_relaxed
> #define arch_try_cmpxchg_relaxed(_ptr, _oldp, _new) \
> ({ \
> - typeof(*(_ptr)) *___op = (_oldp), ___o = *___op, ___r; \
> + typeof(*(_ptr)) *___op = (typeof(_ptr))(_oldp), ___o = *___op, ___r; \
> ___r = arch_cmpxchg_relaxed((_ptr), ___o, (_new)); \
> if (unlikely(___r != ___o)) \
> *___op = ___r; \
> @@ -157,7 +157,7 @@
> #ifndef arch_try_cmpxchg64
> #define arch_try_cmpxchg64(_ptr, _oldp, _new) \
> ({ \
> - typeof(*(_ptr)) *___op = (_oldp), ___o = *___op, ___r; \
> + typeof(*(_ptr)) *___op = (typeof(_ptr))(_oldp), ___o = *___op, ___r; \
> ___r = arch_cmpxchg64((_ptr), ___o, (_new)); \
> if (unlikely(___r != ___o)) \
> *___op = ___r; \
> @@ -168,7 +168,7 @@
> #ifndef arch_try_cmpxchg64_acquire
> #define arch_try_cmpxchg64_acquire(_ptr, _oldp, _new) \
> ({ \
> - typeof(*(_ptr)) *___op = (_oldp), ___o = *___op, ___r; \
> + typeof(*(_ptr)) *___op = (typeof(_ptr))(_oldp), ___o = *___op, ___r; \
> ___r = arch_cmpxchg64_acquire((_ptr), ___o, (_new)); \
> if (unlikely(___r != ___o)) \
> *___op = ___r; \
> @@ -179,7 +179,7 @@
> #ifndef arch_try_cmpxchg64_release
> #define arch_try_cmpxchg64_release(_ptr, _oldp, _new) \
> ({ \
> - typeof(*(_ptr)) *___op = (_oldp), ___o = *___op, ___r; \
> + typeof(*(_ptr)) *___op = (typeof(_ptr))(_oldp), ___o = *___op, ___r; \
> ___r = arch_cmpxchg64_release((_ptr), ___o, (_new)); \
> if (unlikely(___r != ___o)) \
> *___op = ___r; \
> @@ -190,7 +190,7 @@
> #ifndef arch_try_cmpxchg64_relaxed
> #define arch_try_cmpxchg64_relaxed(_ptr, _oldp, _new) \
> ({ \
> - typeof(*(_ptr)) *___op = (_oldp), ___o = *___op, ___r; \
> + typeof(*(_ptr)) *___op = (typeof(_ptr))(_oldp), ___o = *___op, ___r; \
> ___r = arch_cmpxchg64_relaxed((_ptr), ___o, (_new)); \
> if (unlikely(___r != ___o)) \
> *___op = ___r; \
> @@ -2456,4 +2456,4 @@ arch_atomic64_dec_if_positive(atomic64_t *v)
> #endif
>
> #endif /* _LINUX_ATOMIC_FALLBACK_H */
> -// b5e87bdd5ede61470c29f7a7e4de781af3770f09
> +// 1b4d4c82ae653389cd1538d5b07170267d9b3837
> diff --git a/scripts/atomic/gen-atomic-fallback.sh b/scripts/atomic/gen-atomic-fallback.sh
> index 3a07695e3c89..39f447161108 100755
> --- a/scripts/atomic/gen-atomic-fallback.sh
> +++ b/scripts/atomic/gen-atomic-fallback.sh
> @@ -171,7 +171,7 @@ cat <<EOF
> #ifndef arch_try_${cmpxchg}${order}
> #define arch_try_${cmpxchg}${order}(_ptr, _oldp, _new) \\
> ({ \\
> - typeof(*(_ptr)) *___op = (_oldp), ___o = *___op, ___r; \\
> + typeof(*(_ptr)) *___op = (typeof(_ptr))(_oldp), ___o = *___op, ___r; \\
> ___r = arch_${cmpxchg}${order}((_ptr), ___o, (_new)); \\
> if (unlikely(___r != ___o)) \\
> *___op = ___r; \\
> --
> 2.39.2
>
WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com>
To: Uros Bizjak <ubizjak@gmail.com>
Cc: linux-arch@vger.kernel.org, Peter Zijlstra <peterz@infradead.org>,
Will Deacon <will@kernel.org>, Boqun Feng <boqun.feng@gmail.com>,
linux-mips@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-perf-users@vger.kernel.org, loongarch@lists.linux.dev,
linux-alpha@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH 01/10] locking/atomic: Add missing cast to try_cmpxchg() fallbacks
Date: Fri, 24 Mar 2023 14:13:13 +0000 [thread overview]
Message-ID: <ZB2v+avNt52ac/+w@FVFF77S0Q05N> (raw)
In-Reply-To: <20230305205628.27385-2-ubizjak@gmail.com>
On Sun, Mar 05, 2023 at 09:56:19PM +0100, Uros Bizjak wrote:
> Cast _oldp to the type of _ptr to avoid incompatible-pointer-types warning.
Can you give an example of where we are passing an incompatible pointer?
That sounds indicative of a bug in the caller, but maybe I'm missing some
reason this is necessary due to some indirection.
> Fixes: 29f006fdefe6 ("asm-generic/atomic: Add try_cmpxchg() fallbacks")
I'm not sure that this needs a fixes tag. Does anything go wrong today, or only
later in this series?
Thanks,
Mark.
> Cc: Will Deacon <will@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> ---
> include/linux/atomic/atomic-arch-fallback.h | 18 +++++++++---------
> scripts/atomic/gen-atomic-fallback.sh | 2 +-
> 2 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/atomic/atomic-arch-fallback.h b/include/linux/atomic/atomic-arch-fallback.h
> index 77bc5522e61c..19debd501ee7 100644
> --- a/include/linux/atomic/atomic-arch-fallback.h
> +++ b/include/linux/atomic/atomic-arch-fallback.h
> @@ -87,7 +87,7 @@
> #ifndef arch_try_cmpxchg
> #define arch_try_cmpxchg(_ptr, _oldp, _new) \
> ({ \
> - typeof(*(_ptr)) *___op = (_oldp), ___o = *___op, ___r; \
> + typeof(*(_ptr)) *___op = (typeof(_ptr))(_oldp), ___o = *___op, ___r; \
> ___r = arch_cmpxchg((_ptr), ___o, (_new)); \
> if (unlikely(___r != ___o)) \
> *___op = ___r; \
> @@ -98,7 +98,7 @@
> #ifndef arch_try_cmpxchg_acquire
> #define arch_try_cmpxchg_acquire(_ptr, _oldp, _new) \
> ({ \
> - typeof(*(_ptr)) *___op = (_oldp), ___o = *___op, ___r; \
> + typeof(*(_ptr)) *___op = (typeof(_ptr))(_oldp), ___o = *___op, ___r; \
> ___r = arch_cmpxchg_acquire((_ptr), ___o, (_new)); \
> if (unlikely(___r != ___o)) \
> *___op = ___r; \
> @@ -109,7 +109,7 @@
> #ifndef arch_try_cmpxchg_release
> #define arch_try_cmpxchg_release(_ptr, _oldp, _new) \
> ({ \
> - typeof(*(_ptr)) *___op = (_oldp), ___o = *___op, ___r; \
> + typeof(*(_ptr)) *___op = (typeof(_ptr))(_oldp), ___o = *___op, ___r; \
> ___r = arch_cmpxchg_release((_ptr), ___o, (_new)); \
> if (unlikely(___r != ___o)) \
> *___op = ___r; \
> @@ -120,7 +120,7 @@
> #ifndef arch_try_cmpxchg_relaxed
> #define arch_try_cmpxchg_relaxed(_ptr, _oldp, _new) \
> ({ \
> - typeof(*(_ptr)) *___op = (_oldp), ___o = *___op, ___r; \
> + typeof(*(_ptr)) *___op = (typeof(_ptr))(_oldp), ___o = *___op, ___r; \
> ___r = arch_cmpxchg_relaxed((_ptr), ___o, (_new)); \
> if (unlikely(___r != ___o)) \
> *___op = ___r; \
> @@ -157,7 +157,7 @@
> #ifndef arch_try_cmpxchg64
> #define arch_try_cmpxchg64(_ptr, _oldp, _new) \
> ({ \
> - typeof(*(_ptr)) *___op = (_oldp), ___o = *___op, ___r; \
> + typeof(*(_ptr)) *___op = (typeof(_ptr))(_oldp), ___o = *___op, ___r; \
> ___r = arch_cmpxchg64((_ptr), ___o, (_new)); \
> if (unlikely(___r != ___o)) \
> *___op = ___r; \
> @@ -168,7 +168,7 @@
> #ifndef arch_try_cmpxchg64_acquire
> #define arch_try_cmpxchg64_acquire(_ptr, _oldp, _new) \
> ({ \
> - typeof(*(_ptr)) *___op = (_oldp), ___o = *___op, ___r; \
> + typeof(*(_ptr)) *___op = (typeof(_ptr))(_oldp), ___o = *___op, ___r; \
> ___r = arch_cmpxchg64_acquire((_ptr), ___o, (_new)); \
> if (unlikely(___r != ___o)) \
> *___op = ___r; \
> @@ -179,7 +179,7 @@
> #ifndef arch_try_cmpxchg64_release
> #define arch_try_cmpxchg64_release(_ptr, _oldp, _new) \
> ({ \
> - typeof(*(_ptr)) *___op = (_oldp), ___o = *___op, ___r; \
> + typeof(*(_ptr)) *___op = (typeof(_ptr))(_oldp), ___o = *___op, ___r; \
> ___r = arch_cmpxchg64_release((_ptr), ___o, (_new)); \
> if (unlikely(___r != ___o)) \
> *___op = ___r; \
> @@ -190,7 +190,7 @@
> #ifndef arch_try_cmpxchg64_relaxed
> #define arch_try_cmpxchg64_relaxed(_ptr, _oldp, _new) \
> ({ \
> - typeof(*(_ptr)) *___op = (_oldp), ___o = *___op, ___r; \
> + typeof(*(_ptr)) *___op = (typeof(_ptr))(_oldp), ___o = *___op, ___r; \
> ___r = arch_cmpxchg64_relaxed((_ptr), ___o, (_new)); \
> if (unlikely(___r != ___o)) \
> *___op = ___r; \
> @@ -2456,4 +2456,4 @@ arch_atomic64_dec_if_positive(atomic64_t *v)
> #endif
>
> #endif /* _LINUX_ATOMIC_FALLBACK_H */
> -// b5e87bdd5ede61470c29f7a7e4de781af3770f09
> +// 1b4d4c82ae653389cd1538d5b07170267d9b3837
> diff --git a/scripts/atomic/gen-atomic-fallback.sh b/scripts/atomic/gen-atomic-fallback.sh
> index 3a07695e3c89..39f447161108 100755
> --- a/scripts/atomic/gen-atomic-fallback.sh
> +++ b/scripts/atomic/gen-atomic-fallback.sh
> @@ -171,7 +171,7 @@ cat <<EOF
> #ifndef arch_try_${cmpxchg}${order}
> #define arch_try_${cmpxchg}${order}(_ptr, _oldp, _new) \\
> ({ \\
> - typeof(*(_ptr)) *___op = (_oldp), ___o = *___op, ___r; \\
> + typeof(*(_ptr)) *___op = (typeof(_ptr))(_oldp), ___o = *___op, ___r; \\
> ___r = arch_${cmpxchg}${order}((_ptr), ___o, (_new)); \\
> if (unlikely(___r != ___o)) \\
> *___op = ___r; \\
> --
> 2.39.2
>
next prev parent reply other threads:[~2023-03-24 14:13 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-05 20:56 [PATCH 00/10] locking: Introduce local{,64}_try_cmpxchg Uros Bizjak
2023-03-05 20:56 ` Uros Bizjak
2023-03-05 20:56 ` Uros Bizjak
2023-03-05 20:56 ` [PATCH 01/10] locking/atomic: Add missing cast to try_cmpxchg() fallbacks Uros Bizjak
2023-03-05 20:56 ` Uros Bizjak
2023-03-24 14:13 ` Mark Rutland [this message]
2023-03-24 14:13 ` Mark Rutland
2023-03-24 15:43 ` Uros Bizjak
2023-03-24 15:43 ` Uros Bizjak
2023-03-24 15:43 ` Uros Bizjak
2023-03-24 16:14 ` Mark Rutland
2023-03-24 16:14 ` Mark Rutland
2023-03-24 16:14 ` Mark Rutland
2023-03-24 16:32 ` Mark Rutland
2023-03-24 16:32 ` Mark Rutland
2023-03-24 16:32 ` Mark Rutland
2023-03-26 19:28 ` Uros Bizjak
2023-03-26 19:28 ` Uros Bizjak
2023-04-03 10:19 ` Mark Rutland
2023-04-03 10:19 ` Mark Rutland
2023-04-04 12:24 ` Uros Bizjak
2023-04-04 12:24 ` Uros Bizjak
2023-04-04 13:19 ` Mark Rutland
2023-04-04 13:19 ` Mark Rutland
2023-04-04 13:23 ` Uros Bizjak
2023-04-04 13:23 ` Uros Bizjak
2023-03-05 20:56 ` [PATCH 02/10] locking/atomic: Add generic try_cmpxchg{,64}_local support Uros Bizjak
2023-03-05 20:56 ` Uros Bizjak
2023-03-05 20:56 ` [PATCH 03/10] locking/alpha: Wire up local_try_cmpxchg Uros Bizjak
2023-03-05 20:56 ` Uros Bizjak
2023-03-05 20:56 ` [PATCH 04/10] locking/loongarch: " Uros Bizjak
2023-03-05 20:56 ` Uros Bizjak
2023-03-05 20:56 ` [PATCH 05/10] locking/mips: " Uros Bizjak
2023-03-05 20:56 ` Uros Bizjak
2023-03-05 20:56 ` [PATCH 06/10] locking/powerpc: " Uros Bizjak
2023-03-05 20:56 ` Uros Bizjak
2023-03-05 20:56 ` [PATCH 07/10] locking/x86: " Uros Bizjak
2023-03-05 20:56 ` Uros Bizjak
2023-03-05 20:56 ` [PATCH 08/10] locking/generic: Wire up local{,64}_try_cmpxchg Uros Bizjak
2023-03-05 20:56 ` [PATCH 09/10] locking/x86: Enable local{,64}_try_cmpxchg Uros Bizjak
2023-03-05 20:56 ` Uros Bizjak
2023-03-05 20:56 ` [PATCH 10/10] perf/ring_buffer: use local_try_cmpxchg in __perf_output_begin Uros Bizjak
2023-03-05 20:56 ` Uros Bizjak
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=ZB2v+avNt52ac/+w@FVFF77S0Q05N \
--to=mark.rutland@arm.com \
--cc=boqun.feng@gmail.com \
--cc=linux-alpha@vger.kernel.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=loongarch@lists.linux.dev \
--cc=peterz@infradead.org \
--cc=ubizjak@gmail.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.