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 4:24 PM, Alexei Starovoitov wrote:
> On Fri, Aug 18, 2023 at 02:37:41PM -0400, 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)?
> 
> +1.
> 
> I think it might conflict with kernel:
> include/linux/compiler_types.h:# define __percpu	BTF_TYPE_TAG(percpu)
> It's the same tag name, but the kernel semantics are different from our kptr
> semantics inside bpf prog.
> I think we have to use a different tag like:
> #define __percpu_kptr __attribute__((btf_type_tag("percpu_kptr")))

Agree. Will use __percpu_kptr in the next revision.

> 
>>> index 60e80e90c37d..e6348fd0a785 100644
>>> --- a/include/linux/bpf.h
>>> +++ b/include/linux/bpf.h
>>> @@ -180,14 +180,15 @@ enum btf_field_type {
>>>   	BPF_TIMER      = (1 << 1),
>>>   	BPF_KPTR_UNREF = (1 << 2),
>>>   	BPF_KPTR_REF   = (1 << 3),
>>> -	BPF_KPTR       = BPF_KPTR_UNREF | BPF_KPTR_REF,
>>> -	BPF_LIST_HEAD  = (1 << 4),
>>> -	BPF_LIST_NODE  = (1 << 5),
>>> -	BPF_RB_ROOT    = (1 << 6),
>>> -	BPF_RB_NODE    = (1 << 7),
>>> +	BPF_KPTR_PERCPU_REF   = (1 << 4),
> 
> I think _REF is redundant here. _UNREF is obsolete. We might remove it and
> rename BPF_KPTR_REF to just BPF_KPTR.
> BPF_KPTR_PERCPU should be clear enough.

Okay, will use BPF_KPTR_PERCPU in the next revision.