LinuxLists.cc - [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN

2021-06-21 23:19:56

Subject: [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO

We don't want compiler instrumentation to touch noinstr functions, which
are annotated with the no_profile_instrument_function function
attribute. Add a Kconfig test for this and make PGO and GCOV depend on
it.

If an architecture is using noinstr, it should denote that via this
Kconfig value. That makes Kconfigs that depend on noinstr able to
express dependencies in an architecturally agnostic way.

Cc: Masahiro Yamada <[email protected]>
Cc: Peter Oberparleiter <[email protected]>
Link: https://lore.kernel.org/lkml/[email protected]/
Link: https://lore.kernel.org/lkml/YMcssV%[email protected]/
Suggested-by: Nathan Chancellor <[email protected]>
Suggested-by: Peter Zijlstra <[email protected]>
Signed-off-by: Nick Desaulniers <[email protected]>
---
Changes V1 -> V2:
* Add ARCH_WANTS_NO_INSTR
* Change depdendencies to be !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
rather than list architectures explicitly, as per Nathan.
* s/no_profile/no_profile_instrument_function/

arch/Kconfig | 7 +++++++
arch/arm64/Kconfig | 1 +
arch/s390/Kconfig | 1 +
arch/x86/Kconfig | 1 +
init/Kconfig | 3 +++
kernel/gcov/Kconfig | 1 +
kernel/pgo/Kconfig | 3 ++-
7 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 2b4109b0edee..2113c6b3b801 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -285,6 +285,13 @@ config ARCH_THREAD_STACK_ALLOCATOR
config ARCH_WANTS_DYNAMIC_TASK_STRUCT
bool

+config ARCH_WANTS_NO_INSTR
+ bool
+ help
+ An architecure should select this if the noinstr macro is being used on
+ functions to denote that the toolchain should avoid instrumenting such
+ functions and is required for correctness.
+
config ARCH_32BIT_OFF_T
bool
depends on !64BIT
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 9f1d8566bbf9..39bf982b06f8 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -93,6 +93,7 @@ config ARM64
select ARCH_WANT_FRAME_POINTERS
select ARCH_WANT_HUGE_PMD_SHARE if ARM64_4K_PAGES || (ARM64_16K_PAGES && !ARM64_VA_BITS_36)
select ARCH_WANT_LD_ORPHAN_WARN
+ select ARCH_WANTS_NO_INSTR
select ARCH_HAS_UBSAN_SANITIZE_ALL
select ARM_AMBA
select ARM_ARCH_TIMER
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index b4c7c34069f8..bd60310f33b9 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -117,6 +117,7 @@ config S390
select ARCH_USE_BUILTIN_BSWAP
select ARCH_USE_CMPXCHG_LOCKREF
select ARCH_WANTS_DYNAMIC_TASK_STRUCT
+ select ARCH_WANTS_NO_INSTR
select ARCH_WANT_DEFAULT_BPF_JIT
select ARCH_WANT_IPC_PARSE_VERSION
select BUILDTIME_TABLE_SORT
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index da43fd046149..7d6a44bb9b0e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -114,6 +114,7 @@ config X86
select ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
select ARCH_WANT_DEFAULT_BPF_JIT if X86_64
select ARCH_WANTS_DYNAMIC_TASK_STRUCT
+ select ARCH_WANTS_NO_INSTR
select ARCH_WANT_HUGE_PMD_SHARE
select ARCH_WANT_LD_ORPHAN_WARN
select ARCH_WANTS_THP_SWAP if X86_64
diff --git a/init/Kconfig b/init/Kconfig
index 1ea12c64e4c9..31397a7a45fb 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -83,6 +83,9 @@ config TOOLS_SUPPORT_RELR
config CC_HAS_ASM_INLINE
def_bool $(success,echo 'void foo(void) { asm inline (""); }' | $(CC) -x c - -c -o /dev/null)

+config CC_HAS_NO_PROFILE_FN_ATTR
+ def_bool $(success,echo '__attribute__((no_profile_instrument_function)) int x();' | $(CC) -x c - -c -o /dev/null -Werror)
+
config CONSTRUCTORS
bool

diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
index 58f87a3092f3..053447183ac5 100644
--- a/kernel/gcov/Kconfig
+++ b/kernel/gcov/Kconfig
@@ -5,6 +5,7 @@ config GCOV_KERNEL
bool "Enable gcov-based kernel profiling"
depends on DEBUG_FS
depends on !CC_IS_CLANG || CLANG_VERSION >= 110000
+ depends on !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
select CONSTRUCTORS
default n
help
diff --git a/kernel/pgo/Kconfig b/kernel/pgo/Kconfig
index d2053df1111c..ce7fe04f303d 100644
--- a/kernel/pgo/Kconfig
+++ b/kernel/pgo/Kconfig
@@ -8,7 +8,8 @@ config PGO_CLANG
bool "Enable clang's PGO-based kernel profiling"
depends on DEBUG_FS
depends on ARCH_SUPPORTS_PGO_CLANG
- depends on CC_IS_CLANG && CLANG_VERSION >= 120000
+ depends on CC_IS_CLANG
+ depends on !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
help
This option enables clang's PGO (Profile Guided Optimization) based
code profiling to better optimize the kernel.
--
2.32.0.288.g62a8d224e6-goog

2021-06-21 23:47:20

by Nathan Chancellor

[permalink] [raw]

Subject: Re: [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO

On 6/21/2021 4:18 PM, 'Nick Desaulniers' via Clang Built Linux wrote:
> We don't want compiler instrumentation to touch noinstr functions, which
> are annotated with the no_profile_instrument_function function
> attribute. Add a Kconfig test for this and make PGO and GCOV depend on
> it.
>
> If an architecture is using noinstr, it should denote that via this
> Kconfig value. That makes Kconfigs that depend on noinstr able to
> express dependencies in an architecturally agnostic way.
>
> Cc: Masahiro Yamada <[email protected]>
> Cc: Peter Oberparleiter <[email protected]>
> Link: https://lore.kernel.org/lkml/[email protected]/
> Link: https://lore.kernel.org/lkml/YMcssV%[email protected]/
> Suggested-by: Nathan Chancellor <[email protected]>
> Suggested-by: Peter Zijlstra <[email protected]>
> Signed-off-by: Nick Desaulniers <[email protected]>

Reviewed-by: Nathan Chancellor <[email protected]>

Small nit below.

> ---
> Changes V1 -> V2:
> * Add ARCH_WANTS_NO_INSTR
> * Change depdendencies to be !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> rather than list architectures explicitly, as per Nathan.
> * s/no_profile/no_profile_instrument_function/
>
> arch/Kconfig | 7 +++++++
> arch/arm64/Kconfig | 1 +
> arch/s390/Kconfig | 1 +
> arch/x86/Kconfig | 1 +
> init/Kconfig | 3 +++
> kernel/gcov/Kconfig | 1 +
> kernel/pgo/Kconfig | 3 ++-
> 7 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 2b4109b0edee..2113c6b3b801 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -285,6 +285,13 @@ config ARCH_THREAD_STACK_ALLOCATOR
> config ARCH_WANTS_DYNAMIC_TASK_STRUCT
> bool
>
> +config ARCH_WANTS_NO_INSTR
> + bool
> + help
> + An architecure should select this if the noinstr macro is being used on

Architecture

> + functions to denote that the toolchain should avoid instrumenting such
> + functions and is required for correctness.
> +
> config ARCH_32BIT_OFF_T
> bool
> depends on !64BIT
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 9f1d8566bbf9..39bf982b06f8 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -93,6 +93,7 @@ config ARM64
> select ARCH_WANT_FRAME_POINTERS
> select ARCH_WANT_HUGE_PMD_SHARE if ARM64_4K_PAGES || (ARM64_16K_PAGES && !ARM64_VA_BITS_36)
> select ARCH_WANT_LD_ORPHAN_WARN
> + select ARCH_WANTS_NO_INSTR
> select ARCH_HAS_UBSAN_SANITIZE_ALL
> select ARM_AMBA
> select ARM_ARCH_TIMER
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index b4c7c34069f8..bd60310f33b9 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -117,6 +117,7 @@ config S390
> select ARCH_USE_BUILTIN_BSWAP
> select ARCH_USE_CMPXCHG_LOCKREF
> select ARCH_WANTS_DYNAMIC_TASK_STRUCT
> + select ARCH_WANTS_NO_INSTR
> select ARCH_WANT_DEFAULT_BPF_JIT
> select ARCH_WANT_IPC_PARSE_VERSION
> select BUILDTIME_TABLE_SORT
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index da43fd046149..7d6a44bb9b0e 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -114,6 +114,7 @@ config X86
> select ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
> select ARCH_WANT_DEFAULT_BPF_JIT if X86_64
> select ARCH_WANTS_DYNAMIC_TASK_STRUCT
> + select ARCH_WANTS_NO_INSTR
> select ARCH_WANT_HUGE_PMD_SHARE
> select ARCH_WANT_LD_ORPHAN_WARN
> select ARCH_WANTS_THP_SWAP if X86_64
> diff --git a/init/Kconfig b/init/Kconfig
> index 1ea12c64e4c9..31397a7a45fb 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -83,6 +83,9 @@ config TOOLS_SUPPORT_RELR
> config CC_HAS_ASM_INLINE
> def_bool $(success,echo 'void foo(void) { asm inline (""); }' | $(CC) -x c - -c -o /dev/null)
>
> +config CC_HAS_NO_PROFILE_FN_ATTR
> + def_bool $(success,echo '__attribute__((no_profile_instrument_function)) int x();' | $(CC) -x c - -c -o /dev/null -Werror)
> +
> config CONSTRUCTORS
> bool
>
> diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
> index 58f87a3092f3..053447183ac5 100644
> --- a/kernel/gcov/Kconfig
> +++ b/kernel/gcov/Kconfig
> @@ -5,6 +5,7 @@ config GCOV_KERNEL
> bool "Enable gcov-based kernel profiling"
> depends on DEBUG_FS
> depends on !CC_IS_CLANG || CLANG_VERSION >= 110000
> + depends on !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> select CONSTRUCTORS
> default n
> help
> diff --git a/kernel/pgo/Kconfig b/kernel/pgo/Kconfig
> index d2053df1111c..ce7fe04f303d 100644
> --- a/kernel/pgo/Kconfig
> +++ b/kernel/pgo/Kconfig
> @@ -8,7 +8,8 @@ config PGO_CLANG
> bool "Enable clang's PGO-based kernel profiling"
> depends on DEBUG_FS
> depends on ARCH_SUPPORTS_PGO_CLANG
> - depends on CC_IS_CLANG && CLANG_VERSION >= 120000
> + depends on CC_IS_CLANG
> + depends on !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> help
> This option enables clang's PGO (Profile Guided Optimization) based
> code profiling to better optimize the kernel.
>

2021-06-22 07:27:02

by Peter Zijlstra

[permalink] [raw]

Subject: Re: [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO

2021-06-22 08:10:10

by Marco Elver

[permalink] [raw]

Subject: Re: [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO

On Tue, 22 Jun 2021 at 09:26, Peter Zijlstra <[email protected]> wrote:
>
> On Mon, Jun 21, 2021 at 04:18:22PM -0700, Nick Desaulniers wrote:
> > We don't want compiler instrumentation to touch noinstr functions, which
> > are annotated with the no_profile_instrument_function function
> > attribute. Add a Kconfig test for this and make PGO and GCOV depend on
> > it.
> >
> > If an architecture is using noinstr, it should denote that via this
> > Kconfig value. That makes Kconfigs that depend on noinstr able to
> > express dependencies in an architecturally agnostic way.
> >
> > Cc: Masahiro Yamada <[email protected]>
> > Cc: Peter Oberparleiter <[email protected]>
> > Link: https://lore.kernel.org/lkml/[email protected]/
> > Link: https://lore.kernel.org/lkml/YMcssV%[email protected]/
> > Suggested-by: Nathan Chancellor <[email protected]>
> > Suggested-by: Peter Zijlstra <[email protected]>
> > Signed-off-by: Nick Desaulniers <[email protected]>
> > ---
> > Changes V1 -> V2:
> > * Add ARCH_WANTS_NO_INSTR
> > * Change depdendencies to be !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> > rather than list architectures explicitly, as per Nathan.
> > * s/no_profile/no_profile_instrument_function/
> >
> > arch/Kconfig | 7 +++++++
> > arch/arm64/Kconfig | 1 +
> > arch/s390/Kconfig | 1 +
> > arch/x86/Kconfig | 1 +
> > init/Kconfig | 3 +++
> > kernel/gcov/Kconfig | 1 +
> > kernel/pgo/Kconfig | 3 ++-
> > 7 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > index 2b4109b0edee..2113c6b3b801 100644
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -285,6 +285,13 @@ config ARCH_THREAD_STACK_ALLOCATOR
> > config ARCH_WANTS_DYNAMIC_TASK_STRUCT
> > bool
> >
> > +config ARCH_WANTS_NO_INSTR
> > + bool
> > + help
> > + An architecure should select this if the noinstr macro is being used on
> > + functions to denote that the toolchain should avoid instrumenting such
> > + functions and is required for correctness.
> > +
> > config ARCH_32BIT_OFF_T
> > bool
> > depends on !64BIT
>
> There's also CC_HAS_WORKING_NOSANITIZE_ADDRESS in lib/Kconfig.kasan that
> might want to be hooked into this, but that can be done separately I
> suppose.

KASAN already depends on this for all compiler instrumentation modes,
not just for 'noinstr' but also to avoid false positives. So it's not
just for noinstr's benefit, and we should not weaken the requirement
there.

2021-06-22 09:07:19

by Mark Rutland

[permalink] [raw]

Subject: Re: [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO

On Mon, Jun 21, 2021 at 04:18:22PM -0700, Nick Desaulniers wrote:
> We don't want compiler instrumentation to touch noinstr functions, which
> are annotated with the no_profile_instrument_function function
> attribute. Add a Kconfig test for this and make PGO and GCOV depend on
> it.
>
> If an architecture is using noinstr, it should denote that via this
> Kconfig value. That makes Kconfigs that depend on noinstr able to
> express dependencies in an architecturally agnostic way.
>
> Cc: Masahiro Yamada <[email protected]>
> Cc: Peter Oberparleiter <[email protected]>
> Link: https://lore.kernel.org/lkml/[email protected]/
> Link: https://lore.kernel.org/lkml/YMcssV%[email protected]/
> Suggested-by: Nathan Chancellor <[email protected]>
> Suggested-by: Peter Zijlstra <[email protected]>
> Signed-off-by: Nick Desaulniers <[email protected]>

FWIW, this looks good to me:

Acked-by: Mark Rutland <[email protected]>

Catalin, Will, are you happy iwth the arm64 bit?

Thanks,
Makr.

> ---
> Changes V1 -> V2:
> * Add ARCH_WANTS_NO_INSTR
> * Change depdendencies to be !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> rather than list architectures explicitly, as per Nathan.
> * s/no_profile/no_profile_instrument_function/
>
> arch/Kconfig | 7 +++++++
> arch/arm64/Kconfig | 1 +
> arch/s390/Kconfig | 1 +
> arch/x86/Kconfig | 1 +
> init/Kconfig | 3 +++
> kernel/gcov/Kconfig | 1 +
> kernel/pgo/Kconfig | 3 ++-
> 7 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 2b4109b0edee..2113c6b3b801 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -285,6 +285,13 @@ config ARCH_THREAD_STACK_ALLOCATOR
> config ARCH_WANTS_DYNAMIC_TASK_STRUCT
> bool
>
> +config ARCH_WANTS_NO_INSTR
> + bool
> + help
> + An architecure should select this if the noinstr macro is being used on
> + functions to denote that the toolchain should avoid instrumenting such
> + functions and is required for correctness.
> +
> config ARCH_32BIT_OFF_T
> bool
> depends on !64BIT
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 9f1d8566bbf9..39bf982b06f8 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -93,6 +93,7 @@ config ARM64
> select ARCH_WANT_FRAME_POINTERS
> select ARCH_WANT_HUGE_PMD_SHARE if ARM64_4K_PAGES || (ARM64_16K_PAGES && !ARM64_VA_BITS_36)
> select ARCH_WANT_LD_ORPHAN_WARN
> + select ARCH_WANTS_NO_INSTR
> select ARCH_HAS_UBSAN_SANITIZE_ALL
> select ARM_AMBA
> select ARM_ARCH_TIMER
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index b4c7c34069f8..bd60310f33b9 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -117,6 +117,7 @@ config S390
> select ARCH_USE_BUILTIN_BSWAP
> select ARCH_USE_CMPXCHG_LOCKREF
> select ARCH_WANTS_DYNAMIC_TASK_STRUCT
> + select ARCH_WANTS_NO_INSTR
> select ARCH_WANT_DEFAULT_BPF_JIT
> select ARCH_WANT_IPC_PARSE_VERSION
> select BUILDTIME_TABLE_SORT
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index da43fd046149..7d6a44bb9b0e 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -114,6 +114,7 @@ config X86
> select ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
> select ARCH_WANT_DEFAULT_BPF_JIT if X86_64
> select ARCH_WANTS_DYNAMIC_TASK_STRUCT
> + select ARCH_WANTS_NO_INSTR
> select ARCH_WANT_HUGE_PMD_SHARE
> select ARCH_WANT_LD_ORPHAN_WARN
> select ARCH_WANTS_THP_SWAP if X86_64
> diff --git a/init/Kconfig b/init/Kconfig
> index 1ea12c64e4c9..31397a7a45fb 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -83,6 +83,9 @@ config TOOLS_SUPPORT_RELR
> config CC_HAS_ASM_INLINE
> def_bool $(success,echo 'void foo(void) { asm inline (""); }' | $(CC) -x c - -c -o /dev/null)
>
> +config CC_HAS_NO_PROFILE_FN_ATTR
> + def_bool $(success,echo '__attribute__((no_profile_instrument_function)) int x();' | $(CC) -x c - -c -o /dev/null -Werror)
> +
> config CONSTRUCTORS
> bool
>
> diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
> index 58f87a3092f3..053447183ac5 100644
> --- a/kernel/gcov/Kconfig
> +++ b/kernel/gcov/Kconfig
> @@ -5,6 +5,7 @@ config GCOV_KERNEL
> bool "Enable gcov-based kernel profiling"
> depends on DEBUG_FS
> depends on !CC_IS_CLANG || CLANG_VERSION >= 110000
> + depends on !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> select CONSTRUCTORS
> default n
> help
> diff --git a/kernel/pgo/Kconfig b/kernel/pgo/Kconfig
> index d2053df1111c..ce7fe04f303d 100644
> --- a/kernel/pgo/Kconfig
> +++ b/kernel/pgo/Kconfig
> @@ -8,7 +8,8 @@ config PGO_CLANG
> bool "Enable clang's PGO-based kernel profiling"
> depends on DEBUG_FS
> depends on ARCH_SUPPORTS_PGO_CLANG
> - depends on CC_IS_CLANG && CLANG_VERSION >= 120000
> + depends on CC_IS_CLANG
> + depends on !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> help
> This option enables clang's PGO (Profile Guided Optimization) based
> code profiling to better optimize the kernel.
> --
> 2.32.0.288.g62a8d224e6-goog
>

2021-06-22 09:35:45

by Heiko Carstens

[permalink] [raw]

Subject: Re: [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO

2021-06-22 09:37:25

by Peter Oberparleiter

[permalink] [raw]

Subject: Re: [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO

On 22.06.2021 01:18, Nick Desaulniers wrote:
> We don't want compiler instrumentation to touch noinstr functions, which
> are annotated with the no_profile_instrument_function function
> attribute. Add a Kconfig test for this and make PGO and GCOV depend on
> it.
>
> If an architecture is using noinstr, it should denote that via this
> Kconfig value. That makes Kconfigs that depend on noinstr able to
> express dependencies in an architecturally agnostic way.
>
> Cc: Masahiro Yamada <[email protected]>
> Cc: Peter Oberparleiter <[email protected]>
> Link: https://lore.kernel.org/lkml/[email protected]/
> Link: https://lore.kernel.org/lkml/YMcssV%[email protected]/
> Suggested-by: Nathan Chancellor <[email protected]>
> Suggested-by: Peter Zijlstra <[email protected]>
> Signed-off-by: Nick Desaulniers <[email protected]>

Looks good to me - thanks for resolving this problem!

Reviewed-by: Peter Oberparleiter <[email protected]>

Regards,
Peter

--
Peter Oberparleiter
Linux on Z Development - IBM Germany

2021-06-22 11:14:12

by Will Deacon

[permalink] [raw]

Subject: Re: [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO

On Tue, Jun 22, 2021 at 10:05:40AM +0100, Mark Rutland wrote:
> On Mon, Jun 21, 2021 at 04:18:22PM -0700, Nick Desaulniers wrote:
> > We don't want compiler instrumentation to touch noinstr functions, which
> > are annotated with the no_profile_instrument_function function
> > attribute. Add a Kconfig test for this and make PGO and GCOV depend on
> > it.
> >
> > If an architecture is using noinstr, it should denote that via this
> > Kconfig value. That makes Kconfigs that depend on noinstr able to
> > express dependencies in an architecturally agnostic way.
> >
> > Cc: Masahiro Yamada <[email protected]>
> > Cc: Peter Oberparleiter <[email protected]>
> > Link: https://lore.kernel.org/lkml/[email protected]/
> > Link: https://lore.kernel.org/lkml/YMcssV%[email protected]/
> > Suggested-by: Nathan Chancellor <[email protected]>
> > Suggested-by: Peter Zijlstra <[email protected]>
> > Signed-off-by: Nick Desaulniers <[email protected]>
>
> FWIW, this looks good to me:
>
> Acked-by: Mark Rutland <[email protected]>
>
> Catalin, Will, are you happy iwth the arm64 bit?

Looks fine to me.

Will