On Mon, 2022-08-15 at 22:02 -0700, Andrii Nakryiko wrote:
> On Wed, Aug 10, 2022 at 5:17 PM Kui-Feng Lee <kuifeng@xxxxxx> 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;
>
> You don't seem to use pid_fd in bpf_iter_aux_info at all, is that
> right? Drop it? And for tid/tgid, I'd use kernel-side terminology for
> this internal data structure and just have single u32 pid here. Then
> type determines whether you are iterating tasks or task leaders
> (processes), no ambiguity.
Yes, it should be removed from struct bpf_iter_aux_info.
Using just single u32 pid here is also fine to distinguish field names
by the type of data instead of the purpose of data.
>
> > };
> >
> > 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.
> > + */
> > + struct {
> > + __u32 tid;
> > + __u32 tgid;
> > + __u32 pid_fd;
> > + } task;
> > };
> >
>
> [...]
