| Message ID | 20230407070517.204676-1-andrea.righi@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | scripts: rust: drop is_rust_module.sh | expand |
On Fri, Apr 07, 2023 at 09:05:17AM +0200, Andrea Righi wrote: > Commit c1177979af9c ("btf, scripts: Exclude Rust CUs with pahole") > introduced the constraint "!DEBUG_INFO_BTF || PAHOLE_HAS_LANG_EXCLUDE" > to enable RUST. > > With this constraint we don't need is_rust_module.sh anymore, because > 'pahole --lang_exclude=rust' already has the capability to exclude Rust > CUs. If pahole isn't recent enough (< 1.24) to support --lang_exclude, > then DEBUG_INFO_BTF can't be enabled with RUST and is_rust_module.sh > isn't used as well. > > In any case is_rust_module.sh is obsolete and we can just drop it. > > Link: https://lore.kernel.org/lkml/Y+p2xKIN6TJnQinK@righiandr-XPS-13-7390/ > Signed-off-by: Andrea Righi <andrea.righi@canonical.com> Reviewed-by: Nathan Chancellor <nathan@kernel.org> > --- > rust/macros/module.rs | 2 +- > scripts/Makefile.modfinal | 2 -- > scripts/is_rust_module.sh | 16 ---------------- > 3 files changed, 1 insertion(+), 19 deletions(-) > delete mode 100755 scripts/is_rust_module.sh > > diff --git a/rust/macros/module.rs b/rust/macros/module.rs > index a7e363c2b044..608406f33a70 100644 > --- a/rust/macros/module.rs > +++ b/rust/macros/module.rs > @@ -179,7 +179,7 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream { > /// Used by the printing macros, e.g. [`info!`]. > const __LOG_PREFIX: &[u8] = b\"{name}\\0\"; > > - /// The \"Rust loadable module\" mark, for `scripts/is_rust_module.sh`. > + /// The \"Rust loadable module\" mark. > // > // This may be best done another way later on, e.g. as a new modinfo > // key or a new section. For the moment, keep it simple. It seems like this whole section can be dropped? I only see __IS_RUST_MODULE used in scripts/is_rust_module.sh, as the comment states. > diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal x> index 4703f652c009..e6f8fa3c9b8d 100644 > --- a/scripts/Makefile.modfinal > +++ b/scripts/Makefile.modfinal > @@ -41,8 +41,6 @@ quiet_cmd_btf_ko = BTF [M] $@ > cmd_btf_ko = \ > if [ ! -f vmlinux ]; then \ > printf "Skipping BTF generation for %s due to unavailability of vmlinux\n" $@ 1>&2; \ > - elif [ -n "$(CONFIG_RUST)" ] && $(srctree)/scripts/is_rust_module.sh $@; then \ > - printf "Skipping BTF generation for %s because it's a Rust module\n" $@ 1>&2; \ > else \ > LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ > $(RESOLVE_BTFIDS) -b vmlinux $@; \ > diff --git a/scripts/is_rust_module.sh b/scripts/is_rust_module.sh > deleted file mode 100755 > index 28b3831a7593..000000000000 > --- a/scripts/is_rust_module.sh > +++ /dev/null > @@ -1,16 +0,0 @@ > -#!/bin/sh > -# SPDX-License-Identifier: GPL-2.0 > -# > -# is_rust_module.sh module.ko > -# > -# Returns `0` if `module.ko` is a Rust module, `1` otherwise. > - > -set -e > - > -# Using the `16_` prefix ensures other symbols with the same substring > -# are not picked up (even if it would be unlikely). The last part is > -# used just in case LLVM decides to use the `.` suffix. > -# > -# In the future, checking for the `.comment` section may be another > -# option, see https://github.com/rust-lang/rust/pull/97550. > -${NM} "$*" | grep -qE '^[0-9a-fA-F]+ r _R[^[:space:]]+16___IS_RUST_MODULE[^[:space:]]*$' > -- > 2.39.2 >
On Fri, Apr 07, 2023 at 10:53:18AM -0700, Nathan Chancellor wrote: > On Fri, Apr 07, 2023 at 09:05:17AM +0200, Andrea Righi wrote: > > Commit c1177979af9c ("btf, scripts: Exclude Rust CUs with pahole") > > introduced the constraint "!DEBUG_INFO_BTF || PAHOLE_HAS_LANG_EXCLUDE" > > to enable RUST. > > > > With this constraint we don't need is_rust_module.sh anymore, because > > 'pahole --lang_exclude=rust' already has the capability to exclude Rust > > CUs. If pahole isn't recent enough (< 1.24) to support --lang_exclude, > > then DEBUG_INFO_BTF can't be enabled with RUST and is_rust_module.sh > > isn't used as well. > > > > In any case is_rust_module.sh is obsolete and we can just drop it. > > > > Link: https://lore.kernel.org/lkml/Y+p2xKIN6TJnQinK@righiandr-XPS-13-7390/ > > Signed-off-by: Andrea Righi <andrea.righi@canonical.com> > > Reviewed-by: Nathan Chancellor <nathan@kernel.org> > > > --- > > rust/macros/module.rs | 2 +- > > scripts/Makefile.modfinal | 2 -- > > scripts/is_rust_module.sh | 16 ---------------- > > 3 files changed, 1 insertion(+), 19 deletions(-) > > delete mode 100755 scripts/is_rust_module.sh > > > > diff --git a/rust/macros/module.rs b/rust/macros/module.rs > > index a7e363c2b044..608406f33a70 100644 > > --- a/rust/macros/module.rs > > +++ b/rust/macros/module.rs > > @@ -179,7 +179,7 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream { > > /// Used by the printing macros, e.g. [`info!`]. > > const __LOG_PREFIX: &[u8] = b\"{name}\\0\"; > > > > - /// The \"Rust loadable module\" mark, for `scripts/is_rust_module.sh`. > > + /// The \"Rust loadable module\" mark. > > // > > // This may be best done another way later on, e.g. as a new modinfo > > // key or a new section. For the moment, keep it simple. > > It seems like this whole section can be dropped? I only see > __IS_RUST_MODULE used in scripts/is_rust_module.sh, as the comment > states. In the scope of the kernel, yes, only is_rust_module.sh used that, but outside the kernel "something" may still rely on this section. Keeping it seems safer for now, to avoid potential user-space breakage. Thanks, -Andrea
On Fri, 7 Apr 2023 at 08:05, Andrea Righi <andrea.righi@canonical.com> wrote: > > Commit c1177979af9c ("btf, scripts: Exclude Rust CUs with pahole") > introduced the constraint "!DEBUG_INFO_BTF || PAHOLE_HAS_LANG_EXCLUDE" > to enable RUST. > > With this constraint we don't need is_rust_module.sh anymore, because > 'pahole --lang_exclude=rust' already has the capability to exclude Rust > CUs. If pahole isn't recent enough (< 1.24) to support --lang_exclude, > then DEBUG_INFO_BTF can't be enabled with RUST and is_rust_module.sh > isn't used as well. > > In any case is_rust_module.sh is obsolete and we can just drop it. > > Link: https://lore.kernel.org/lkml/Y+p2xKIN6TJnQinK@righiandr-XPS-13-7390/ > Signed-off-by: Andrea Righi <andrea.righi@canonical.com> Ran this in an Asahi Fedora build to be sure. Tested-by: Eric Curtin <ecurtin@redhat.com> Reviewed-by: Eric Curtin <ecurtin@redhat.com> Is mise le meas/Regards, Eric Curtin > --- > rust/macros/module.rs | 2 +- > scripts/Makefile.modfinal | 2 -- > scripts/is_rust_module.sh | 16 ---------------- > 3 files changed, 1 insertion(+), 19 deletions(-) > delete mode 100755 scripts/is_rust_module.sh > > diff --git a/rust/macros/module.rs b/rust/macros/module.rs > index a7e363c2b044..608406f33a70 100644 > --- a/rust/macros/module.rs > +++ b/rust/macros/module.rs > @@ -179,7 +179,7 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream { > /// Used by the printing macros, e.g. [`info!`]. > const __LOG_PREFIX: &[u8] = b\"{name}\\0\"; > > - /// The \"Rust loadable module\" mark, for `scripts/is_rust_module.sh`. > + /// The \"Rust loadable module\" mark. > // > // This may be best done another way later on, e.g. as a new modinfo > // key or a new section. For the moment, keep it simple. > diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal > index 4703f652c009..e6f8fa3c9b8d 100644 > --- a/scripts/Makefile.modfinal > +++ b/scripts/Makefile.modfinal > @@ -41,8 +41,6 @@ quiet_cmd_btf_ko = BTF [M] $@ > cmd_btf_ko = \ > if [ ! -f vmlinux ]; then \ > printf "Skipping BTF generation for %s due to unavailability of vmlinux\n" $@ 1>&2; \ > - elif [ -n "$(CONFIG_RUST)" ] && $(srctree)/scripts/is_rust_module.sh $@; then \ > - printf "Skipping BTF generation for %s because it's a Rust module\n" $@ 1>&2; \ > else \ > LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ > $(RESOLVE_BTFIDS) -b vmlinux $@; \ > diff --git a/scripts/is_rust_module.sh b/scripts/is_rust_module.sh > deleted file mode 100755 > index 28b3831a7593..000000000000 > --- a/scripts/is_rust_module.sh > +++ /dev/null > @@ -1,16 +0,0 @@ > -#!/bin/sh > -# SPDX-License-Identifier: GPL-2.0 > -# > -# is_rust_module.sh module.ko > -# > -# Returns `0` if `module.ko` is a Rust module, `1` otherwise. > - > -set -e > - > -# Using the `16_` prefix ensures other symbols with the same substring > -# are not picked up (even if it would be unlikely). The last part is > -# used just in case LLVM decides to use the `.` suffix. > -# > -# In the future, checking for the `.comment` section may be another > -# option, see https://github.com/rust-lang/rust/pull/97550. > -${NM} "$*" | grep -qE '^[0-9a-fA-F]+ r _R[^[:space:]]+16___IS_RUST_MODULE[^[:space:]]*$' > -- > 2.39.2 >
On Fri, Apr 7, 2023 at 3:05 AM Andrea Righi <andrea.righi@canonical.com> wrote: > > Commit c1177979af9c ("btf, scripts: Exclude Rust CUs with pahole") > introduced the constraint "!DEBUG_INFO_BTF || PAHOLE_HAS_LANG_EXCLUDE" > to enable RUST. > > With this constraint we don't need is_rust_module.sh anymore, because > 'pahole --lang_exclude=rust' already has the capability to exclude Rust > CUs. If pahole isn't recent enough (< 1.24) to support --lang_exclude, > then DEBUG_INFO_BTF can't be enabled with RUST and is_rust_module.sh > isn't used as well. > > In any case is_rust_module.sh is obsolete and we can just drop it. > > Link: https://lore.kernel.org/lkml/Y+p2xKIN6TJnQinK@righiandr-XPS-13-7390/ > Signed-off-by: Andrea Righi <andrea.righi@canonical.com> > --- > rust/macros/module.rs | 2 +- > scripts/Makefile.modfinal | 2 -- > scripts/is_rust_module.sh | 16 ---------------- > 3 files changed, 1 insertion(+), 19 deletions(-) > delete mode 100755 scripts/is_rust_module.sh > > diff --git a/rust/macros/module.rs b/rust/macros/module.rs > index a7e363c2b044..608406f33a70 100644 > --- a/rust/macros/module.rs > +++ b/rust/macros/module.rs > @@ -179,7 +179,7 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream { > /// Used by the printing macros, e.g. [`info!`]. > const __LOG_PREFIX: &[u8] = b\"{name}\\0\"; > > - /// The \"Rust loadable module\" mark, for `scripts/is_rust_module.sh`. > + /// The \"Rust loadable module\" mark. > // > // This may be best done another way later on, e.g. as a new modinfo > // key or a new section. For the moment, keep it simple. > diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal > index 4703f652c009..e6f8fa3c9b8d 100644 > --- a/scripts/Makefile.modfinal > +++ b/scripts/Makefile.modfinal > @@ -41,8 +41,6 @@ quiet_cmd_btf_ko = BTF [M] $@ > cmd_btf_ko = \ > if [ ! -f vmlinux ]; then \ > printf "Skipping BTF generation for %s due to unavailability of vmlinux\n" $@ 1>&2; \ > - elif [ -n "$(CONFIG_RUST)" ] && $(srctree)/scripts/is_rust_module.sh $@; then \ > - printf "Skipping BTF generation for %s because it's a Rust module\n" $@ 1>&2; \ > else \ > LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ > $(RESOLVE_BTFIDS) -b vmlinux $@; \ > diff --git a/scripts/is_rust_module.sh b/scripts/is_rust_module.sh > deleted file mode 100755 > index 28b3831a7593..000000000000 > --- a/scripts/is_rust_module.sh > +++ /dev/null > @@ -1,16 +0,0 @@ > -#!/bin/sh > -# SPDX-License-Identifier: GPL-2.0 > -# > -# is_rust_module.sh module.ko > -# > -# Returns `0` if `module.ko` is a Rust module, `1` otherwise. > - > -set -e > - > -# Using the `16_` prefix ensures other symbols with the same substring > -# are not picked up (even if it would be unlikely). The last part is > -# used just in case LLVM decides to use the `.` suffix. > -# > -# In the future, checking for the `.comment` section may be another > -# option, see https://github.com/rust-lang/rust/pull/97550. > -${NM} "$*" | grep -qE '^[0-9a-fA-F]+ r _R[^[:space:]]+16___IS_RUST_MODULE[^[:space:]]*$' > -- > 2.39.2 > LGTM. Reviewed-by: Neal Gompa <neal@gompa.dev> -- 真実はいつも一つ！/ Always, there's only one truth!
On Fri, Apr 7, 2023 at 4:05 PM Andrea Righi <andrea.righi@canonical.com> wrote: > > Commit c1177979af9c ("btf, scripts: Exclude Rust CUs with pahole") > introduced the constraint "!DEBUG_INFO_BTF || PAHOLE_HAS_LANG_EXCLUDE" > to enable RUST. > > With this constraint we don't need is_rust_module.sh anymore, because > 'pahole --lang_exclude=rust' already has the capability to exclude Rust > CUs. If pahole isn't recent enough (< 1.24) to support --lang_exclude, > then DEBUG_INFO_BTF can't be enabled with RUST and is_rust_module.sh > isn't used as well. > > In any case is_rust_module.sh is obsolete and we can just drop it. > > Link: https://lore.kernel.org/lkml/Y+p2xKIN6TJnQinK@righiandr-XPS-13-7390/ > Signed-off-by: Andrea Righi Reviewed-by: Masahiro Yamada <masahiroy@kernel.org>
On 4/7/23 04:05, Andrea Righi wrote: > Commit c1177979af9c ("btf, scripts: Exclude Rust CUs with pahole") > introduced the constraint "!DEBUG_INFO_BTF || PAHOLE_HAS_LANG_EXCLUDE" > to enable RUST. > > With this constraint we don't need is_rust_module.sh anymore, because > 'pahole --lang_exclude=rust' already has the capability to exclude Rust > CUs. If pahole isn't recent enough (< 1.24) to support --lang_exclude, > then DEBUG_INFO_BTF can't be enabled with RUST and is_rust_module.sh > isn't used as well. > > In any case is_rust_module.sh is obsolete and we can just drop it. > > Link: https://lore.kernel.org/lkml/Y+p2xKIN6TJnQinK@righiandr-XPS-13-7390/ > Signed-off-by: Andrea Righi <andrea.righi@canonical.com> > --- > [...] Go on Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
diff --git a/rust/macros/module.rs b/rust/macros/module.rs index a7e363c2b044..608406f33a70 100644 --- a/rust/macros/module.rs +++ b/rust/macros/module.rs @@ -179,7 +179,7 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream { /// Used by the printing macros, e.g. [`info!`]. const __LOG_PREFIX: &[u8] = b\"{name}\\0\"; - /// The \"Rust loadable module\" mark, for `scripts/is_rust_module.sh`. + /// The \"Rust loadable module\" mark. // // This may be best done another way later on, e.g. as a new modinfo // key or a new section. For the moment, keep it simple. diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal index 4703f652c009..e6f8fa3c9b8d 100644 --- a/scripts/Makefile.modfinal +++ b/scripts/Makefile.modfinal @@ -41,8 +41,6 @@ quiet_cmd_btf_ko = BTF [M] $@ cmd_btf_ko = \ if [ ! -f vmlinux ]; then \ printf "Skipping BTF generation for %s due to unavailability of vmlinux\n" $@ 1>&2; \ - elif [ -n "$(CONFIG_RUST)" ] && $(srctree)/scripts/is_rust_module.sh $@; then \ - printf "Skipping BTF generation for %s because it's a Rust module\n" $@ 1>&2; \ else \ LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ $(RESOLVE_BTFIDS) -b vmlinux $@; \ diff --git a/scripts/is_rust_module.sh b/scripts/is_rust_module.sh deleted file mode 100755 index 28b3831a7593..000000000000 --- a/scripts/is_rust_module.sh +++ /dev/null @@ -1,16 +0,0 @@ -#!/bin/sh -# SPDX-License-Identifier: GPL-2.0 -# -# is_rust_module.sh module.ko -# -# Returns `0` if `module.ko` is a Rust module, `1` otherwise. - -set -e - -# Using the `16_` prefix ensures other symbols with the same substring -# are not picked up (even if it would be unlikely). The last part is -# used just in case LLVM decides to use the `.` suffix. -# -# In the future, checking for the `.comment` section may be another -# option, see https://github.com/rust-lang/rust/pull/97550. -${NM} "$*" | grep -qE '^[0-9a-fA-F]+ r _R[^[:space:]]+16___IS_RUST_MODULE[^[:space:]]*$'
Commit c1177979af9c ("btf, scripts: Exclude Rust CUs with pahole") introduced the constraint "!DEBUG_INFO_BTF || PAHOLE_HAS_LANG_EXCLUDE" to enable RUST. With this constraint we don't need is_rust_module.sh anymore, because 'pahole --lang_exclude=rust' already has the capability to exclude Rust CUs. If pahole isn't recent enough (< 1.24) to support --lang_exclude, then DEBUG_INFO_BTF can't be enabled with RUST and is_rust_module.sh isn't used as well. In any case is_rust_module.sh is obsolete and we can just drop it. Link: https://lore.kernel.org/lkml/Y+p2xKIN6TJnQinK@righiandr-XPS-13-7390/ Signed-off-by: Andrea Righi <andrea.righi@canonical.com> --- rust/macros/module.rs | 2 +- scripts/Makefile.modfinal | 2 -- scripts/is_rust_module.sh | 16 ---------------- 3 files changed, 1 insertion(+), 19 deletions(-) delete mode 100755 scripts/is_rust_module.sh