From: Kui-Feng Lee <kuifeng@fb.com>
To: "olsajiri@gmail.com" <olsajiri@gmail.com>
Cc: "daniel@iogearbox.net" <daniel@iogearbox.net>,
Kernel Team <Kernel-team@fb.com>, Yonghong Song <yhs@fb.com>,
"ast@kernel.org" <ast@kernel.org>,
"andrii@kernel.org" <andrii@kernel.org>,
"bpf@vger.kernel.org" <bpf@vger.kernel.org>
Subject: Re: [PATCH bpf-next 1/3] bpf: Parameterize task iterators.
Date: Wed, 27 Jul 2022 06:56:46 +0000 [thread overview]
Message-ID: <9e6967ec22f410edf7da3dc6e5d7c867431e3a30.camel@fb.com> (raw)
In-Reply-To: <Yt/aXYiVmGKP282Q@krava>
On Tue, 2022-07-26 at 14:13 +0200, Jiri Olsa wrote:
> On Mon, Jul 25, 2022 at 10:17:11PM -0700, 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 | 4 ++
> > include/uapi/linux/bpf.h | 23 ++++++++++
> > kernel/bpf/task_iter.c | 81 +++++++++++++++++++++++++-----
> > ----
> > tools/include/uapi/linux/bpf.h | 23 ++++++++++
> > 4 files changed, 109 insertions(+), 22 deletions(-)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index 11950029284f..c8d164404e20 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -1718,6 +1718,10 @@ int bpf_obj_get_user(const char __user
> > *pathname, int flags);
> >
> > struct bpf_iter_aux_info {
> > struct bpf_map *map;
> > + struct {
> > + __u32 tid;
>
> should be just u32 ?
Or, should change the following 'type' to __u8?
>
> > + u8 type;
> > + } task;
> > };
> >
>
> SNIP
>
> >
> > /* BPF syscall commands, see bpf(2) man-page for more details. */
> > diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c
> > index 8c921799def4..7979aacb651e 100644
> > --- a/kernel/bpf/task_iter.c
> > +++ b/kernel/bpf/task_iter.c
> > @@ -12,6 +12,8 @@
> >
> > struct bpf_iter_seq_task_common {
> > struct pid_namespace *ns;
> > + u32 tid;
> > + u8 type;
> > };
> >
> > struct bpf_iter_seq_task_info {
> > @@ -22,18 +24,31 @@ struct bpf_iter_seq_task_info {
> > u32 tid;
> > };
> >
> > -static struct task_struct *task_seq_get_next(struct pid_namespace
> > *ns,
> > +static struct task_struct *task_seq_get_next(struct
> > bpf_iter_seq_task_common *common,
> > u32 *tid,
> > bool
> > skip_if_dup_files)
> > {
> > struct task_struct *task = NULL;
> > struct pid *pid;
> >
> > + if (common->type == BPF_TASK_ITER_TID) {
> > + if (*tid)
> > + return NULL;
>
> I tested and this condition breaks it for fd iterations, not sure
> about
> the task and vma, because they share this function
>
> if bpf_seq_read is called with small buffer there will be multiple
> calls
> to task_file_seq_get_next and second one will stop in here, even if
> there
> are more files to be displayed for the task in filter
>
> it'd be nice to have some test for this ;-) or perhaps compare with
> the
> not filtered output
>
> SNIP
>
> > static const struct seq_operations task_seq_ops = {
> > .start = task_seq_start,
> > .next = task_seq_next,
> > @@ -137,8 +166,7 @@ struct bpf_iter_seq_task_file_info {
> > static struct file *
> > task_file_seq_get_next(struct bpf_iter_seq_task_file_info *info)
> > {
> > - struct pid_namespace *ns = info->common.ns;
> > - u32 curr_tid = info->tid;
> > + u32 saved_tid = info->tid;
> > struct task_struct *curr_task;
> > unsigned int curr_fd = info->fd;
> >
> > @@ -151,21 +179,18 @@ task_file_seq_get_next(struct
> > bpf_iter_seq_task_file_info *info)
> > curr_task = info->task;
> > curr_fd = info->fd;
> > } else {
> > - curr_task = task_seq_get_next(ns, &curr_tid,
> > true);
> > + curr_task = task_seq_get_next(&info->common, &info-
> > >tid, true);
> > if (!curr_task) {
> > info->task = NULL;
> > - info->tid = curr_tid;
> > return NULL;
> > }
>
> nit, looks like we're missing proper indent in here
Yes, we mixed spaces and tabs. Should I change these lines to tabs?
>
>
> >
> > - /* set info->task and info->tid */
> > + /* set info->task */
> > info->task = curr_task;
> > - if (curr_tid == info->tid) {
> > + if (saved_tid == info->tid)
> > curr_fd = info->fd;
> > - } else {
> > - info->tid = curr_tid;
> > + else
>
> SNIP
next prev parent reply other threads:[~2022-07-27 6:56 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-26 5:17 [PATCH bpf-next 0/3] Parameterize task iterators Kui-Feng Lee
2022-07-26 5:17 ` [PATCH bpf-next 1/3] bpf: " Kui-Feng Lee
2022-07-26 12:13 ` Jiri Olsa
2022-07-26 13:19 ` Jiri Olsa
2022-07-27 6:39 ` Kui-Feng Lee
2022-07-27 6:56 ` Kui-Feng Lee [this message]
2022-07-27 8:19 ` Kumar Kartikeya Dwivedi
2022-07-28 5:25 ` Kui-Feng Lee
2022-07-28 8:47 ` Kumar Kartikeya Dwivedi
2022-07-28 15:16 ` Kui-Feng Lee
2022-07-28 16:22 ` Kumar Kartikeya Dwivedi
2022-07-28 16:40 ` Kui-Feng Lee
2022-07-28 17:08 ` Yonghong Song
2022-07-28 17:52 ` Kumar Kartikeya Dwivedi
2022-07-28 19:11 ` Hao Luo
2022-08-02 14:25 ` Kumar Kartikeya Dwivedi
2022-07-28 22:54 ` Yonghong Song
2022-07-29 9:10 ` Christian Brauner
[not found] ` <CAP01T74HRHapKDAfj104KNGnzCgNQSu_M5-KfEvGBNzLWNfd+Q@mail.gmail.com>
2022-07-30 2:46 ` Kui-Feng Lee
2022-07-28 18:01 ` Hao Luo
2022-07-26 5:17 ` [PATCH bpf-next 2/3] bpf: Handle bpf_link_info for the parameterized task BPF iterators Kui-Feng Lee
2022-07-26 5:17 ` [PATCH bpf-next 3/3] selftests/bpf: Test " Kui-Feng Lee
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9e6967ec22f410edf7da3dc6e5d7c867431e3a30.camel@fb.com \
--to=kuifeng@fb.com \
--cc=Kernel-team@fb.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=olsajiri@gmail.com \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.