linux-kernel - Re: [PATCH v2] LoongArch/percpu: Simplify _percpu_read() and _percpu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAFULd4aZcWcX5J==n7N_AQJre13Jeh6eJRK1eHmdSzRS3ZagCw@mail.gmail.com>
Date: Wed, 4 Sep 2024 20:37:50 +0200
From: Uros Bizjak <ubizjak@...il.com>
To: Huacai Chen <chenhuacai@...nel.org>
Cc: loongarch@...ts.linux.dev, linux-kernel@...r.kernel.org, 
	WANG Xuerui <kernel@...0n.name>, Xi Ruoyao <xry111@...111.site>, 
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH v2] LoongArch/percpu: Simplify _percpu_read() and _percpu_write()

On Wed, Sep 4, 2024 at 5:24 PM Uros Bizjak <ubizjak@...il.com> wrote:
>
> On Wed, Sep 4, 2024 at 5:02 PM Huacai Chen <chenhuacai@...nel.org> wrote:
> >
> > Hi, Uros,
> >
> > Thank you for your patch.
> >
> > On Wed, Sep 4, 2024 at 5:52 PM Uros Bizjak <ubizjak@...il.com> wrote:
> > >
> > > _percpu_read() and _percpu_write() macros call __percpu_read()
> > > and __percpu_write() static inline functions that result in a single
> > > assembly instruction. Percpu infrastructure expects its leaf
> > > definitions to encode the size of their percpu variable, so the patch
> > > merges asm clauses from the static inline function into the
> > > corresponding leaf macros.
> > It seems in some other places we prefer inline functions rather than
> > macros, but this patch is the opposite...
>
> Please note that these are leaf macros (functions), always used
> through the upper level macro (see e.g. the definition of
> raw_cpu_read() and __pcpu_size_call_return() in
> include/linux/percpu-defs.h). These upper level macros do type check
> on the pointer, so there is no need to do it again in the leaf macro.
> The percpu address space checks on x86 depend on the presence of these
> checks.
>
> > >
> > > The secondary effect of this change is to avoid explicit __percpu
> > > function arguments. Currently, __percpu macro is defined in
> > > include/linux/compiler_types.h, but with proposed patch [1],
> > > __percpu definition will need macros from include/asm-generic/percpu.h,
> > > creating forward dependency loop.
> > Macros don't check types, so use macros to drop "__percpu" checking?
> > Seems a little strange.
>
> As explained above, types are checked in the upper level macro (that
> uses these leaf macros) through __verify_pcpu_ptr(). These checks
> currently use sparse to check __percpu tag, but x86 will soon use the
> compiler infrastructure with much more powerful checks in this place.
>
> So, there is really no need to type check percpu pointer also in leaf functions.

OTOH, you are right, we should also typecheck _val in case of
_percpu_write(). We need to add:

    if (0) {                                                \
        typeof(_var) pto_tmp__;                    \
        pto_tmp__ = (_val);                    \
        (void)pto_tmp__;                    \
    }                                \

to the _percpu_write() macro.

Let me test this amendment a bit, please expect the V3 patch tomorrow.

Thanks,
Uros.