Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200517152916.3146539-1-brgerst@gmail.com> <20200517152916.3146539-3-brgerst@gmail.com>
 <CAKwvOdnCcpS_9A2y9tMqeiAg2NfcVx=gNeA2V=+zHknit7wGkg@mail.gmail.com>
In-Reply-To: <CAKwvOdnCcpS_9A2y9tMqeiAg2NfcVx=gNeA2V=+zHknit7wGkg@mail.gmail.com>
From:   Brian Gerst <brgerst@gmail.com>
Date:   Mon, 18 May 2020 23:38:34 -0400
Message-ID: <CAMzpN2gGbZABjtLQyiTdtaL5LYW-cfVQZqj6w=oZNa9Fw9GdCQ@mail.gmail.com>
Subject: Re: [PATCH 2/7] x86/percpu: Clean up percpu_to_op()
To:     Nick Desaulniers <ndesaulniers@google.com>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@kernel.org>, Borislav Petkov <bp@alien8.de>,
        "H . Peter Anvin" <hpa@zytor.com>,
        Andy Lutomirski <luto@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, May 18, 2020 at 5:15 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Sun, May 17, 2020 at 8:29 AM Brian Gerst <brgerst@gmail.com> wrote:
> >
> > The core percpu macros already have a switch on the data size, so the switch
> > in the x86 code is redundant and produces more dead code.
> >
> > Also use appropriate types for the width of the instructions.  This avoids
> > errors when compiling with Clang.
> >
> > Signed-off-by: Brian Gerst <brgerst@gmail.com>
> > ---
> >  arch/x86/include/asm/percpu.h | 90 ++++++++++++++---------------------
> >  1 file changed, 35 insertions(+), 55 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
> > index 89f918a3e99b..233c7a78d1a6 100644
> > --- a/arch/x86/include/asm/percpu.h
> > +++ b/arch/x86/include/asm/percpu.h
> > @@ -117,37 +117,17 @@ extern void __bad_percpu_size(void);
> >  #define __pcpu_reg_imm_4(x) "ri" (x)
> >  #define __pcpu_reg_imm_8(x) "re" (x)
> >
> > -#define percpu_to_op(qual, op, var, val)               \
> > -do {                                                   \
> > -       typedef typeof(var) pto_T__;                    \
> > -       if (0) {                                        \
> > -               pto_T__ pto_tmp__;                      \
> > -               pto_tmp__ = (val);                      \
> > -               (void)pto_tmp__;                        \
> > -       }                                               \
> > -       switch (sizeof(var)) {                          \
> > -       case 1:                                         \
> > -               asm qual (op "b %1,"__percpu_arg(0)     \
> > -                   : "+m" (var)                        \
> > -                   : "qi" ((pto_T__)(val)));           \
> > -               break;                                  \
> > -       case 2:                                         \
> > -               asm qual (op "w %1,"__percpu_arg(0)     \
> > -                   : "+m" (var)                        \
> > -                   : "ri" ((pto_T__)(val)));           \
> > -               break;                                  \
> > -       case 4:                                         \
> > -               asm qual (op "l %1,"__percpu_arg(0)     \
> > -                   : "+m" (var)                        \
> > -                   : "ri" ((pto_T__)(val)));           \
> > -               break;                                  \
> > -       case 8:                                         \
> > -               asm qual (op "q %1,"__percpu_arg(0)     \
> > -                   : "+m" (var)                        \
> > -                   : "re" ((pto_T__)(val)));           \
> > -               break;                                  \
> > -       default: __bad_percpu_size();                   \
> > -       }                                               \
> > +#define percpu_to_op(size, qual, op, _var, _val)                       \
> > +do {                                                                   \
> > +       __pcpu_type_##size pto_val__ = __pcpu_cast_##size(_val);        \
> > +       if (0) {                                                        \
> > +               typeof(_var) pto_tmp__;                                 \
> > +               pto_tmp__ = (_val);                                     \
> > +               (void)pto_tmp__;                                        \
> > +       }                                                               \
>
> Please replace the whole `if (0)` block with:
> ```c
> __same_type(_var, _val);
> ```
> from include/linux/compiler.h.

The problem with __builtin_types_compatible_p() is that it considers
unsigned long and u64 (aka unsigned long long) as different types even
though they are the same width on x86-64.  While this may be a good
cleanup to look at in the future, it's not a simple drop-in
replacement.

--
Brian Gerst