From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 44E3FC761A6 for ; Fri, 31 Mar 2023 18:20:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231241AbjCaSUB (ORCPT ); Fri, 31 Mar 2023 14:20:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54424 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229675AbjCaSUA (ORCPT ); Fri, 31 Mar 2023 14:20:00 -0400 Received: from mail-ed1-x529.google.com (mail-ed1-x529.google.com [IPv6:2a00:1450:4864:20::529]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 039D449CD; Fri, 31 Mar 2023 11:19:59 -0700 (PDT) Received: by mail-ed1-x529.google.com with SMTP id ek18so93152982edb.6; Fri, 31 Mar 2023 11:19:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1680286797; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=ptCDsCZ9M1AP2auqMhwxD2e0Wct9w+HUGpQl1ZXGBuE=; b=Gw584kuBZB4NgIUzfMojlADjttI/sZS9NQ+lUHsKb0GpFWzrCiX5KxgPEM6YM3wfKb L7qyK+P8fd+4VG+CXmujXlxC7BXlewyQ8+k2Te9ebzk+rMzpahTWnav1kHCwZ0u4nDMK qSYzdA+/2PNTicJltJMZRFMjFud3XUTM6iTecSpNvUfmcC4NBDmoiTEsW6VNG4voh0Pl Dd3fimhzfdzOlXUg1Eycw8horsLfQpEwQdTlxhLhd1mTanssQvPjrFvkfEsYOGyq/O+I O9I5u9eAGOgFNajpeowt/EpJIH9cYLLO3sQlQulNKMqZZ9G9hP2k4+Rc4AmRLMzBFnll /QuQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680286797; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ptCDsCZ9M1AP2auqMhwxD2e0Wct9w+HUGpQl1ZXGBuE=; b=jCSBCXIgL9Z7jCOL+IrNjMh758tclOOq7YbFS2e0VQCIo9OgvfpMjyNFxbIdAZqOrH Aq2yG2FiAKm7dkV7N/lLYy3cg+7qH9W1mdIMuE2Yra8d/89yH9UgIKxAikL5VDNijpTd r4mTgPIFfyZKjX1ObirgAhmsTTToKGBb7IBnbY52kYgP02TTbbvi/IFn1MnzTvB2RNaj dFyDj8NpED4wnh3mz0AT/CyBYGmz+FXiF1UjfZ+I+zyoZVhK/DdusjohaXmuh6mvJ61P S6NwpDd1d7wPQbSi0uUvbw0IjJ+iXYxRHH7Ngl5bKd3FnvGHq6jw/fQmHgsc/uk1BsyY 6rsA== X-Gm-Message-State: AAQBX9dUGj5lfghSVmNDTzUVVushyj7o6Hwh/PU18TtK5KJJpPJl4rSS +cSQGFPXjsUJ92G7clq/5IWRBv9FGjU+QSosiFtoYRHR X-Google-Smtp-Source: AKy350Yj+IgRzl/cUwZ8ZsG3CzqlZxtGxVHdl50ppqglRAoEyoUXrbm8X7qBqemI62LpK7yLYFn/NsSgTus1uzv5ngw= X-Received: by 2002:a50:d694:0:b0:4fc:f0b8:7da0 with SMTP id r20-20020a50d694000000b004fcf0b87da0mr14424020edi.1.1680286797322; Fri, 31 Mar 2023 11:19:57 -0700 (PDT) MIME-Version: 1.0 References: <20230316170149.4106586-1-jolsa@kernel.org> In-Reply-To: From: Andrii Nakryiko Date: Fri, 31 Mar 2023 11:19:45 -0700 Message-ID: Subject: Re: [PATCHv3 bpf-next 0/9] mm/bpf/perf: Store build id in file object To: Arnaldo Carvalho de Melo Cc: Matthew Wilcox , Jiri Olsa , Alexei Starovoitov , Andrii Nakryiko , Hao Luo , Andrew Morton , Alexander Viro , Peter Zijlstra , Ingo Molnar , 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 , Song Liu , Yonghong Song , John Fastabend , KP Singh , Stanislav Fomichev , Daniel Borkmann , Namhyung Kim , Dave Chinner Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-perf-users@vger.kernel.org On Wed, Mar 22, 2023 at 8:45=E2=80=AFAM Arnaldo Carvalho de Melo wrote: > > Em Sat, Mar 18, 2023 at 03:16:45PM +0000, Matthew Wilcox escreveu: > > On Sat, Mar 18, 2023 at 09:33:49AM +0100, Jiri Olsa wrote: > > > On Thu, Mar 16, 2023 at 05:34:41PM +0000, Matthew Wilcox wrote: > > > > On Thu, Mar 16, 2023 at 06:01:40PM +0100, Jiri Olsa wrote: > > > > > hi, > > > > > this patchset adds build id object pointer to struct file object. > > > > > > > > > > We have several use cases for build id to be used in BPF programs > > > > > [2][3]. > > > > > > > > Yes, you have use cases, but you never answered the question I aske= d: > > > > > > > > Is this going to be enabled by every distro kernel, or is it for sp= ecial > > > > use-cases where only people doing a very specialised thing who are > > > > willing to build their own kernels will use it? > > > > > > I hope so, but I guess only time tell.. given the response by Ian and= Andrii > > > there are 3 big users already > > > > So the whole "There's a config option to turn it off" shtick is just a > > fig-leaf. I won't ever see it turned off. You're imposing the cost of > > this on EVERYONE who runs a distro kernel. And almost nobody will see > > any benefits from it. Thanks for admitting that. > > I agree that build-ids are not useful for all 'struct file' uses, just > for executable files and for people wanting to have better observability > capabilities. > > Having said that, it seems there will be no extra memory overhead at > least for a fedora:36 x86_64 kernel: > > void __init files_init(void) > { > filp_cachep =3D kmem_cache_create("filp", sizeof(struct file), 0, > SLAB_HWCACHE_ALIGN | SLAB_PANIC | SLAB_ACCOUNT, N= ULL); > percpu_counter_init(&nr_files, 0, GFP_KERNEL); > } > > [root@quaco ~]# pahole file | grep size: -A2 > /* size: 232, cachelines: 4, members: 20 */ > /* sum members: 228, holes: 1, sum holes: 4 */ > /* last cacheline: 40 bytes */ > [acme@quaco perf-tools]$ uname -a > Linux quaco 6.1.11-100.fc36.x86_64 #1 SMP PREEMPT_DYNAMIC Thu Feb 9 20:3= 6:30 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux > [root@quaco ~]# head -2 /proc/slabinfo > slabinfo - version: 2.1 > # name : tunables : slabdata > [root@quaco ~]# grep -w filp /proc/slabinfo > filp 12452 13056 256 32 2 : tunables 0 0 = 0 : slabdata 408 408 0 > [root@quaco ~]# > > so there are 24 bytes on the 4th cacheline that are not being used, > right? Well, even better then! > > One other observation is that maybe we could do it as the 'struct sock' > hierachy in networking, where we would have a 'struct exec_file' that > would be: > > struct exec_file { > struct file file; > char build_id[20]; > } > > say, and then when we create the 'struct file' in __alloc_file() we > could check some bit in 'flags' like Al Viro suggested and pick a > different slab than 'filp_cachep', that has that extra space for the > build_id (and whatever else exec related state we may end up wanting, if > ever). > > No core fs will need to know about that except when we go free it, to > free from the right slab cache. > > In current distro configs, no overhead would take place if I read that > SLAB_HWCACHE_ALIGN thing right, no? Makes sense to me as well. Whatever the solution, as long as it's usable from NMI contexts would be fine for the purposes of fetching build ID. It would be good to hear from folks that are opposing adding a pointer field to struct file whether they prefer this way instead? > > - Arnaldo