Re: [PATCH bpf-next v5 1/3] bpf: Parameterize task iterators.

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Sat, Aug 13, 2022 at 3:17 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 8/10/22 5:16 PM, Kui-Feng Lee wrote:
> > Allow creating an iterator that loops through resources of one task/thread.
> >
> > People could only create iterators to loop through all resources of
> > files, vma, and tasks in the system, even though they were interested
> > in only the resources of a specific task or process.  Passing the
> > additional parameters, people can now create an iterator to go
> > through all resources or only the resources of a task.
> >
> > Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>
> > ---
> >   include/linux/bpf.h            |  29 ++++++++
> >   include/uapi/linux/bpf.h       |   8 +++
> >   kernel/bpf/task_iter.c         | 126 ++++++++++++++++++++++++++-------
> >   tools/include/uapi/linux/bpf.h |   8 +++
> >   4 files changed, 147 insertions(+), 24 deletions(-)
> >

[...]

> > +     struct {
> > +             __u32   tid;
> > +             __u32   tgid;
> > +             __u32   pid_fd;
>
> The above is a max of kernel and user space terminologies.
> tid/pid are user space concept and tgid is a kernel space
> concept.
>
> In bpf uapi header, we have
>
> struct bpf_pidns_info {
>          __u32 pid;
>          __u32 tgid;
> };
>
> which uses kernel terminologies.
>
> So I suggest the bpf_iter_link_info.task can also
> use pure kernel terminology pid/tgid/tgid_fd here.

Except tgid_fd is a made up terminology. It is called pidfd in
documentation and even if pidfd gains add support for threads (tasks),
it would still be created through pidfd_open() syscall, probably. So
it seems better to stick to "pidfd" here.

As for pid/tgid precedent. Are we referring to
bpf_get_current_pid_tgid() BPF helper and struct bpf_pidns_info? Those
are kernel-side BPF helper and kernel-side auxiliary type to describe
return results of another in-kernel helper, so I think it's a bit less
relevant here to set a precedent.

On the other hand, if we look at user-space-facing perf_event
subsystem UAPI, for example, it seems to be using pid/tid terminology
(and so does getpid()/gettid() syscall, etc). So I wonder if for a
user-space-facing API it's better to stick with user-space-facing
terminology?

>
> Alternative, using pure user space terminology
> can be tid/pid/pid_fd but seems the kernel terminology
> might be better since we already have precedence.
>
>
> > +     } task;
> >   };
> >

[...]