* Re: [PATCH] scripts: rust: drop is_rust_module.sh
2023-04-07 7:05 [PATCH] scripts: rust: drop is_rust_module.sh Andrea Righi
@ 2023-04-07 17:53 ` Nathan Chancellor
2023-04-07 19:07 ` Andrea Righi
2023-04-08 10:24 ` Eric Curtin
` (3 subsequent siblings)
4 siblings, 1 reply; 7+ messages in thread
From: Nathan Chancellor @ 2023-04-07 17:53 UTC (permalink / raw)
To: Andrea Righi
Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Masahiro Yamada, Nick Desaulniers,
Nicolas Schier, Tom Rix, Eric Curtin, Martin Rodriguez Reboredo,
Daniel Borkmann, Neal Gompa, Arnaldo Carvalho de Melo,
rust-for-linux, linux-kbuild, llvm, linux-kernel
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
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] scripts: rust: drop is_rust_module.sh
2023-04-07 17:53 ` Nathan Chancellor
@ 2023-04-07 19:07 ` Andrea Righi
0 siblings, 0 replies; 7+ messages in thread
From: Andrea Righi @ 2023-04-07 19:07 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Masahiro Yamada, Nick Desaulniers,
Nicolas Schier, Tom Rix, Eric Curtin, Martin Rodriguez Reboredo,
Daniel Borkmann, Neal Gompa, Arnaldo Carvalho de Melo,
rust-for-linux, linux-kbuild, llvm, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] scripts: rust: drop is_rust_module.sh
2023-04-07 7:05 [PATCH] scripts: rust: drop is_rust_module.sh Andrea Righi
2023-04-07 17:53 ` Nathan Chancellor
@ 2023-04-08 10:24 ` Eric Curtin
2023-04-08 10:28 ` Neal Gompa
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Eric Curtin @ 2023-04-08 10:24 UTC (permalink / raw)
To: Andrea Righi
Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Masahiro Yamada,
Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Tom Rix,
Martin Rodriguez Reboredo, Daniel Borkmann, Neal Gompa,
Arnaldo Carvalho de Melo, rust-for-linux, linux-kbuild, llvm,
linux-kernel
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
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] scripts: rust: drop is_rust_module.sh
2023-04-07 7:05 [PATCH] scripts: rust: drop is_rust_module.sh Andrea Righi
2023-04-07 17:53 ` Nathan Chancellor
2023-04-08 10:24 ` Eric Curtin
@ 2023-04-08 10:28 ` Neal Gompa
2023-04-08 13:41 ` Masahiro Yamada
2023-04-08 18:24 ` Martin Rodriguez Reboredo
4 siblings, 0 replies; 7+ messages in thread
From: Neal Gompa @ 2023-04-08 10:28 UTC (permalink / raw)
To: Andrea Righi
Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Masahiro Yamada,
Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Tom Rix,
Eric Curtin, Martin Rodriguez Reboredo, Daniel Borkmann,
Arnaldo Carvalho de Melo, rust-for-linux, linux-kbuild, llvm,
linux-kernel
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!
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] scripts: rust: drop is_rust_module.sh
2023-04-07 7:05 [PATCH] scripts: rust: drop is_rust_module.sh Andrea Righi
` (2 preceding siblings ...)
2023-04-08 10:28 ` Neal Gompa
@ 2023-04-08 13:41 ` Masahiro Yamada
2023-04-08 18:24 ` Martin Rodriguez Reboredo
4 siblings, 0 replies; 7+ messages in thread
From: Masahiro Yamada @ 2023-04-08 13:41 UTC (permalink / raw)
To: Andrea Righi
Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Nathan Chancellor,
Nick Desaulniers, Nicolas Schier, Tom Rix, Eric Curtin,
Martin Rodriguez Reboredo, Daniel Borkmann, Neal Gompa,
Arnaldo Carvalho de Melo, rust-for-linux, linux-kbuild, llvm,
linux-kernel
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>
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] scripts: rust: drop is_rust_module.sh
2023-04-07 7:05 [PATCH] scripts: rust: drop is_rust_module.sh Andrea Righi
` (3 preceding siblings ...)
2023-04-08 13:41 ` Masahiro Yamada
@ 2023-04-08 18:24 ` Martin Rodriguez Reboredo
4 siblings, 0 replies; 7+ messages in thread
From: Martin Rodriguez Reboredo @ 2023-04-08 18:24 UTC (permalink / raw)
To: Andrea Righi, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho
Cc: Boqun Feng, Gary Guo, Björn Roy Baron, Masahiro Yamada,
Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Tom Rix,
Eric Curtin, Daniel Borkmann, Neal Gompa,
Arnaldo Carvalho de Melo, rust-for-linux, linux-kbuild, llvm,
linux-kernel
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>
^ permalink raw reply [flat|nested] 7+ messages in thread