Re: [PATCH bpf-next v4 4/7] bpf: Introduce BPF_F_CPU and BPF_F_ALL_CPUS flags for percpu_hash and lru_percpu_hash maps

["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:
>>

[...]

>> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
>> index 71f9931ac64cd..031a74c1b7fd7 100644
>> --- a/kernel/bpf/hashtab.c
>> +++ b/kernel/bpf/hashtab.c
>> @@ -937,24 +937,39 @@ static void free_htab_elem(struct bpf_htab *htab, struct htab_elem *l)
>>  }
>>
>>  static void pcpu_copy_value(struct bpf_htab *htab, void __percpu *pptr,
>> -                           void *value, bool onallcpus)
>> +                           void *value, bool onallcpus, u64 map_flags)
>>  {
>> +       int cpu = map_flags & BPF_F_CPU ? map_flags >> 32 : 0;
>> +       int current_cpu = raw_smp_processor_id();
>> +
>>         if (!onallcpus) {
>>                 /* copy true value_size bytes */
>> -               copy_map_value(&htab->map, this_cpu_ptr(pptr), value);
>> +               copy_map_value(&htab->map, (map_flags & BPF_F_CPU) && cpu != current_cpu ?
>> +                              per_cpu_ptr(pptr, cpu) : this_cpu_ptr(pptr), value);
>
> FWIW, I still feel like this_cpu_ptr() micro-optimization is
> unnecessary and is just a distraction. This code is called when
> user-space updates/looks up per-CPU value, it's not a hot path by any
> means where this_cpu_ptr() vs per_cpu_ptr() makes any measurable
> difference
>

OK.

I'll remove it in next revision.

>>         } else {
>>                 u32 size = round_up(htab->map.value_size, 8);
>> -               int off = 0, cpu;
>> +               int off = 0;
>> +
>> +               if (map_flags & BPF_F_CPU) {
>> +                       copy_map_value_long(&htab->map, cpu != current_cpu ?
>> +                                           per_cpu_ptr(pptr, cpu) : this_cpu_ptr(pptr), value);
>> +                       return;
>> +               }
>>
>>                 for_each_possible_cpu(cpu) {
>>                         copy_map_value_long(&htab->map, per_cpu_ptr(pptr, cpu), value + off);
>> -                       off += size;
>> +                       /* same user-provided value is used if
>> +                        * BPF_F_ALL_CPUS is specified, otherwise value is
>> +                        * an array of per-cpu values.
>> +                        */
>> +                       if (!(map_flags & BPF_F_ALL_CPUS))
>> +                               off += size;
>>                 }
>>         }
>
> this couldn't have been replaced by bpf_percpu_copy_to_user?.. (and I
> just want to emphasize how hard did you make it to review all this by
> putting those bpf_percpu_copy_to_user and bpf_percpu_copy_from_user on
> its own in patch #2, instead of having a proper refactoring patch...)
>

Sorry about this.

I will use 'bpf_percpu_copy_from_user()' here in next revision.

Thanks,
Leon