All of lore.kernel.org
 help / color / mirror / Atom feed
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 v9 0/5] Parameterize task iterators.
Date: Wed, 31 Aug 2022 17:31:10 +0000	[thread overview]
Message-ID: <877be66f99a4981dd9645657fa92466807701d71.camel@fb.com> (raw)
In-Reply-To: <Yw9GzoOhUBxSs8fz@krava>

On Wed, 2022-08-31 at 13:32 +0200, Jiri Olsa wrote:
> On Tue, Aug 30, 2022 at 07:37:39PM -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.
> > 
> > Major Changes:
> > 
> >  - Add new parameters in bpf_iter_link_info to indicate to go
> > through
> >    all tasks or to go through a specific task.
> > 
> >  - Change the implementations of BPF iterators of vma, files, and
> >    tasks to allow going through only the resources of a specific
> > task.
> > 
> >  - Provide the arguments of parameterized task iterators in
> >    bpf_link_info.
> 
> hi,
> I'm getting bpf_iter/vma_offset fail:
> 
> test_task_vma_offset_common:PASS:bpf_iter_vma_offset__open_and_load 0
> nsec
> test_task_vma_offset_common:PASS:attach_iter 0 nsec
> test_task_vma_offset_common:PASS:create_iter 0 nsec
> test_task_vma_offset_common:PASS:strcmp 0 nsec
> test_task_vma_offset_common:FAIL:offset unexpected offset: actual
> 257222 != expected 203974
> test_task_vma_offset_common:PASS:unique_tgid_count 0 nsec
> test_task_vma_offset_common:PASS:bpf_iter_vma_offset__open_and_load 0
> nsec
> test_task_vma_offset_common:PASS:attach_iter 0 nsec
> test_task_vma_offset_common:PASS:create_iter 0 nsec
> test_task_vma_offset_common:PASS:strcmp 0 nsec
> test_task_vma_offset_common:FAIL:offset unexpected offset: actual
> 257222 != expected 203974
> test_task_vma_offset_common:PASS:unique_tgid_count 0 nsec
> test_task_vma_offset_common:PASS:bpf_iter_vma_offset__open_and_load 0
> nsec
> test_task_vma_offset_common:PASS:attach_iter 0 nsec
> test_task_vma_offset_common:PASS:create_iter 0 nsec
> test_task_vma_offset_common:PASS:strcmp 0 nsec
> test_task_vma_offset_common:FAIL:offset unexpected offset: actual
> 257222 != expected 203974
> test_task_vma_offset_common:PASS:unique_tgid_count 0 nsec
> #11/38   bpf_iter/vma_offset:FAIL
> 
> jirka

I could not reproduce it.  However, I found it should be an incorrect
boundary check of computing page_shift.  On my device, it happened to
be loaded with an offset 0 from the head of the file, so it passed the
test case accidentally.

Anyway, I will fix it.

> 
> > 
> > Differences from v8:
> > 
> >  - Fix uninitialized variable.
> > 
> >  - Avoid redundant work of getting task from pid.
> > 
> >  - Change format string to use %u instead of %d.
> > 
> >  - Use the value of page_shift to compute correct offset in
> >    bpf_iter_vm_offset.c.
> > 
> > Differences from v7:
> > 
> >  - Travel the tasks of a process through task_group linked list
> >    instead of traveling through the whole namespace.
> > 
> > Differences from v6:
> > 
> >  - Add part 5 to make bpftool show the value of parameters.
> > 
> >  - Change of wording of show_fdinfo() to show pid or tid instead of
> >    always pid.
> > 
> >  - Simplify error handling and naming of test cases.
> > 
> > Differences from v5:
> > 
> >  - Use user-space tid/pid terminologies in bpf_iter_link_info and
> >    bpf_link_info.
> > 
> >  - Fix reference count
> > 
> >  - Merge all variants to one 'u32 pid' in internal structs.
> >    (bpf_iter_aux_info and bpf_iter_seq_task_common)
> > 
> >  - Compare the result of get_uprobe_offset() with the
> > implementation
> >    with the vma iterators.
> > 
> >  - Implement show_fdinfo.
> > 
> > Differences from v4:
> > 
> >  - Remove 'type' from bpf_iter_link_info and bpf_link_info.
> > 
> > v8:
> > https://lore.kernel.org/bpf/20220829192317.486946-1-kuifeng@fb.com/
> > v7:
> > https://lore.kernel.org/bpf/20220826003712.2810158-1-kuifeng@fb.com/
> > v6:
> > https://lore.kernel.org/bpf/20220819220927.3409575-1-kuifeng@fb.com/
> > v5:
> > https://lore.kernel.org/bpf/20220811001654.1316689-1-kuifeng@fb.com/
> > v4:
> > https://lore.kernel.org/bpf/20220809195429.1043220-1-kuifeng@fb.com/
> > v3:
> > https://lore.kernel.org/bpf/20220809063501.667610-1-kuifeng@fb.com/
> > v2:
> > https://lore.kernel.org/bpf/20220801232649.2306614-1-kuifeng@fb.com/
> > v1:
> > https://lore.kernel.org/bpf/20220726051713.840431-1-kuifeng@fb.com/
> > 
> > Kui-Feng Lee (5):
> >   bpf: Parameterize task iterators.
> >   bpf: Handle bpf_link_info for the parameterized task BPF
> > iterators.
> >   bpf: Handle show_fdinfo for the parameterized task BPF iterators
> >   selftests/bpf: Test parameterized task BPF iterators.
> >   bpftool: Show parameters of BPF task iterators.
> > 
> >  include/linux/bpf.h                           |  25 ++
> >  include/uapi/linux/bpf.h                      |  10 +
> >  kernel/bpf/task_iter.c                        | 227 ++++++++++++--
> >  tools/bpf/bpftool/link.c                      |  19 ++
> >  tools/include/uapi/linux/bpf.h                |  10 +
> >  .../selftests/bpf/prog_tests/bpf_iter.c       | 282
> > ++++++++++++++++--
> >  .../selftests/bpf/prog_tests/btf_dump.c       |   2 +-
> >  .../selftests/bpf/progs/bpf_iter_task.c       |   9 +
> >  .../selftests/bpf/progs/bpf_iter_task_file.c  |   9 +-
> >  .../selftests/bpf/progs/bpf_iter_task_vma.c   |   7 +-
> >  .../selftests/bpf/progs/bpf_iter_vma_offset.c |  37 +++
> >  11 files changed, 591 insertions(+), 46 deletions(-)
> >  create mode 100644
> > tools/testing/selftests/bpf/progs/bpf_iter_vma_offset.c
> > 
> > -- 
> > 2.30.2
> > 


      reply	other threads:[~2022-08-31 17:31 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-31  2:37 [PATCH bpf-next v9 0/5] Parameterize task iterators Kui-Feng Lee
2022-08-31  2:37 ` [PATCH bpf-next v9 1/5] bpf: " Kui-Feng Lee
2022-08-31  4:04   ` Yonghong Song
2022-08-31  2:37 ` [PATCH bpf-next v9 2/5] bpf: Handle bpf_link_info for the parameterized task BPF iterators Kui-Feng Lee
2022-08-31  2:37 ` [PATCH bpf-next v9 3/5] bpf: Handle show_fdinfo " Kui-Feng Lee
2022-08-31  2:37 ` [PATCH bpf-next v9 4/5] selftests/bpf: Test " Kui-Feng Lee
2022-08-31  3:49   ` Yonghong Song
2022-08-31  2:37 ` [PATCH bpf-next v9 5/5] bpftool: Show parameters of BPF task iterators Kui-Feng Lee
2022-08-31 11:32 ` [PATCH bpf-next v9 0/5] Parameterize " Jiri Olsa
2022-08-31 17:31   ` Kui-Feng Lee [this message]

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=877be66f99a4981dd9645657fa92466807701d71.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.