[GIT PULL] asm-generic fixes for 6.3

[GIT PULL] asm-generic fixes for 6.3

[Arnd Bergmann]

The following changes since commit fe15c26ee26efa11741a7b632e9f23b01aca4cc6:

  Linux 6.3-rc1 (2023-03-05 14:52:03 -0800)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/arnd/asm-generic.git tags/asm-generic-fixes-6.3

for you to fetch changes up to 656e9007ef5862746cdf7ac16267c8e06e7b0989:

  asm-generic: avoid __generic_cmpxchg_local warnings (2023-04-04 17:58:11 +0200)

----------------------------------------------------------------
asm-generic fixes for 6.3

These are minor fixes to address false-positive build warnings:

Some of the less common I/O accessors are missing __force casts and
cause sparse warnings for their implied byteswap, and a recent change
to __generic_cmpxchg_local() causes a warning about constant integer
truncation.

----------------------------------------------------------------
Arnd Bergmann (1):
      asm-generic: avoid __generic_cmpxchg_local warnings

Vladimir Oltean (2):
      asm-generic/io.h: suppress endianness warnings for readq() and writeq()
      asm-generic/io.h: suppress endianness warnings for relaxed accessors

 include/asm-generic/atomic.h        |  4 ++--
 include/asm-generic/cmpxchg-local.h | 12 ++++++------
 include/asm-generic/cmpxchg.h       |  6 +++---
 include/asm-generic/io.h            | 16 ++++++++--------
 4 files changed, 19 insertions(+), 19 deletions(-)

Re: [GIT PULL] asm-generic fixes for 6.3

[Linus Torvalds]

On Thu, Apr 6, 2023 at 1:13 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> Some of the less common I/O accessors are missing __force casts and
> cause sparse warnings for their implied byteswap, and a recent change
> to __generic_cmpxchg_local() causes a warning about constant integer
> truncation.

Ugh. I'm not super-happy about those casts, and maybe sparse should be
less chatty about these things. It shouldn't be impossible to have
sparse not warn about losing bits in casts in code that is statically
dead.

But we seem to have lost our sparse maintainer, so I've pulled this.

I also wish we had a size-specific version of "_Generic()" instead of
having to play games with "switch (sizeof(..))" like we traditionally
do.

But things like xchg() and user accesses really just care about the
size of the object, and there is no size-specific "_Generic()" thing,
and I can't think of any cute trick either.

               Linus

Re: [GIT PULL] asm-generic fixes for 6.3

[pr-tracker-bot]

The pull request you sent on Thu, 06 Apr 2023 10:12:53 +0200:

> https://git.kernel.org/pub/scm/linux/kernel/git/arnd/asm-generic.git tags/asm-generic-fixes-6.3

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/fcff5f99eaf06ff6818e14751ffeeb677a325127

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

Re: [GIT PULL] asm-generic fixes for 6.3

[Arnd Bergmann]

On Thu, Apr 6, 2023, at 19:04, Linus Torvalds wrote:
> On Thu, Apr 6, 2023 at 1:13 AM Arnd Bergmann <arnd@arndb.de> wrote:
>>
>> Some of the less common I/O accessors are missing __force casts and
>> cause sparse warnings for their implied byteswap, and a recent change
>> to __generic_cmpxchg_local() causes a warning about constant integer
>> truncation.
>
> Ugh. I'm not super-happy about those casts, and maybe sparse should be
> less chatty about these things. It shouldn't be impossible to have
> sparse not warn about losing bits in casts in code that is statically
> dead.
>
> But we seem to have lost our sparse maintainer, so I've pulled this.
>
> I also wish we had a size-specific version of "_Generic()" instead of
> having to play games with "switch (sizeof(..))" like we traditionally
> do.
>
> But things like xchg() and user accesses really just care about the
> size of the object, and there is no size-specific "_Generic()" thing,
> and I can't think of any cute trick either.

There is actually one idea I had a while ago that would (mostly)
solve this:

As far as I can tell, almost no users of
{cmp,}xchg{,_local,_relaxed,acquire,release} that actually use
8-bit and 16-bit objects, and they are not even implemented on
some architectures.

There is already a special case for the 64-bit xchg()/cmpxchg()
variants that can get called on 32-bit architectures, so what
I'd prefer is having each architecture implement only explicit
fixed length cmpxchg8(), cmpxchg16(), cmpgxchg32() and optionally
cmpxchg64() interfaces as normal inline functions that work on
the respective integer types.

The existing interfaces then just need to deal with non-integer
arguments (four byte structures, pointers) that they handle today,
as well as multiplexing between the 32-bit and 64-bit integers
on 64-bit architectures. That still leaves a theoretical sparse
warning when something passes a 64-bit constant, but I don't think
any code does that.

     Arnd

Re: [GIT PULL] asm-generic fixes for 6.3

[Linus Torvalds]

On Thu, Apr 6, 2023 at 1:03 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> As far as I can tell, almost no users of
> {cmp,}xchg{,_local,_relaxed,acquire,release} that actually use
> 8-bit and 16-bit objects, and they are not even implemented on
> some architectures.

Yeah, I think we only have a driver or two that wants to do a 8/16-bit
cmpxchg, and many architectures don't support it at all natively (ie
it has to be implemented as a load-mask-store).

But iirc we *did* have a couple of uses, so..

> There is already a special case for the 64-bit xchg()/cmpxchg()
> variants that can get called on 32-bit architectures, so what
> I'd prefer is having each architecture implement only explicit
> fixed length cmpxchg8(), cmpxchg16(), cmpgxchg32() and optionally
> cmpxchg64() interfaces as normal inline functions that work on
> the respective integer types.

Hmm. Maybe. Except the reason we have those size-dependent macros is
that it is *really* really convenient - to the point of being very
important - when different architectures (or different kernel
configurations) end up causing the same logical thing to have
different sizes.

The whole "switch (sizeof(x))" model with complex macros is very ugly
and I'd much rather strive to implement something that is much more
geared to actual compiler issues where the compiler really explicitly
does select the right thign (rather than have the preprocessor expand
it to multiple things and then depending on the optimizer to do the
"selection").

But at the same time, aside from the macro ugliness, it really ends up
being *hugely* powerful as a concept, and avoiding a lot of
conditionals in the use cases.

Now, cmpxchg *may* be uncommon enough that the use cases are limited
enough that the "generic size" model wouldn't be worth it there, but I
was really thinking about the more generic cases.

Things like "get_user()" and "put_user()" would be *entirely* useless
without the automatic sizing.

How do I know? because we originally didn't have "get_user()". We had
"get_fs_byte()" and "get_fs_word()" etc. The "fs" part was due to the
original x86 implementation, so ignore the naming, but the issue with
"just use fixed types" really DOES NOT WORK when different
architectures have different types.

So switching over to "get_user()" that just automatically did the
right thing based on size instead of the size-specific ones really was
a huge deal. There's no way we could ever go back.

And while 'cmpxchg()' is the problem case here, I really think it
would be lovely if we had some nice *unified* way of handling these
"use size of access to determine variant" thing. Something like
_Generic, just based on size.

I guess __builtin_choose_expr() might be the right way to go there.

That's very much a "compiler statically picks one over the other"
interface. I despise the syntax, which is why it's not very commonly
used, but it might be what we should use.

Of course, another reason - other than syntax - for our "switch
(sizeof())" model is simply history. The kernel "get_user()"
interfaces go back to 1995 and Linux-1.3.0, and actually predates
__builtin_choose_expt() support in gcc by several years. Gcc only
started doing that in 2001 (gcc-3.4.5)

So we do have a few uses of __builtin_choose_expr() in the kernel, but
our really old interfaces simply didn't even have it available.

                 Linus