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

From: Uros Bizjak
Date: Wed Dec 18 2024 - 03:53:06 EST


On Wed, Dec 18, 2024 at 9:46 AM Gal Pressman <gal@xxxxxxxxxx> wrote:
>
> On 18/12/2024 10:40, Uros Bizjak wrote:
> > 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.
>
> Awesome, I'll submit a patch unless I hear any objections.

Please note that this is/was a preparation patch for further changes,
applied to akpm's mm branch (typeof() -> TYPEOF_UNQUAL()). I'll take
care to forward-port your patch to mm branch.

AFAICS, the issue triggers only when KASAN is enabled.

Thanks,
Uros.