Re: [PATCH 3/3] percpu: Cast percpu pointer in PERCPU_PTR() via unsigned long

[Uros Bizjak]

On Wed, Dec 18, 2024 at 8:54 AM Gal Pressman <gal@xxxxxxxxxx> wrote:
>
> On 21/10/2024 11:07, Uros Bizjak wrote:
> > Cast pointer from percpu address space to generic (kernel) address
> > space in PERCPU_PTR() macro via unsigned long intermediate cast [1].
> > This intermediate cast is also required to avoid build failure
> > when GCC's strict named address space checks for x86 targets [2]
> > are enabled.
> >
> > Found by GCC's named address space checks.
> >
> > [1] https://sparse.docs.kernel.org/en/latest/annotations.html#address-space-name
> > [2] https://gcc.gnu.org/onlinedocs/gcc/Named-Address-Spaces.html#x86-Named-Address-Spaces
> >
> > Signed-off-by: Uros Bizjak <ubizjak@xxxxxxxxx>
> > Cc: Dennis Zhou <dennis@xxxxxxxxxx>
> > Cc: Tejun Heo <tj@xxxxxxxxxx>
> > Cc: Christoph Lameter <cl@xxxxxxxxx>
> > Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> > ---
> >  include/linux/percpu-defs.h | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/percpu-defs.h b/include/linux/percpu-defs.h
> > index e1cf7982424f..35842d1e3879 100644
> > --- a/include/linux/percpu-defs.h
> > +++ b/include/linux/percpu-defs.h
> > @@ -221,7 +221,10 @@ do {                                                                     \
> >  } while (0)
> >
> >  #define PERCPU_PTR(__p)                                                      \
> > -     (typeof(*(__p)) __force __kernel *)(__p);
> > +({                                                                   \
> > +     unsigned long __pcpu_ptr = (__force unsigned long)(__p);        \
> > +     (typeof(*(__p)) __force __kernel *)(__pcpu_ptr);                \
> > +})
> >
> >  #ifdef CONFIG_SMP
> >
>
> Hello Uros,
>
> We've encountered a kernel panic on boot [1] bisected to this patch.
> I believe the patch is fine and the issue is caused by a compiler bug.
> The panic reproduces when compiling the kernel with gcc 11.3.1, but does
> not reproduce with latest gcc/clang.
>
> I have a patch that workarounds the issue by ditching the intermediate
> variable and does the casting in a single line. Will that be enough to
> solve the sparse/build issues?

Yes, single line like:

(typeof(*(__p)) __force __kernel *)(__force unsigned long)(__pcpu_ptr);

should be OK.

Thanks,
Uros.