On Sun, Mar 20, 2022 at 03:15:05AM +0530, Kumar Kartikeya Dwivedi wrote:
> On Sun, Mar 20, 2022 at 02:56:20AM IST, Alexei Starovoitov wrote:
> > On Sun, Mar 20, 2022 at 12:34:09AM +0530, Kumar Kartikeya Dwivedi wrote:
> > > On Sun, Mar 20, 2022 at 12:00:28AM IST, Alexei Starovoitov wrote:
> > > > On Thu, Mar 17, 2022 at 05:29:47PM +0530, Kumar Kartikeya Dwivedi wrote:
> > > > > Make adjustments to the code to allow storing percpu PTR_TO_BTF_ID in a
> > > > > map. Similar to 'kptr_ref' tag, a new 'kptr_percpu' allows tagging types
> > > > > of pointers accepting stores of such register types. On load, verifier
> > > > > marks destination register as having type PTR_TO_BTF_ID | MEM_PERCPU |
> > > > > PTR_MAYBE_NULL.
> > > > >
> > > > > Cc: Hao Luo <haoluo@xxxxxxxxxx>
> > > > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
> > > > > ---
> > > > > include/linux/bpf.h | 3 ++-
> > > > > kernel/bpf/btf.c | 13 ++++++++++---
> > > > > kernel/bpf/verifier.c | 26 +++++++++++++++++++++-----
> > > > > 3 files changed, 33 insertions(+), 9 deletions(-)
> > > > >
> > > > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > > > > index 702aa882e4a3..433f5cb161cf 100644
> > > > > --- a/include/linux/bpf.h
> > > > > +++ b/include/linux/bpf.h
> > > > > @@ -161,7 +161,8 @@ enum {
> > > > > };
> > > > >
> > > > > enum {
> > > > > - BPF_MAP_VALUE_OFF_F_REF = (1U << 0),
> > > > > + BPF_MAP_VALUE_OFF_F_REF = (1U << 0),
> > > > > + BPF_MAP_VALUE_OFF_F_PERCPU = (1U << 1),
> > > >
> > > > What is the use case for storing __percpu pointer into a map?
> > >
> > > No specific use case for me, just thought it would be useful, especially now
> > > that __percpu tag is understood by verifier for kernel BTF, so it may also refer
> > > to dynamically allocated per-CPU memory, not just global percpu variables. But
> > > fine with dropping both this and user kptr if you don't feel like keeping them.
> >
> > I prefer to drop it for now.
> > The patch is trivial but kptr_percpu tag would stay forever.
>
> Ok, I'll drop both this and user kptr for now.
>
> > Maybe we can allow storing percpu pointers in a map with just kptr tag.
> > The verifier should be able to understand from btf whether that pointer
> > is percpu or not.
>
> This won't work (unless I missed something), it is possible to see the type when
> a store is being done, but we cannot know whether the pointer was percpu or not
> when doing a load (which is needed to decide whether it will be marked with
> MEM_PERCPU, so that user has to call bpf_this_cpu_ptr or bpf_per_cpu_ptr to
> obtain actual pointer). So some extra tagging is needed.
The pointer in bpf program should probably be marked as normal __percpu then.
So that types match during both store and load.
It will be a combination of btf_tags __kptr and __percpu.
Anyway let's table this discussion until main feature lands.
