Re: [PATCH bpf-next 02/15] bpf: Add BPF_KPTR_PERCPU_REF as a field type

[Yonghong Song <yonghong.song@linux.dev>]

On 8/18/23 11:37 AM, David Marchevsky wrote:
> On 8/14/23 1:28 PM, Yonghong Song wrote:
>> BPF_KPTR_PERCPU_REF represents a percpu field type like below
>>
>>    struct val_t {
>>      ... fields ...
>>    };
>>    struct t {
>>      ...
>>      struct val_t __percpu *percpu_data_ptr;
>>      ...
>>    };
>>
>> where
>>    #define __percpu __attribute__((btf_type_tag("percpu")))
> 
> nit: Maybe this should be __percpu_kptr (and similar for the actual tag)?
> 
> I don't feel strongly about this. It's certainly less concise, but given that
> existing docs mention kptrs a few times, and anyone using percpu kptrs can
> probably be expected to have some familiarity with "normal" kptrs, making
> it more clear to BPF program writers that their existing mental model of
> what a kptr is and how it should be used seems beneficial.

Thanks for suggestion. As Alexei suggested later as well,
__percpu_kptr is better than __percpu so users won't be confused
with kernel __percpu tag.

> 
>>
>> While BPF_KPTR_REF points to a trusted kernel object or a trusted
>> local object, BPF_KPTR_PERCPU_REF points to a trusted local
>> percpu object.
>>
>> This patch added basic support for BPF_KPTR_PERCPU_REF
>> related to percpu field parsing, recording and free operations.
>> BPF_KPTR_PERCPU_REF also supports the same map types
>> as BPF_KPTR_REF does.
>>
>> Note that unlike a local kptr, it is possible that
>> a BPF_KTPR_PERCUP_REF struct may not contain any
> 
> nit: typo here ("BPF_KTPR_PERCUP_REF" -> "BPF_KPTR_PERCPU_REF")

Ack. Thanks!

> 
>> special fields like other kptr, bpf_spin_lock, bpf_list_head, etc.
>>
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
>>   include/linux/bpf.h  | 18 ++++++++++++------
>>   kernel/bpf/btf.c     |  5 +++++
>>   kernel/bpf/syscall.c |  7 ++++++-
>>   3 files changed, 23 insertions(+), 7 deletions(-)
>>
[...]