| Message ID | 20200828053815.817726-1-yhs@fb.com |
|---|---|
| Headers | show |
| Series | bpf: avoid iterating duplicated files for task_file iterator | expand |
On Thu, Aug 27, 2020 at 10:38 PM Yonghong Song <yhs@fb.com> wrote: > > Modified existing bpf_iter_test_file.c program to check whether > all accessed files from the main thread or not. > > Modified existing bpf_iter_test_file program to check > whether all accessed files from the main thread or not. > $ ./test_progs -n 4 > ... > #4/7 task_file:OK > ... > #4 bpf_iter:OK > Summary: 1/24 PASSED, 0 SKIPPED, 0 FAILED > > Signed-off-by: Yonghong Song <yhs@fb.com> > --- Acked-by: Andrii Nakryiko <andriin@fb.com> > .../selftests/bpf/prog_tests/bpf_iter.c | 21 +++++++++++++++++++ > .../selftests/bpf/progs/bpf_iter_task_file.c | 10 ++++++++- > 2 files changed, 30 insertions(+), 1 deletion(-) > [...] > + if (CHECK(pthread_join(thread_id, &ret) || ret != NULL, > + "pthread_join", "pthread_join failed\n")) > + goto done; > + > + CHECK(skel->bss->count != 0, "", nit: please use non-empty string for second argument > + "invalid non pthread file visit %d\n", skel->bss->count); > + > +done: > bpf_iter_task_file__destroy(skel); > } > [...]
On 9/1/20 5:41 PM, Andrii Nakryiko wrote: > On Thu, Aug 27, 2020 at 10:38 PM Yonghong Song <yhs@fb.com> wrote: >> >> Modified existing bpf_iter_test_file.c program to check whether >> all accessed files from the main thread or not. >> >> Modified existing bpf_iter_test_file program to check >> whether all accessed files from the main thread or not. >> $ ./test_progs -n 4 >> ... >> #4/7 task_file:OK >> ... >> #4 bpf_iter:OK >> Summary: 1/24 PASSED, 0 SKIPPED, 0 FAILED >> >> Signed-off-by: Yonghong Song <yhs@fb.com> >> --- > > Acked-by: Andrii Nakryiko <andriin@fb.com> > >> .../selftests/bpf/prog_tests/bpf_iter.c | 21 +++++++++++++++++++ >> .../selftests/bpf/progs/bpf_iter_task_file.c | 10 ++++++++- >> 2 files changed, 30 insertions(+), 1 deletion(-) >> > > [...] > >> + if (CHECK(pthread_join(thread_id, &ret) || ret != NULL, >> + "pthread_join", "pthread_join failed\n")) >> + goto done; >> + >> + CHECK(skel->bss->count != 0, "", > > nit: please use non-empty string for second argument Okay, will change to "check_count" instead of empty string. Thanks! > >> + "invalid non pthread file visit %d\n", skel->bss->count); >> + >> +done: >> bpf_iter_task_file__destroy(skel); >> } >> > > [...] >
Commit e679654a704e ("bpf: Fix a rcu_sched stall issue with bpf task/task_file iterator") introduced rate limiting in bpf_seq_read() to fix a case where traversing too many tasks and files (tens of millions of files) may cause kernel rcu stall. But rate limiting won't reduce the amount of work to traverse all these files. In practice, for a user process, typically all threads belongs to that process share the same file table and there is no need to visit every thread for its files. This patch added additional logic for task_file iterator to skip tasks if those tasks are not group_leaders and their files are the same as those of group_leaders. Such reduction of unnecessary work will make iterator runtime much faster if there are a lot of non-main threads and open files for the process. Patch #1 is the kernel implementation and Patch #2 is the selftest. v1 -> v2: - for task_file, no need for additional user parameter, kernel can just skip those files already visited, and this should not impact user space. (Andrii) - to add group_leader-only customization for task will be considered later. - remove Patch #1 and sent it separately as this patch set won't depend on it any more. Yonghong Song (2): bpf: avoid iterating duplicated files for task_file iterator selftests/bpf: test task_file iterator without visiting pthreads kernel/bpf/task_iter.c | 14 ++++++++----- .../selftests/bpf/prog_tests/bpf_iter.c | 21 +++++++++++++++++++ .../selftests/bpf/progs/bpf_iter_task_file.c | 10 ++++++++- 3 files changed, 39 insertions(+), 6 deletions(-)