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 16:32:52 +0000 [thread overview]
Message-ID: <ZB3QtDYuWdpiD5qk@FVFF77S0Q05N> (raw)
In-Reply-To: <ZB3MR8lGbnea9ui6@FVFF77S0Q05N>
On Fri, Mar 24, 2023 at 04:14:22PM +0000, Mark Rutland wrote:
> On Fri, Mar 24, 2023 at 04:43:32PM +0100, Uros Bizjak wrote:
> > On Fri, Mar 24, 2023 at 3:13 PM Mark Rutland <mark.rutland@arm.com> wrote:
> > >
> > > 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?
> >
> > An example is patch 10/10 from the series, which will fail without
> > this fix when fallback code is used. We have:
> >
> > - } while (local_cmpxchg(&rb->head, offset, head) != offset);
> > + } while (!local_try_cmpxchg(&rb->head, &offset, head));
> >
> > where rb->head is defined as:
> >
> > typedef struct {
> > atomic_long_t a;
> > } local_t;
> >
> > while offset is defined as 'unsigned long'.
>
> Ok, but that's because we're doing the wrong thing to start with.
>
> Since local_t is defined in terms of atomic_long_t, we should define the
> generic local_try_cmpxchg() in terms of atomic_long_try_cmpxchg(). We'll still
> have a mismatch between 'long *' and 'unsigned long *', but then we can fix
> that in the callsite:
>
> while (!local_try_cmpxchg(&rb->head, &(long *)offset, head))
Sorry, that should be:
while (!local_try_cmpxchg(&rb->head, (long *)&offset, head))
The fundamenalthing I'm trying to say is that the
atomic/atomic64/atomic_long/local/local64 APIs should be type-safe, and for
their try_cmpxchg() implementations, the type signature should be:
${atomictype}_try_cmpxchg(${atomictype} *ptr, ${inttype} *old, ${inttype} new)
Thanks,
Mark.
WARNING: multiple messages have this Message-ID (diff)
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 16:32:52 +0000 [thread overview]
Message-ID: <ZB3QtDYuWdpiD5qk@FVFF77S0Q05N> (raw)
In-Reply-To: <ZB3MR8lGbnea9ui6@FVFF77S0Q05N>
On Fri, Mar 24, 2023 at 04:14:22PM +0000, Mark Rutland wrote:
> On Fri, Mar 24, 2023 at 04:43:32PM +0100, Uros Bizjak wrote:
> > On Fri, Mar 24, 2023 at 3:13 PM Mark Rutland <mark.rutland@arm.com> wrote:
> > >
> > > 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?
> >
> > An example is patch 10/10 from the series, which will fail without
> > this fix when fallback code is used. We have:
> >
> > - } while (local_cmpxchg(&rb->head, offset, head) != offset);
> > + } while (!local_try_cmpxchg(&rb->head, &offset, head));
> >
> > where rb->head is defined as:
> >
> > typedef struct {
> > atomic_long_t a;
> > } local_t;
> >
> > while offset is defined as 'unsigned long'.
>
> Ok, but that's because we're doing the wrong thing to start with.
>
> Since local_t is defined in terms of atomic_long_t, we should define the
> generic local_try_cmpxchg() in terms of atomic_long_try_cmpxchg(). We'll still
> have a mismatch between 'long *' and 'unsigned long *', but then we can fix
> that in the callsite:
>
> while (!local_try_cmpxchg(&rb->head, &(long *)offset, head))
Sorry, that should be:
while (!local_try_cmpxchg(&rb->head, (long *)&offset, head))
The fundamenalthing I'm trying to say is that the
atomic/atomic64/atomic_long/local/local64 APIs should be type-safe, and for
their try_cmpxchg() implementations, the type signature should be:
${atomictype}_try_cmpxchg(${atomictype} *ptr, ${inttype} *old, ${inttype} new)
Thanks,
Mark.
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 16:32:52 +0000 [thread overview]
Message-ID: <ZB3QtDYuWdpiD5qk@FVFF77S0Q05N> (raw)
In-Reply-To: <ZB3MR8lGbnea9ui6@FVFF77S0Q05N>
On Fri, Mar 24, 2023 at 04:14:22PM +0000, Mark Rutland wrote:
> On Fri, Mar 24, 2023 at 04:43:32PM +0100, Uros Bizjak wrote:
> > On Fri, Mar 24, 2023 at 3:13 PM Mark Rutland <mark.rutland@arm.com> wrote:
> > >
> > > 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?
> >
> > An example is patch 10/10 from the series, which will fail without
> > this fix when fallback code is used. We have:
> >
> > - } while (local_cmpxchg(&rb->head, offset, head) != offset);
> > + } while (!local_try_cmpxchg(&rb->head, &offset, head));
> >
> > where rb->head is defined as:
> >
> > typedef struct {
> > atomic_long_t a;
> > } local_t;
> >
> > while offset is defined as 'unsigned long'.
>
> Ok, but that's because we're doing the wrong thing to start with.
>
> Since local_t is defined in terms of atomic_long_t, we should define the
> generic local_try_cmpxchg() in terms of atomic_long_try_cmpxchg(). We'll still
> have a mismatch between 'long *' and 'unsigned long *', but then we can fix
> that in the callsite:
>
> while (!local_try_cmpxchg(&rb->head, &(long *)offset, head))
Sorry, that should be:
while (!local_try_cmpxchg(&rb->head, (long *)&offset, head))
The fundamenalthing I'm trying to say is that the
atomic/atomic64/atomic_long/local/local64 APIs should be type-safe, and for
their try_cmpxchg() implementations, the type signature should be:
${atomictype}_try_cmpxchg(${atomictype} *ptr, ${inttype} *old, ${inttype} new)
Thanks,
Mark.
next prev parent reply other threads:[~2023-03-24 16:32 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
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 [this message]
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=ZB3QtDYuWdpiD5qk@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.