On 11/4/22 3:51 AM, Kumar Kartikeya Dwivedi wrote: > On Fri, Nov 04, 2022 at 11:27:04AM IST, Alexei Starovoitov wrote: >> On Thu, Nov 3, 2022 at 12:11 PM Kumar Kartikeya Dwivedi >> <memxor@xxxxxxxxx> wrote: >>> >>> Introduce local kptrs, i.e. PTR_TO_BTF_ID that point to a type in >>> program BTF. This is indicated by the presence of MEM_TYPE_LOCAL type >>> tag in reg->type to avoid having to check btf_is_kernel when trying to >>> match argument types in helpers. >> ... >>> >>> + /* MEM is of a type from program BTF, not kernel BTF. This is used to >>> + * tag PTR_TO_BTF_ID allocated using bpf_kptr_alloc. >>> + */ >>> + MEM_TYPE_LOCAL = BIT(11 + BPF_BASE_TYPE_BITS), >>> + >> >> I know we have bpf_core_type_id_local. >> It sort-of makes sense in the context of the program. >> type_id_local -> inside the program >> type_id_kernel -> kernel >> >> but in the context of the verifier "local kptr" doesn't read right. >> Especially in MEM_TYPE_LOCAL. > > Yes, "local kptr" is not the best name. "kptr to local type" is too verbose > though, do you have any suggestions on what to call this? > >> >> Also, since it applies to PTR_TO_BTF_ID, should it prefix with PTR_? >> Probably MEM_ is actually cleaner. >> And we're not consistent already with MEM_PERCPU. >> We can live with this inconsistency for now. >> >> So how about we rename MEM_ALLOC to MEM_RINGBUF, >> since it's special bpf_ringbuf_reserve() memory >> and use MEM_ALLOC to indicate the memory that came from bpf_obj_new ? >> > > Yes, it makes sense. I think Andrii has expressed the same wish to rename it to > something similar to MEM_RINGBUF before in [0]. I like this idea as well. I've been poking on a small refactoring patchset in the background which will get rid of (current) MEM_ALLOC anyways. > > [0]: https://lore.kernel.org/bpf/CAEf4BzYK939fgyc3LwNvoz3vPk2avyskP_3wRZO344irubXPtg@xxxxxxxxxxxxxx > >> ... which made me realize that the comment above should >> s/bpf_kptr_alloc/bpf_obj_new/ > > I'll fix the comment.
