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 93E24C27C48 for ; Sat, 28 Oct 2023 13:36:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229493AbjJ1NgP (ORCPT ); Sat, 28 Oct 2023 09:36:15 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57938 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229449AbjJ1NgN (ORCPT ); Sat, 28 Oct 2023 09:36:13 -0400 Received: from mail-lf1-x135.google.com (mail-lf1-x135.google.com [IPv6:2a00:1450:4864:20::135]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A89C6C0; Sat, 28 Oct 2023 06:36:10 -0700 (PDT) Received: by mail-lf1-x135.google.com with SMTP id 2adb3069b0e04-507bd644a96so4306385e87.3; Sat, 28 Oct 2023 06:36:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1698500169; x=1699104969; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:date:from:from:to :cc:subject:date:message-id:reply-to; bh=AHn8iBxAhpfsJ1tGpFmcP+H91hcjJA0+WNnhiH9LU48=; b=PgJ9+bWagXnWGzpjwDMfcEVaytBCj7iIdiLlLw7/hoKRzeZJCn4vOsfIchS/MobuVM SFvOF6KO73JyFPnAQ8nudaRp1F+fYHrrOMQEoLiZSXmqg2qnWhFvDAaV1nVdxIbQmP10 x2u6o1lysyL90am5/HNwq8uITdENS1G7oxa8eP4SbVkTpCPRT0zUJtKo3RM0Y7QOWg48 twp7mwZNmuGRCO4497CftP5FyIGVArhdWHJQ7I/a7sLFnN19ntOH8LZIu9EUJwk/zJ2c bmiG1qSRuTjqq/cbtoG6G0pRti2L+gP2XZPLFfsIvcvQ5Aym0RAlzHQFZp9aD8wPo2jk Cs6Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698500169; x=1699104969; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:date:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=AHn8iBxAhpfsJ1tGpFmcP+H91hcjJA0+WNnhiH9LU48=; b=B3euwU8e9HBiHPRiiYMVtds4EbpHKPZzlNs42+hV4+nGSR+ckSJtm0KML3pXvTy3Bk nBeVGcKWfqJ1I+r6IcRDKBTrIL/m84pviwZEgAlLcd3jgpvTVewSOg++kMs9RWUIQBKg ZqOFYumvSlAcFD2eMK10hnjXzfRcelpr/MiIHX71vSb+PPF94yMU8/7qC2El7gQmPP/a p6XA3Gy+WejnJZ8RAWYXoVxXmb2vOLNS9S3rETrFC66qxngRmsBjuQFvRHooj7HZiJaN Ay94FSIGqSXA6NmsYFE9rymu6rTbS4oNr03Chi3NoLdteaajwDOAfQXKPwI84xpNTgR7 rK4w== X-Gm-Message-State: AOJu0YwbsY1Q4tIKgX5eO2Gh8qEkvuHgYrD9dqsP6Qe23IbPo9yEEbfP S5iDPYpKAGmo21QJSwV6elg= X-Google-Smtp-Source: AGHT+IHmsoLQdHcXptq9dP2IARH0nCbjdWTuFJVGv/VB4z72AvQ1VBkpNms0j1/bnNmxE5+f6OnzvA== X-Received: by 2002:a05:6512:2525:b0:507:b7b7:e740 with SMTP id be37-20020a056512252500b00507b7b7e740mr6506658lfb.43.1698500168527; Sat, 28 Oct 2023 06:36:08 -0700 (PDT) Received: from krava (2001-1ae9-1c2-4c00-726e-c10f-8833-ff22.ip6.tmcz.cz. [2001:1ae9:1c2:4c00:726e:c10f:8833:ff22]) by smtp.gmail.com with ESMTPSA id t17-20020a50c251000000b0053eb69ca1bcsm2891517edf.92.2023.10.28.06.36.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 28 Oct 2023 06:36:08 -0700 (PDT) From: Jiri Olsa X-Google-Original-From: Jiri Olsa Date: Sat, 28 Oct 2023 15:36:06 +0200 To: Masahiro Yamada Cc: Andrii Nakryiko , Jiri Olsa , linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org, Nathan Chancellor , Nick Desaulniers , Nicolas Schier , bpf@vger.kernel.org Subject: Re: [bpf-next PATCH v2 4/4] kbuild: refactor module BTF rule Message-ID: References: <20231018151950.205265-4-masahiroy@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Oct 28, 2023 at 09:00:11PM +0900, Masahiro Yamada wrote: > On Mon, Oct 23, 2023 at 12:19 PM Andrii Nakryiko > wrote: > > > > On Sun, Oct 22, 2023 at 1:24 PM Masahiro Yamada wrote: > > > > > > On Sun, Oct 22, 2023 at 4:33 AM Andrii Nakryiko > > > wrote: > > > > > > > > On Sat, Oct 21, 2023 at 4:38 AM Masahiro Yamada wrote: > > > > > > > > > > On Sat, Oct 21, 2023 at 5:52 AM Andrii Nakryiko > > > > > wrote: > > > > > > > > > > > > On Fri, Oct 20, 2023 at 12:03 AM Masahiro Yamada wrote: > > > > > > > > > > > > > > On Fri, Oct 20, 2023 at 7:55 AM Andrii Nakryiko > > > > > > > wrote: > > > > > > > > > > > > > > > > On Thu, Oct 19, 2023 at 1:15 AM Jiri Olsa wrote: > > > > > > > > > > > > > > > > > > On Thu, Oct 19, 2023 at 12:19:50AM +0900, Masahiro Yamada wrote: > > > > > > > > > > newer_prereqs_except and if_changed_except are ugly hacks of the > > > > > > > > > > newer-prereqs and if_changed in scripts/Kbuild.include. > > > > > > > > > > > > > > > > > > > > Remove. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Masahiro Yamada > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > Changes in v2: > > > > > > > > > > - Fix if_changed_except to if_changed > > > > > > > > > > > > > > > > > > > > scripts/Makefile.modfinal | 25 ++++++------------------- > > > > > > > > > > 1 file changed, 6 insertions(+), 19 deletions(-) > > > > > > > > > > > > > > > > > > > > diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal > > > > > > > > > > index 9fd7a26e4fe9..fc07854bb7b9 100644 > > > > > > > > > > --- a/scripts/Makefile.modfinal > > > > > > > > > > +++ b/scripts/Makefile.modfinal > > > > > > > > > > @@ -19,6 +19,9 @@ vmlinux := > > > > > > > > > > ifdef CONFIG_DEBUG_INFO_BTF_MODULES > > > > > > > > > > ifneq ($(wildcard vmlinux),) > > > > > > > > > > vmlinux := vmlinux > > > > > > > > > > +cmd_btf = ; \ > > > > > > > > > > + LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ > > > > > > > > > > + $(RESOLVE_BTFIDS) -b vmlinux $@ > > > > > > > > > > else > > > > > > > > > > $(warning Skipping BTF generation due to unavailability of vmlinux) > > > > > > > > > > endif > > > > > > > > > > @@ -41,27 +44,11 @@ quiet_cmd_ld_ko_o = LD [M] $@ > > > > > > > > > > cmd_ld_ko_o += \ > > > > > > > > > > $(LD) -r $(KBUILD_LDFLAGS) \ > > > > > > > > > > $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE) \ > > > > > > > > > > - -T scripts/module.lds -o $@ $(filter %.o, $^) > > > > > > > > > > + -T scripts/module.lds -o $@ $(filter %.o, $^) \ > > > > > > > > > > + $(cmd_btf) > > > > > > > > > > > > > > > > > > > > -quiet_cmd_btf_ko = BTF [M] $@ > > > > > > > > > > > > > > > > > > nit not sure it's intentional but we no longer display 'BTF [M] ...ko' lines, > > > > > > > > > I don't mind not displaying that, but we should mention that in changelog > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for spotting this! I think those messages are useful and > > > > > > > > important to keep. Masahiro, is it possible to preserve them? > > > > > > > > > > > > > > > > > > > > > > > > > > > > No, I do not think so. > > > > > > > > > > > > > > > > > > > That's too bad, I think it's a useful one. > > > > > > > > > > > > > > > > > > > > I prioritize that the code is correct. > > > > > > > > > > > > > Could you please also prioritize not regressing informativeness of a > > > > build log? With your changes it's not clear now if BTF was generated > > > > or not for a kernel module, while previously it was obvious and was > > > > easy to spot if for some reason BTF was not generated. I'd like to > > > > preserve this > > > > property, thank you. > > > > > > > > E.g, can we still have BTF generation as a separate command and do a > > > > separate $(call if_changed,btf_ko)? Or something along those lines. > > > > Would that work? > > > > > > If we have an intermediate file (say, *.no-btf.ko), > > > it would make sense to have separate > > > $(call if_changed,ld_ko_o) and $(call if_changed,btf_ko). > > > > Currently we don't generate intermediate files, but we do rewrite > > original .ko file as a post-processing step. > > > > And that rewriting step might not happen depending on Kconfig and > > toolchain (e.g., too old pahole makes it impossible to generate kernel > > module BTF). And that's why having a separate BTF [M] message in the > > build log is important. > > > > > > > > > > > LD RESOLVE_BTFIDS > > > *.mod.o ------> *.no-btf.ko ------------> *.ko > > > > > > > > > When vmlinux is changed, only the second step would > > > be re-run, but that would require extra file copy. > > > > Today we rewrite .ko with a new .ko ELF file which gains a new ELF > > section (.BTF), so we already pay this price when BTF is enabled (if > > that's your concern). > > > > > > > > Is this what you want to see? > > > > I don't have strong preferences for exact implementation, but what you > > propose will work, I think. What I'd like to avoid is unnecessarily > > relinking .ko files if all we need to do is regenerate BTF. > > > > > Is there any way to make pahole/resolve_btfids > take separate input and output files > instead of in-place modification? for pahole I think it'd be possible to get object file with .BTF section and just link it with other module objects (it's done like that for vmlinux) but I'm not sure which module linking stage this could happen for resolve_btfids it's not possible at the moment, it just updates the .BTF_ids section in the object file I'm working on changing resolve_btfids to actually generate separate object with .BTF_ids section, which is then link-ed with the final object, but will take more time.. especially because I'm not sure where to place this logic in module linking ;-) jirka