On Sat, Aug 13, 2022 at 3:17 PM Yonghong Song <yhs@xxxxxx> 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@xxxxxx>
> > ---
> > 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(-)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index 11950029284f..6bbe53d06faa 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -1716,8 +1716,37 @@ int bpf_obj_get_user(const char __user *pathname, int flags);
> > extern int bpf_iter_ ## target(args); \
> > int __init bpf_iter_ ## target(args) { return 0; }
> >
> > +/*
> > + * The task type of iterators.
> > + *
> > + * For BPF task iterators, they can be parameterized with various
> > + * parameters to visit only some of tasks.
> > + *
> > + * BPF_TASK_ITER_ALL (default)
> > + * Iterate over resources of every task.
> > + *
> > + * BPF_TASK_ITER_TID
> > + * Iterate over resources of a task/tid.
> > + *
> > + * BPF_TASK_ITER_TGID
> > + * Iterate over reosurces of evevry task of a process / task group.
> > + */
> > +enum bpf_iter_task_type {
> > + BPF_TASK_ITER_ALL = 0,
> > + BPF_TASK_ITER_TID,
> > + BPF_TASK_ITER_TGID,
> > +};
> > +
> > struct bpf_iter_aux_info {
> > struct bpf_map *map;
> > + struct {
> > + enum bpf_iter_task_type type;
> > + union {
> > + u32 tid;
> > + u32 tgid;
> > + u32 pid_fd;
> > + };
> > + } task;
> > };
> >
> > typedef int (*bpf_iter_attach_target_t)(struct bpf_prog *prog,
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index ffcbf79a556b..6328aca0cf5c 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -91,6 +91,14 @@ union bpf_iter_link_info {
> > struct {
> > __u32 map_fd;
> > } map;
> > + /*
> > + * Parameters of task iterators.
> > + */
>
> The comment can be put into one line.
>
> > + 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.
>
> 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.
Great catch and excellent point!
