All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Qian Cai <cai@lca.pw>, Jason Gunthorpe <jgg@mellanox.com>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Mark Rutland <mark.rutland@arm.com>
Subject: Re: [PATCH] mm/debug: add a cast to u64 for atomic64_read()
Date: Wed, 13 Mar 2019 14:46:55 +0100	[thread overview]
Message-ID: <CAK8P3a3_2O7KBKTSD-QC5tcpohy8bkVVHsdAJnanTU1B+H12-w@mail.gmail.com> (raw)
In-Reply-To: <20190313091844.GA24390@hirez.programming.kicks-ass.net>

On Wed, Mar 13, 2019 at 10:19 AM Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Mar 11, 2019 at 03:20:04PM +0100, Arnd Bergmann wrote:
> > On Mon, Mar 11, 2019 at 3:00 PM Qian Cai <cai@lca.pw> wrote:
> >
> > At least the atomic_long part we discussed there has been resolved now
> > as part of commit b5d47ef9ea5c ("locking/atomics: Switch to generated
> > atomic-long").
> >
> > Adding Mark Rutland to Cc, maybe he has some ideas of how to use
> > the infrastructure he added to use consistent types for atomic64()
> > on the remaining 64-bit architectures.
>
> A quick count shows there's only 5 definitions of atomic64_t in the
> tree, it would be trivial to align them on type.
>
> $ git grep "} atomic64_t"
> arch/arc/include/asm/atomic.h:} atomic64_t;
> arch/arm/include/asm/atomic.h:} atomic64_t;
> arch/x86/include/asm/atomic64_32.h:} atomic64_t;
> include/asm-generic/atomic64.h:} atomic64_t;
> include/linux/types.h:} atomic64_t;

Right, that would make sense as well.

> Note that the one used in _most_ cases, is the one from linux/types.h,
> and that is using 'long'. The others, all typically on ILP32 platforms,
> obviously must use long long.
>
> I have no objection to changing the types.h one to long long or all of
> them to s64. It really shouldn't matter at all.

I thiunk it needs an '__attribute__((aligned(8)))' annotation at least on
x86-32, but it should be harmless to do that everywhere. The
32-bit architectures of course already use a 'long long' base type
(unsigned long long on x86 and arc), but we'd still need to
change all the 64-bit architectures to consistently use s64
in their implementation. This would be the majority of the work, e.g.

arch/powerpc/include/asm/atomic.h:
static __inline__ void atomic64_##op(long a, atomic64_t *v)             \

arch/riscv/include/asm/atomic.h
static __always_inline                                                  \
c_type atomic##prefix##_fetch_##op(c_type i, atomic##prefix##_t *v)     \

arch/sparc/include/asm/atomic_64.h:
long atomic64_##op##_return(long, atomic64_t *);

arch/s390/include/asm/atomic.h:
static inline void atomic64_##op(long i, atomic64_t *v)                 \

arch/mips/include/asm/atomic.h:
static __inline__ void atomic64_##op(long i, atomic64_t * v)                  \

arch/ia64/include/asm/atomic.h:
static __inline__ long                                                  \
ia64_atomic64_##op (__s64 i, atomic64_t *v)                             \

arch/alpha/include/asm/atomic.h:
static __inline__ void atomic64_##op(long i, atomic64_t * v)            \

arch/parisc/include/asm/atomic.h:
static __inline__ s64 atomic64_##op##_return(s64 i, atomic64_t *v)      \

The problem is not that any of those would be hard to change,
it's more that there are so many functions across 10 architectures,
and everything has some subtle differences somewhere.

It would be tempting to use scripts/atomic/* to generate more of
the code in a consistent way, but that is likely to be even more
work and more error-prone at the start.

      Arnd

  reply	other threads:[~2019-03-13 13:47 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-10 18:30 Qian Cai
2019-03-11  3:58 ` Davidlohr Bueso
2019-03-11 12:21   ` Jason Gunthorpe
2019-03-11 14:00     ` Qian Cai
2019-03-11 14:00       ` Qian Cai
2019-03-11 14:20       ` Arnd Bergmann
2019-03-11 14:20         ` Arnd Bergmann
2019-03-13  9:18         ` Peter Zijlstra
2019-03-13 13:46           ` Arnd Bergmann [this message]
2019-03-13 13:46             ` Arnd Bergmann
2019-03-13 14:21             ` Peter Zijlstra
2019-03-13 14:24             ` Peter Zijlstra
2019-03-13 14:35           ` Mark Rutland
2019-03-13 14:57             ` Arnd Bergmann
2019-03-13 14:57               ` Arnd Bergmann
2019-03-13 16:29               ` Mark Rutland
2019-03-11 14:00     ` Arnd Bergmann
2019-03-11 14:00       ` Arnd Bergmann

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=CAK8P3a3_2O7KBKTSD-QC5tcpohy8bkVVHsdAJnanTU1B+H12-w@mail.gmail.com \
    --to=arnd@arndb.de \
    --cc=akpm@linux-foundation.org \
    --cc=cai@lca.pw \
    --cc=jgg@mellanox.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mark.rutland@arm.com \
    --cc=peterz@infradead.org \
    --subject='Re: [PATCH] mm/debug: add a cast to u64 for atomic64_read()' \
    /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

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.