From:   Jiri Olsa <olsajiri@gmail.com>
Date:   Thu, 9 Feb 2023 15:18:26 +0100
To:     Alexei Starovoitov <ast@kernel.org>,
        Andrii Nakryiko <andrii@kernel.org>,
        Hao Luo <haoluo@google.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Matthew Wilcox <willy@infradead.org>
Cc:     bpf@vger.kernel.org, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        linux-perf-users@vger.kernel.org, Martin KaFai Lau <kafai@fb.com>,
        Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@chromium.org>,
        Stanislav Fomichev <sdf@google.com>,
        Daniel Borkmann <daniel@iogearbox.net>
Subject: Re: [RFC 0/5] mm/bpf/perf: Store build id in file object
Message-ID: <Y+UAsr8A+xT0bUY/@krava>
References: <20230201135737.800527-1-jolsa@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230201135737.800527-1-jolsa@kernel.org>
Precedence: bulk

On Wed, Feb 01, 2023 at 02:57:32PM +0100, Jiri Olsa wrote:
> hi,
> we have a use cases for bpf programs to use binary file's build id.
> 
> After some attempts to add helpers/kfuncs [1] [2] Andrii had an idea [3]
> to store build id directly in the file object. That would solve our use
> case and might be beneficial for other profiling/tracing use cases with
> bpf programs.
> 
> This RFC patchset adds new config CONFIG_FILE_BUILD_ID option, which adds
> build id object pointer to the file object when enabled. The build id is
> read/populated when the file is mmap-ed.
> 
> I also added bpf and perf changes that would benefit from this.
> 
> I'm not sure what's the policy on adding stuff to file object, so apologies
> if that's out of line. I'm open to any feedback or suggestions if there's
> better place or way to do this.

hi,
Matthew suggested on irc to consider inode for storing build id

I tried that and it seems to have better stats wrt allocated build
id objects, because inode is being shared among file objects

I took /proc/slabinfo output after running bpf tests

- build id stored in file:

  # name            <active_objs> <num_objs> <objsize> <objperslab> <pagesperslab> : tunables <limit> <batchcount> <sharedfactor> : slabdata <active_slabs> <num_slabs> <sharedavail>
  build_id             668    775    160   25    1 : tunables    0    0    0 : slabdata     31     31      0

- build id stored in inode:

  # name            <active_objs> <num_objs> <objsize> <objperslab> <pagesperslab> : tunables <limit> <batchcount> <sharedfactor> : slabdata <active_slabs> <num_slabs> <sharedavail>
  build_id             222    225    160   25    1 : tunables    0    0    0 : slabdata      9      9      0


I'm stranger to inode/fs/mm code so I'll spend some time checking on
what I possibly broke in there before I send it, but I'd appreciate
any early feedback ;-)

the code is in here:
  git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
  inode_build_id

I'll send another version with inode if there's no objection

thanks,
jirka


> 
> thanks,
> jirka
> 
> 
> [1] https://lore.kernel.org/bpf/20221108222027.3409437-1-jolsa@kernel.org/
> [2] https://lore.kernel.org/bpf/20221128132915.141211-1-jolsa@kernel.org/
> [3] https://lore.kernel.org/bpf/CAEf4BzaZCUoxN_X2ALXwQeFTCwtL17R4P_B_-hUCcidfyO2xyQ@mail.gmail.com/
> ---
> Jiri Olsa (5):
>       mm: Store build id in file object
>       bpf: Use file object build id in stackmap
>       perf: Use file object build id in perf_event_mmap_event
>       selftests/bpf: Add file_build_id test
>       selftests/bpf: Add iter_task_vma_buildid test
> 
>  fs/file_table.c                                               |  3 +++
>  include/linux/buildid.h                                       | 17 +++++++++++++++++
>  include/linux/fs.h                                            |  3 +++
>  kernel/bpf/stackmap.c                                         |  8 ++++++++
>  kernel/events/core.c                                          | 43 +++++++++++++++++++++++++++++++++++++++----
>  lib/buildid.c                                                 | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  mm/Kconfig                                                    |  7 +++++++
>  mm/mmap.c                                                     | 15 +++++++++++++++
>  tools/testing/selftests/bpf/prog_tests/bpf_iter.c             | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tools/testing/selftests/bpf/prog_tests/file_build_id.c        | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tools/testing/selftests/bpf/progs/bpf_iter_task_vma_buildid.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  tools/testing/selftests/bpf/progs/file_build_id.c             | 34 ++++++++++++++++++++++++++++++++++
>  tools/testing/selftests/bpf/trace_helpers.c                   | 35 +++++++++++++++++++++++++++++++++++
>  tools/testing/selftests/bpf/trace_helpers.h                   |  1 +
>  14 files changed, 413 insertions(+), 4 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/file_build_id.c
>  create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_task_vma_buildid.c
>  create mode 100644 tools/testing/selftests/bpf/progs/file_build_id.c