[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZB3QtDYuWdpiD5qk@FVFF77S0Q05N>
Date: Fri, 24 Mar 2023 16:32:52 +0000
From: Mark Rutland <mark.rutland@....com>
To: Uros Bizjak <ubizjak@...il.com>
Cc: linux-alpha@...r.kernel.org, linux-kernel@...r.kernel.org,
loongarch@...ts.linux.dev, linux-mips@...r.kernel.org,
linuxppc-dev@...ts.ozlabs.org, linux-arch@...r.kernel.org,
linux-perf-users@...r.kernel.org, Will Deacon <will@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Boqun Feng <boqun.feng@...il.com>
Subject: Re: [PATCH 01/10] locking/atomic: Add missing cast to try_cmpxchg()
fallbacks
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@....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.
Powered by blists - more mailing lists