On Tue, Aug 13, 2024 at 9:25 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > On Sun, 2024-08-11 at 18:13 +0200, Uros Bizjak wrote: > > In arraymap.c: > > > > In bpf_array_map_seq_start() and bpf_array_map_seq_next() > > cast return values from the __percpu address space to > > the generic address space via uintptr_t [1]. > > > > Correct the declaration of pptr pointer in __bpf_array_map_seq_show() > > to void __percpu * and cast the value from the generic address > > space to the __percpu address space via uintptr_t [1]. > > > > In hashtab.c: > > > > Assign the return value from bpf_mem_cache_alloc() to void pointer > > and cast the value to void __percpu ** (void pointer to percpu void > > pointer) before dereferencing. > > > > In memalloc.c: > > > > Explicitly declare __percpu variables. > > > > Cast obj to void __percpu **. > > > > In helpers.c: > > > > Cast ptr in BPF_CALL_1 and BPF_CALL_2 from generic address space > > to __percpu address space via const uintptr_t [1]. > > > > Found by GCC's named address space checks. > > > > There were no changes in the resulting object files. > > > > [1] https://sparse.docs.kernel.org/en/latest/annotations.html#address-space-name > > > > Signed-off-by: Uros Bizjak <ubizjak@xxxxxxxxx> > > Cc: Alexei Starovoitov <ast@xxxxxxxxxx> > > Cc: Daniel Borkmann <daniel@xxxxxxxxxxxxx> > > Cc: Andrii Nakryiko <andrii@xxxxxxxxxx> > > Cc: Martin KaFai Lau <martin.lau@xxxxxxxxx> > > Cc: Eduard Zingerman <eddyz87@xxxxxxxxx> > > Cc: Song Liu <song@xxxxxxxxxx> > > Cc: Yonghong Song <yonghong.song@xxxxxxxxx> > > Cc: John Fastabend <john.fastabend@xxxxxxxxx> > > Cc: KP Singh <kpsingh@xxxxxxxxxx> > > Cc: Stanislav Fomichev <sdf@xxxxxxxxxxx> > > Cc: Hao Luo <haoluo@xxxxxxxxxx> > > Cc: Jiri Olsa <jolsa@xxxxxxxxxx> > > --- > > v2: - cast return values from the __percpu address space to > > the generic address space in bpf_array_map_seq_{start,next}(). > > - correct the declaration of pptr pointer in > > __bpf_array_map_seq_show() to void __percpu * > > --- > > Looks ok, thank you for applying suggested changes. > The only nit I have is that '(void *)(uintptr_t)p' (and it's inverse) > looks quite bulky, hiding it behind some macro might make some sense. True, but as argued in [1], the macro wouldn't be flexible enough to cover all possible variants of the casts. Fortunately, this casting pattern appears only a few times, and perhaps its bulky appearance would motivate developers to rethink the cast. [1] https://lore.kernel.org/lkml/CAFULd4YOf0Mz-JbR6LEWxM2M=4GTxqC9m-q_QAZJw8Ws16yrTA@xxxxxxxxxxxxxx/ > Acked-by: Eduard Zingerman <eddyz87@xxxxxxxxx> Thanks! BR, Uros.
