Re: [PATCH bpf-next v4 2/7] bpf: Introduce BPF_F_CPU and BPF_F_ALL_CPUS flags

["Leon Hwang" <leon.hwang@linux.dev>]

On Thu Aug 28, 2025 at 7:18 AM +08, Andrii Nakryiko wrote:
> On Wed, Aug 27, 2025 at 9:45 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>>

[...]

>>
>> +#ifdef CONFIG_BPF_SYSCALL
>> +static inline void bpf_percpu_copy_to_user(struct bpf_map *map, void __percpu *pptr, void *value,
>> +                                          u32 size, u64 flags)
>> +{
>> +       int current_cpu = raw_smp_processor_id();
>> +       int cpu, off = 0;
>> +
>> +       if (flags & BPF_F_CPU) {
>> +               cpu = flags >> 32;
>> +               copy_map_value_long(map, value, cpu != current_cpu ? per_cpu_ptr(pptr, cpu) :
>> +                                   this_cpu_ptr(pptr));
>> +               check_and_init_map_value(map, value);
>
> I'm not sure it's the question to you, but why would we
> "check_and_init_map_value" when copying data to user space?... this is
> so confusing...
>

After reading its code, I think it's to hide some kernel details from
user space, e.g. refcount, list nodes, rb nodes.

>> +       } else {
>> +               for_each_possible_cpu(cpu) {
>> +                       copy_map_value_long(map, value + off, per_cpu_ptr(pptr, cpu));
>> +                       check_and_init_map_value(map, value + off);
>> +                       off += size;
>> +               }
>> +       }
>> +}
>> +
>> +void bpf_obj_free_fields(const struct btf_record *rec, void *obj);
>> +
>> +static inline void bpf_percpu_copy_from_user(struct bpf_map *map, void __percpu *pptr, void *value,
>> +                                            u32 size, u64 flags)
>> +{

[...]

>> +}
>> +#endif
>
> hm... these helpers are just here with no way to validate that they
> generalize existing logic correctly... Do a separate patch where you
> introduce this helper before adding per-CPU flags *and* make use of
> them in existing code? Then we can check that you didn't introduce any
> subtle differences? Then in this patch you can adjust helpers to
> handle BPF_F_CPU and BPF_F_ALL_CPUS?
>

Get it.

I'll send a separate patch later.

Thanks,
Leon