Message ID | 20231018151950.205265-4-masahiroy@kernel.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:2908:b0:403:3b70:6f57 with SMTP id ib8csp4867591vqb; Wed, 18 Oct 2023 08:20:54 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGOC6psec90fncxO4eWrQdoTN8dM8izCHqO626HksYSA6TRh2LDg5FAyu4PPtWnA9o54HAN X-Received: by 2002:a17:90b:4f88:b0:274:99ed:a80c with SMTP id qe8-20020a17090b4f8800b0027499eda80cmr5960869pjb.3.1697642453986; Wed, 18 Oct 2023 08:20:53 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1697642453; cv=none; d=google.com; s=arc-20160816; b=TkAAmGqhQ2LlJoggaWE67TM3uLY0syR83bHdcGDHhsxF8CScv6B/QUmIB4TLxB3NqK yCS0aBULsHiz/ehA0oEDc1a7cghFTEvFyf0myqkXs3jHE1GjJ2M0j6N7mId9UqC9f1ke cne4soR+XsLrp7h5DuQbyG+76zdS9Vdwmgy0i9tO5NtL7vnuXqm7NkbrNRN5UoxJiOAR 8aesA/1MahvHBoxBHUZm4R+CdLi2oAMMNjg3YIQsWqo6aNjpkoL5IGeqTONSGQ+Ec5yS j1yhkAbu/CNfRbSKPoIGCPG5nU+ARQkJwnnBBFDPDmBgpjWlKkOZOzWAle9Bz8T9JKn2 E2uw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=5kZdTBGuRP0qDnOA7mYxbW355fu9Amod16b3ktNpHFM=; fh=nY9BXmM6OvqPxK+j1qrAlSrJerCssKDI8ELrZ5Y0WGQ=; b=b1BEpAC2+u+r6UAsrQwhzyuef9CiQNuKsuYgiW/QKtJN+oqnbktHg8JSHWxhzes4AO 9fqMC84WAyB86HNxFgE/t3r+aOjcwyJx9rAH3Er/lRsmmOfr+sE4e+Q6AFsS4dcx4aoX ek/8lNnaM9mj088ITKw1+0ckKICLMTE4R5Pw0uxmTkttnV9pLUjdVcgUDuugJPu4Xn+V gYV214O1ci59axTQabz/Nn03tR/kRyQuqAd17ThB7fnraz6+6mPbtk4LRz0ZZChoCG4A BzzHOZFV6A4EXNl2lL4FumUziN30yQXSo9Tnvx7lcTo07IRPjkuCGdZmwQr3PmiH7VUG nmjg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=fTIyxfwr; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from agentk.vger.email (agentk.vger.email. [23.128.96.32]) by mx.google.com with ESMTPS id gj19-20020a17090b109300b0027d0c811520si72913pjb.95.2023.10.18.08.20.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 Oct 2023 08:20:53 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=fTIyxfwr; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by agentk.vger.email (Postfix) with ESMTP id 0B42080DBAFC; Wed, 18 Oct 2023 08:20:46 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at agentk.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1344783AbjJRPUW (ORCPT <rfc822;zwp10758@gmail.com> + 23 others); Wed, 18 Oct 2023 11:20:22 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59824 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231960AbjJRPUO (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 18 Oct 2023 11:20:14 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BAFF8114; Wed, 18 Oct 2023 08:20:12 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id ED347C433CA; Wed, 18 Oct 2023 15:20:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1697642412; bh=64NbOJwL+HDqWyuM6JoXFyBcAlHXP3zz+QUuLv6Cm7Q=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=fTIyxfwrpTY5bgbva8rPbkcGlgqoZai0aO3PkJiKLsjwlaQN26q9Yhf7zRd2wjkRW WZduFe/lFbU/q+9u4epT/mVv1LqRuauO0bz5OWF23Kd8JMxngIYbgEwwc+f4kmivhh VJTyepbpLsgfF5FtZVGl8dMHPuvoQ8QD6No7cMYjk689j0+PdrMxAx2gcLvk6LY2TW Ety129FzUIr6FfwD15PVqI3wl9DSL9mvUBfMHnmNmKqGI6SnKNxfdm/eFbapyE1BFX gpiFW89j2Z6bS6GGInAnAKbVxMu33QTPYYdJA91bLmvH3+wqGmgTWgilv6JfdU0Nwc KdvBwAIq+VCYg== From: Masahiro Yamada <masahiroy@kernel.org> To: linux-kbuild@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Masahiro Yamada <masahiroy@kernel.org>, Nathan Chancellor <nathan@kernel.org>, Nick Desaulniers <ndesaulniers@google.com>, Nicolas Schier <nicolas@fjasle.eu>, bpf@vger.kernel.org Subject: [bpf-next PATCH v2 4/4] kbuild: refactor module BTF rule Date: Thu, 19 Oct 2023 00:19:50 +0900 Message-Id: <20231018151950.205265-4-masahiroy@kernel.org> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20231018151950.205265-1-masahiroy@kernel.org> References: <20231018151950.205265-1-masahiroy@kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-1.2 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Wed, 18 Oct 2023 08:20:46 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1780107133740165914 X-GMAIL-MSGID: 1780107133740165914 |
Series | [bpf-next,v2,1/4] kbuild: remove ARCH_POSTLINK from module builds | |
Commit Message
Masahiro Yamada
Oct. 18, 2023, 3:19 p.m. UTC
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 <masahiroy@kernel.org>
---
Changes in v2:
- Fix if_changed_except to if_changed
scripts/Makefile.modfinal | 25 ++++++-------------------
1 file changed, 6 insertions(+), 19 deletions(-)
Comments
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 <masahiroy@kernel.org> > --- > > Changes in v2: > - Fix if_changed_except to if_changed thanks Reviewed-by: Nicolas Schier <nicolas@fjasle.eu> > 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] $@ > - cmd_btf_ko = \ > - LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ > - $(RESOLVE_BTFIDS) -b vmlinux $@ > - > -# Same as newer-prereqs, but allows to exclude specified extra dependencies > -newer_prereqs_except = $(filter-out $(PHONY) $(1),$?) > - > -# Same as if_changed, but allows to exclude specified extra dependencies > -if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check), \ > - $(cmd); \ > - printf '%s\n' 'savedcmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:) > - > -# Re-generate module BTFs if either module's .ko or vmlinux changed > %.ko: %.o %.mod.o scripts/module.lds $(vmlinux) FORCE > - +$(call if_changed_except,ld_ko_o,vmlinux) > -ifdef vmlinux > - +$(if $(newer-prereqs),$(call cmd,btf_ko)) > -endif > + +$(call if_changed,ld_ko_o) > > targets += $(modules:%.o=%.ko) $(modules:%.o=%.mod.o) > > -- > 2.40.1
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 <masahiroy@kernel.org> > --- > > 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 jirka > - cmd_btf_ko = \ > - LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ > - $(RESOLVE_BTFIDS) -b vmlinux $@ > - > -# Same as newer-prereqs, but allows to exclude specified extra dependencies > -newer_prereqs_except = $(filter-out $(PHONY) $(1),$?) > - > -# Same as if_changed, but allows to exclude specified extra dependencies > -if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check), \ > - $(cmd); \ > - printf '%s\n' 'savedcmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:) > - > -# Re-generate module BTFs if either module's .ko or vmlinux changed > %.ko: %.o %.mod.o scripts/module.lds $(vmlinux) FORCE > - +$(call if_changed_except,ld_ko_o,vmlinux) > -ifdef vmlinux > - +$(if $(newer-prereqs),$(call cmd,btf_ko)) > -endif > + +$(call if_changed,ld_ko_o) > > targets += $(modules:%.o=%.ko) $(modules:%.o=%.mod.o) > > -- > 2.40.1 > >
On Thu, Oct 19, 2023 at 1:15 AM Jiri Olsa <olsajiri@gmail.com> 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 <masahiroy@kernel.org> > > --- > > > > 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? > jirka > > > - cmd_btf_ko = \ > > - LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ > > - $(RESOLVE_BTFIDS) -b vmlinux $@ > > - > > -# Same as newer-prereqs, but allows to exclude specified extra dependencies > > -newer_prereqs_except = $(filter-out $(PHONY) $(1),$?) > > - > > -# Same as if_changed, but allows to exclude specified extra dependencies > > -if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check), \ > > - $(cmd); \ > > - printf '%s\n' 'savedcmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:) > > - > > -# Re-generate module BTFs if either module's .ko or vmlinux changed > > %.ko: %.o %.mod.o scripts/module.lds $(vmlinux) FORCE > > - +$(call if_changed_except,ld_ko_o,vmlinux) > > -ifdef vmlinux > > - +$(if $(newer-prereqs),$(call cmd,btf_ko)) > > -endif > > + +$(call if_changed,ld_ko_o) > > > > targets += $(modules:%.o=%.ko) $(modules:%.o=%.mod.o) > > > > -- > > 2.40.1 > > > > >
On Fri, Oct 20, 2023 at 7:55 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Thu, Oct 19, 2023 at 1:15 AM Jiri Olsa <olsajiri@gmail.com> 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 <masahiroy@kernel.org> > > > --- > > > > > > 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. Your code is wrong. To clarify this is a fix, I will replace the commit as follows: ------------------->8---------------------- kbuild: detect btf command change for modules Currently, the command change in cmd_btf_ko does not cause to rebuild the modules because it is not passed to if_changed. Pass everything to if_change so that the btf command is also recorded in the .*.cmd files. This removes the hacky newer_prereqs_except and if_changed_except macros too. ------------------->8---------------------- -- Best Regards Masahiro Yamada
On Fri, Oct 20, 2023 at 12:03 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Fri, Oct 20, 2023 at 7:55 AM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Thu, Oct 19, 2023 at 1:15 AM Jiri Olsa <olsajiri@gmail.com> 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 <masahiroy@kernel.org> > > > > --- > > > > > > > > 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. > Your code is wrong. > Could be, but note the comment you are removing: # Re-generate module BTFs if either module's .ko or vmlinux changed BTF has to be re-generated not just when module .ko is regenerated, but also when the vmlinux image itself changes. I don't see where this is done with your changes. Can you please point it out explicitly? > > To clarify this is a fix, > I will replace the commit as follows: > > > > > ------------------->8---------------------- > kbuild: detect btf command change for modules > > Currently, the command change in cmd_btf_ko does not cause to rebuild > the modules because it is not passed to if_changed. > > Pass everything to if_change so that the btf command is also recorded > in the .*.cmd files. This removes the hacky newer_prereqs_except and > if_changed_except macros too. > ------------------->8---------------------- > > > > > -- > Best Regards > > Masahiro Yamada
On Sat, Oct 21, 2023 at 5:52 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Fri, Oct 20, 2023 at 12:03 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Fri, Oct 20, 2023 at 7:55 AM Andrii Nakryiko > > <andrii.nakryiko@gmail.com> wrote: > > > > > > On Thu, Oct 19, 2023 at 1:15 AM Jiri Olsa <olsajiri@gmail.com> 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 <masahiroy@kernel.org> > > > > > --- > > > > > > > > > > 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. > > > Your code is wrong. > > > > Could be, but note the comment you are removing: > > # Re-generate module BTFs if either module's .ko or vmlinux changed > > BTF has to be re-generated not just when module .ko is regenerated, > but also when the vmlinux image itself changes. > > I don't see where this is done with your changes. Can you please point > it out explicitly? That is too obvious; %.ko depends on $(vmlinux). %.ko: %.o %.mod.o scripts/module.lds $(vmlinux) FORCE
On Sat, Oct 21, 2023 at 4:38 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Sat, Oct 21, 2023 at 5:52 AM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Fri, Oct 20, 2023 at 12:03 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > On Fri, Oct 20, 2023 at 7:55 AM Andrii Nakryiko > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > On Thu, Oct 19, 2023 at 1:15 AM Jiri Olsa <olsajiri@gmail.com> 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 <masahiroy@kernel.org> > > > > > > --- > > > > > > > > > > > > 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? > > > > > > > Your code is wrong. > > > > > > > Could be, but note the comment you are removing: > > > > # Re-generate module BTFs if either module's .ko or vmlinux changed > > > > BTF has to be re-generated not just when module .ko is regenerated, > > but also when the vmlinux image itself changes. > > > > I don't see where this is done with your changes. Can you please point > > it out explicitly? > > > > That is too obvious; %.ko depends on $(vmlinux). Thank you for your gracious answer. We used to not rebuild module's .ko's when vmlinux didn't change (but we did regen BTFs), and that's why I was confused. Now we forcefully recompile modules, which is a change in behavior which would be nice to call out in the commit message. > > > > %.ko: %.o %.mod.o scripts/module.lds $(vmlinux) FORCE > > > > > -- > Best Regards > Masahiro Yamada
On Sun, Oct 22, 2023 at 4:33 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Sat, Oct 21, 2023 at 4:38 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Sat, Oct 21, 2023 at 5:52 AM Andrii Nakryiko > > <andrii.nakryiko@gmail.com> wrote: > > > > > > On Fri, Oct 20, 2023 at 12:03 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > On Fri, Oct 20, 2023 at 7:55 AM Andrii Nakryiko > > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > > > On Thu, Oct 19, 2023 at 1:15 AM Jiri Olsa <olsajiri@gmail.com> 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 <masahiroy@kernel.org> > > > > > > > --- > > > > > > > > > > > > > > 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). 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. Is this what you want to see? > > > > > > > > > > > > Your code is wrong. > > > > > > > > > > Could be, but note the comment you are removing: > > > > > > # Re-generate module BTFs if either module's .ko or vmlinux changed > > > > > > BTF has to be re-generated not just when module .ko is regenerated, > > > but also when the vmlinux image itself changes. > > > > > > I don't see where this is done with your changes. Can you please point > > > it out explicitly? > > > > > > > > That is too obvious; %.ko depends on $(vmlinux). > > Thank you for your gracious answer. We used to not rebuild module's > .ko's when vmlinux didn't change (but we did regen BTFs), and that's > why I was confused. Now we forcefully recompile modules, which is a > change in behavior which would be nice to call out in the commit > message. > > > > > > > > > > %.ko: %.o %.mod.o scripts/module.lds $(vmlinux) FORCE > > > > > > > > > > -- > > Best Regards > > Masahiro Yamada
On Sun, Oct 22, 2023 at 1:24 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Sun, Oct 22, 2023 at 4:33 AM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Sat, Oct 21, 2023 at 4:38 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > On Sat, Oct 21, 2023 at 5:52 AM Andrii Nakryiko > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > On Fri, Oct 20, 2023 at 12:03 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > > On Fri, Oct 20, 2023 at 7:55 AM Andrii Nakryiko > > > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > > > > > On Thu, Oct 19, 2023 at 1:15 AM Jiri Olsa <olsajiri@gmail.com> 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 <masahiroy@kernel.org> > > > > > > > > --- > > > > > > > > > > > > > > > > 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. > > > > > > > > > > > > > > > > > > > > > > Your code is wrong. > > > > > > > > > > > > > Could be, but note the comment you are removing: > > > > > > > > # Re-generate module BTFs if either module's .ko or vmlinux changed > > > > > > > > BTF has to be re-generated not just when module .ko is regenerated, > > > > but also when the vmlinux image itself changes. > > > > > > > > I don't see where this is done with your changes. Can you please point > > > > it out explicitly? > > > > > > > > > > > > That is too obvious; %.ko depends on $(vmlinux). > > > > Thank you for your gracious answer. We used to not rebuild module's > > .ko's when vmlinux didn't change (but we did regen BTFs), and that's > > why I was confused. Now we forcefully recompile modules, which is a > > change in behavior which would be nice to call out in the commit > > message. > > > > > > > > > > > > > > > > %.ko: %.o %.mod.o scripts/module.lds $(vmlinux) FORCE > > > > > > > > > > > > > > > -- > > > Best Regards > > > Masahiro Yamada > > > > -- > Best Regards > Masahiro Yamada
On Mon, Oct 23, 2023 at 12:19 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Sun, Oct 22, 2023 at 1:24 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Sun, Oct 22, 2023 at 4:33 AM Andrii Nakryiko > > <andrii.nakryiko@gmail.com> wrote: > > > > > > On Sat, Oct 21, 2023 at 4:38 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > On Sat, Oct 21, 2023 at 5:52 AM Andrii Nakryiko > > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > > > On Fri, Oct 20, 2023 at 12:03 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > > > > On Fri, Oct 20, 2023 at 7:55 AM Andrii Nakryiko > > > > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > > > > > > > On Thu, Oct 19, 2023 at 1:15 AM Jiri Olsa <olsajiri@gmail.com> 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 <masahiroy@kernel.org> > > > > > > > > > --- > > > > > > > > > > > > > > > > > > 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? Otherwise, explicit "cp *.no-btf.ko *.ko" would be needed. -- Best Regards Masahiro Yamada
On Sat, Oct 28, 2023 at 09:00:11PM +0900, Masahiro Yamada wrote: > On Mon, Oct 23, 2023 at 12:19 PM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Sun, Oct 22, 2023 at 1:24 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > On Sun, Oct 22, 2023 at 4:33 AM Andrii Nakryiko > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > On Sat, Oct 21, 2023 at 4:38 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > > On Sat, Oct 21, 2023 at 5:52 AM Andrii Nakryiko > > > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > > > > > On Fri, Oct 20, 2023 at 12:03 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > > > > > > On Fri, Oct 20, 2023 at 7:55 AM Andrii Nakryiko > > > > > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > > > > > > > > > On Thu, Oct 19, 2023 at 1:15 AM Jiri Olsa <olsajiri@gmail.com> 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 <masahiroy@kernel.org> > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > 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
On Sat, Oct 28, 2023 at 6:36 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > On Sat, Oct 28, 2023 at 09:00:11PM +0900, Masahiro Yamada wrote: > > On Mon, Oct 23, 2023 at 12:19 PM Andrii Nakryiko > > <andrii.nakryiko@gmail.com> wrote: > > > > > > On Sun, Oct 22, 2023 at 1:24 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > On Sun, Oct 22, 2023 at 4:33 AM Andrii Nakryiko > > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > > > On Sat, Oct 21, 2023 at 4:38 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > > > > On Sat, Oct 21, 2023 at 5:52 AM Andrii Nakryiko > > > > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > > > > > > > On Fri, Oct 20, 2023 at 12:03 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > > > > > > > > On Fri, Oct 20, 2023 at 7:55 AM Andrii Nakryiko > > > > > > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > > > > > > > > > > > On Thu, Oct 19, 2023 at 1:15 AM Jiri Olsa <olsajiri@gmail.com> 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 <masahiroy@kernel.org> > > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > > > 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 ;-) pahole also supports mode of generating BTF into a separate file without modifying the original one. The option is called --btf_encode_detached. It was added in v1.22 (currently the minimal version is v1.16), though, so depending on whether we are willing to bump the minimum pahole version, we might use that. That will allow us to also simplify and clean up link-vmlinux.sh a bit, I think. But I don't know if it's worth the trouble right now. > > jirka
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] $@ - cmd_btf_ko = \ - LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ - $(RESOLVE_BTFIDS) -b vmlinux $@ - -# Same as newer-prereqs, but allows to exclude specified extra dependencies -newer_prereqs_except = $(filter-out $(PHONY) $(1),$?) - -# Same as if_changed, but allows to exclude specified extra dependencies -if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check), \ - $(cmd); \ - printf '%s\n' 'savedcmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:) - -# Re-generate module BTFs if either module's .ko or vmlinux changed %.ko: %.o %.mod.o scripts/module.lds $(vmlinux) FORCE - +$(call if_changed_except,ld_ko_o,vmlinux) -ifdef vmlinux - +$(if $(newer-prereqs),$(call cmd,btf_ko)) -endif + +$(call if_changed,ld_ko_o) targets += $(modules:%.o=%.ko) $(modules:%.o=%.mod.o)