Re: [PATCH v2] LoongArch/percpu: Simplify _percpu_read() and _percpu_write()

[Uros Bizjak]

On Wed, Sep 4, 2024 at 5:24 PM Uros Bizjak <ubizjak@xxxxxxxxx> wrote:
>
> On Wed, Sep 4, 2024 at 5:02 PM Huacai Chen <chenhuacai@xxxxxxxxxx> wrote:
> >
> > Hi, Uros,
> >
> > Thank you for your patch.
> >
> > On Wed, Sep 4, 2024 at 5:52 PM Uros Bizjak <ubizjak@xxxxxxxxx> 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.