From: Nick Desaulniers <ndesaulniers@google.com> Date: 2021-06-18 23:30:48
When we say noinstr, we mean noinstr. GCOV and PGO can both instrument
functions. Add a new function annotation __no_profile that expands to
__attribute__((__no_profile__)) and Kconfig value
CC_HAS_NO_PROFILE_FN_ATTR.
Base is
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=for-next/clang/pgo.
Nick Desaulniers (2):
compiler_attributes.h: define __no_profile, add to noinstr
Kconfig: CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO
include/linux/compiler_attributes.h | 12 ++++++++++++
include/linux/compiler_types.h | 2 +-
init/Kconfig | 3 +++
kernel/gcov/Kconfig | 1 +
kernel/pgo/Kconfig | 3 ++-
5 files changed, 19 insertions(+), 2 deletions(-)
base-commit: 4356bc4c0425c81e204f561acf4dd0095544a6cb
--
2.32.0.288.g62a8d224e6-goog
@@ -210,7 +210,7 @@ struct ftrace_likely_data {/* Section for code which can't be instrumented at all */#define noinstr \noinlinenotrace__attribute((__section__(".noinstr.text")))\-__no_kcsan__no_sanitize_address+__no_kcsan__no_sanitize_address__no_profile#endif /* __KERNEL__ */
From: Nick Desaulniers <ndesaulniers@google.com> Date: 2021-06-18 23:30:54
We don't want compiler instrumentation to touch noinstr functions, which
are annotated with the no_profile function attribute. Add a Kconfig test
for this and make PGO and GCOV depend on it.
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
Link: https://lore.kernel.org/lkml/YMTn9yjuemKFLbws@hirez.programming.kicks-ass.net/
Link: https://lore.kernel.org/lkml/YMcssV%2Fn5IBGv4f0@hirez.programming.kicks-ass.net/
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
init/Kconfig | 3 +++
kernel/gcov/Kconfig | 1 +
kernel/pgo/Kconfig | 3 ++-
3 files changed, 6 insertions(+), 1 deletion(-)
From: Fangrui Song <hidden> Date: 2021-06-18 23:52:33
On 2021-06-18, Nick Desaulniers wrote:
When we say noinstr, we mean noinstr. GCOV and PGO can both instrument
functions. Add a new function annotation __no_profile that expands to
__attribute__((__no_profile__)) and Kconfig value
CC_HAS_NO_PROFILE_FN_ATTR.
Base is
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=for-next/clang/pgo.
Nick Desaulniers (2):
compiler_attributes.h: define __no_profile, add to noinstr
Kconfig: CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO
include/linux/compiler_attributes.h | 12 ++++++++++++
include/linux/compiler_types.h | 2 +-
init/Kconfig | 3 +++
kernel/gcov/Kconfig | 1 +
kernel/pgo/Kconfig | 3 ++-
5 files changed, 19 insertions(+), 2 deletions(-)
base-commit: 4356bc4c0425c81e204f561acf4dd0095544a6cb
--
2.32.0.288.g62a8d224e6-goog
Thanks for the attribute work in clang and kernel! Hope we can use clang
PGO in 5.14... (I am a casual contributor to clang PGO/coverage)
On Fri, Jun 18, 2021 at 04:30:21PM -0700, Nick Desaulniers wrote:
When we say noinstr, we mean noinstr. GCOV and PGO can both instrument
functions. Add a new function annotation __no_profile that expands to
__attribute__((__no_profile__)) and Kconfig value
CC_HAS_NO_PROFILE_FN_ATTR.
Base is
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=for-next/clang/pgo.
Nick Desaulniers (2):
compiler_attributes.h: define __no_profile, add to noinstr
Kconfig: CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO
Oh, awesome! Thanks for the fast work on this. If there are no objections,
I'll apply this in front of the PGO series and put it in -next.
-Kees
From: Marco Elver <elver@google.com> Date: 2021-06-19 06:23:52
On Sat, 19 Jun 2021 at 01:30, Nick Desaulniers [off-list ref] wrote:
quoted hunk
We don't want compiler instrumentation to touch noinstr functions, which
are annotated with the no_profile function attribute. Add a Kconfig test
for this and make PGO and GCOV depend on it.
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
Link: https://lore.kernel.org/lkml/YMTn9yjuemKFLbws@hirez.programming.kicks-ass.net/
Link: https://lore.kernel.org/lkml/YMcssV%2Fn5IBGv4f0@hirez.programming.kicks-ass.net/
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
init/Kconfig | 3 +++
kernel/gcov/Kconfig | 1 +
kernel/pgo/Kconfig | 3 ++-
3 files changed, 6 insertions(+), 1 deletion(-)
[+Cc Mark]
arm64 is also starting to rely on noinstr working properly.
This should probably be a 'select ARCH_HAS_GCOV_PROFILE_ALL if
CC_HAS_NO_PROFILE_FN_ATTR' in the relevant arch/../Kconfig.
Alternatively, using:
https://lkml.kernel.org/r/YMcssV/n5IBGv4f0@hirez.programming.kicks-ass.net
But I'd probably not overcomplicate things at this point and just use
ARCH_HAS_GCOV_PROFILE_ALL, because GCOV seems to be a) rarely used,
and b) if someone decides to selectively instrument stuff like entry
code, we can just say it's user error.
I am not sure if it is best or not to have the GCC link in order to be
consistent with the rest of the links (they are for the docs only). Do
we know if GCC going to implement it soon?
Otherwise, it looks good to me.
Cheers,
Miguel
From: Miguel Ojeda <hidden> Date: 2021-06-19 11:32:27
On Sat, Jun 19, 2021 at 1:26 PM Miguel Ojeda
[off-list ref] wrote:
I am not sure if it is best or not to have the GCC link in order to be
consistent with the rest of the links (they are for the docs only). Do
we know if GCC going to implement it soon?
i.e. if GCC does not implement it yet we use elsewhere this kind of
marker instead:
* Optional: not supported by gcc
The first of its kind, normally it is clang/icc there ;-)
We could nevertheless have the link there, something like:
* Optional: not supported by GCC
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223
Cheers,
Miguel
From: Bill Wendling <morbo@google.com> Date: 2021-06-20 08:09:25
On Fri, Jun 18, 2021 at 7:45 PM Kees Cook [off-list ref] wrote:
On Fri, Jun 18, 2021 at 04:30:21PM -0700, Nick Desaulniers wrote:
quoted
When we say noinstr, we mean noinstr. GCOV and PGO can both instrument
functions. Add a new function annotation __no_profile that expands to
__attribute__((__no_profile__)) and Kconfig value
CC_HAS_NO_PROFILE_FN_ATTR.
Base is
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=for-next/clang/pgo.
Nick Desaulniers (2):
compiler_attributes.h: define __no_profile, add to noinstr
Kconfig: CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO
Oh, awesome! Thanks for the fast work on this. If there are no objections,
I'll apply this in front of the PGO series and put it in -next.
From: Miguel Ojeda <hidden> Date: 2021-06-20 14:53:24
On Sat, Jun 19, 2021 at 4:45 AM Kees Cook [off-list ref] wrote:
Oh, awesome! Thanks for the fast work on this. If there are no objections,
I'll apply this in front of the PGO series and put it in -next.
If you are picking both patches on your tree, please see my comment on
the first commit.
With that solved, for the first commit:
Reviewed-by: Miguel Ojeda [off-list ref]
Cheers,
Miguel
From: Nick Desaulniers <ndesaulniers@google.com> Date: 2021-06-21 18:21:23
On Sat, Jun 19, 2021 at 4:32 AM Miguel Ojeda
[off-list ref] wrote:
On Sat, Jun 19, 2021 at 1:26 PM Miguel Ojeda
[off-list ref] wrote:
quoted
I am not sure if it is best or not to have the GCC link in order to be
consistent with the rest of the links (they are for the docs only). Do
we know if GCC going to implement it soon?
i.e. if GCC does not implement it yet we use elsewhere this kind of
marker instead:
* Optional: not supported by gcc
The first of its kind, normally it is clang/icc there ;-)
From: Nick Desaulniers <ndesaulniers@google.com> Date: 2021-06-21 18:22:50
On Fri, Jun 18, 2021 at 11:23 PM Marco Elver [off-list ref] wrote:
On Sat, 19 Jun 2021 at 01:30, Nick Desaulniers [off-list ref] wrote:
quoted
We don't want compiler instrumentation to touch noinstr functions, which
are annotated with the no_profile function attribute. Add a Kconfig test
for this and make PGO and GCOV depend on it.
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
Link: https://lore.kernel.org/lkml/YMTn9yjuemKFLbws@hirez.programming.kicks-ass.net/
Link: https://lore.kernel.org/lkml/YMcssV%2Fn5IBGv4f0@hirez.programming.kicks-ass.net/
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
init/Kconfig | 3 +++
kernel/gcov/Kconfig | 1 +
kernel/pgo/Kconfig | 3 ++-
3 files changed, 6 insertions(+), 1 deletion(-)
[+Cc Mark]
arm64 is also starting to rely on noinstr working properly.
Sure,
Will, Catalin, other arm64 folks:
Any thoughts on requiring GCC 7.1+/Clang 13.0+ for GCOV support? That
way we can better guarantee that GCOV (and eventually, PGO) don't
touch noinstr functions?
If that's ok, I'll add modify the above like:
+ depends on !ARM64 || (ARM64 && CC_HAS_NO_PROFILE_FN_ATTR)
to the above hunk in v2. Oh, looks like arch/s390 also uses noinstr.
Same question applies then:
+ depends on !S390 || (S390 && CC_HAS_NO_PROFILE_FN_ATTR)
Or, we could just do
+ depends on CC_HAS_NO_PROFILE_FN_ATTR
Though that will penalize architectures not using noinstr, that still
would like to use GCOV with versions of GCC older than 7.1. Perhaps
there are no such such users, or they should consider upgrading their
tools to we can stick with the simpler Kconfig? Thoughts?
This should probably be a 'select ARCH_HAS_GCOV_PROFILE_ALL if
CC_HAS_NO_PROFILE_FN_ATTR' in the relevant arch/../Kconfig.
Alternatively, using:
https://lkml.kernel.org/r/YMcssV/n5IBGv4f0@hirez.programming.kicks-ass.net
But I'd probably not overcomplicate things at this point and just use
ARCH_HAS_GCOV_PROFILE_ALL, because GCOV seems to be a) rarely used,
and b) if someone decides to selectively instrument stuff like entry
code, we can just say it's user error.
From: Fangrui Song <hidden> Date: 2021-06-21 18:24:32
On 2021-06-21, Nick Desaulniers wrote:
On Sat, Jun 19, 2021 at 4:32 AM Miguel Ojeda
[off-list ref] wrote:
quoted
On Sat, Jun 19, 2021 at 1:26 PM Miguel Ojeda
[off-list ref] wrote:
quoted
I am not sure if it is best or not to have the GCC link in order to be
consistent with the rest of the links (they are for the docs only). Do
we know if GCC going to implement it soon?
i.e. if GCC does not implement it yet we use elsewhere this kind of
marker instead:
* Optional: not supported by gcc
The first of its kind, normally it is clang/icc there ;-)
Thanks! __attribute__((no_profile_instrument_function)) looks good to me.
Also a reminder that __GCC4_has_attribute___no_profile in v1 misses two
underscores. v2 no_profile_instrument_function may need to fix this.
Reviewed-by: Fangrui Song <redacted>
From: Bill Wendling <morbo@google.com> Date: 2021-06-21 18:50:34
On Mon, Jun 21, 2021 at 11:22 AM Nick Desaulniers
[off-list ref] wrote:
On Fri, Jun 18, 2021 at 11:23 PM Marco Elver [off-list ref] wrote:
quoted
On Sat, 19 Jun 2021 at 01:30, Nick Desaulniers [off-list ref] wrote:
quoted
We don't want compiler instrumentation to touch noinstr functions, which
are annotated with the no_profile function attribute. Add a Kconfig test
for this and make PGO and GCOV depend on it.
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
Link: https://lore.kernel.org/lkml/YMTn9yjuemKFLbws@hirez.programming.kicks-ass.net/
Link: https://lore.kernel.org/lkml/YMcssV%2Fn5IBGv4f0@hirez.programming.kicks-ass.net/
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
init/Kconfig | 3 +++
kernel/gcov/Kconfig | 1 +
kernel/pgo/Kconfig | 3 ++-
3 files changed, 6 insertions(+), 1 deletion(-)
[+Cc Mark]
arm64 is also starting to rely on noinstr working properly.
Sure,
Will, Catalin, other arm64 folks:
Any thoughts on requiring GCC 7.1+/Clang 13.0+ for GCOV support? That
way we can better guarantee that GCOV (and eventually, PGO) don't
touch noinstr functions?
If that's ok, I'll add modify the above like:
+ depends on !ARM64 || (ARM64 && CC_HAS_NO_PROFILE_FN_ATTR)
Wouldn't "!ARM64 || CC_HAS_NO_PROFILE_FN_ATTR" be more succinct?
to the above hunk in v2. Oh, looks like arch/s390 also uses noinstr.
Same question applies then:
+ depends on !S390 || (S390 && CC_HAS_NO_PROFILE_FN_ATTR)
Or, we could just do
+ depends on CC_HAS_NO_PROFILE_FN_ATTR
Though that will penalize architectures not using noinstr, that still
would like to use GCOV with versions of GCC older than 7.1. Perhaps
there are no such such users, or they should consider upgrading their
tools to we can stick with the simpler Kconfig? Thoughts?
quoted
This should probably be a 'select ARCH_HAS_GCOV_PROFILE_ALL if
CC_HAS_NO_PROFILE_FN_ATTR' in the relevant arch/../Kconfig.
Alternatively, using:
https://lkml.kernel.org/r/YMcssV/n5IBGv4f0@hirez.programming.kicks-ass.net
But I'd probably not overcomplicate things at this point and just use
ARCH_HAS_GCOV_PROFILE_ALL, because GCOV seems to be a) rarely used,
and b) if someone decides to selectively instrument stuff like entry
code, we can just say it's user error.
From: Nick Desaulniers <ndesaulniers@google.com> Date: 2021-06-21 20:44:12
On Mon, Jun 21, 2021 at 11:50 AM Bill Wendling [off-list ref] wrote:
On Mon, Jun 21, 2021 at 11:22 AM Nick Desaulniers
[off-list ref] wrote:
quoted
On Fri, Jun 18, 2021 at 11:23 PM Marco Elver [off-list ref] wrote:
quoted
On Sat, 19 Jun 2021 at 01:30, Nick Desaulniers [off-list ref] wrote:
quoted
We don't want compiler instrumentation to touch noinstr functions, which
are annotated with the no_profile function attribute. Add a Kconfig test
for this and make PGO and GCOV depend on it.
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
Link: https://lore.kernel.org/lkml/YMTn9yjuemKFLbws@hirez.programming.kicks-ass.net/
Link: https://lore.kernel.org/lkml/YMcssV%2Fn5IBGv4f0@hirez.programming.kicks-ass.net/
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
init/Kconfig | 3 +++
kernel/gcov/Kconfig | 1 +
kernel/pgo/Kconfig | 3 ++-
3 files changed, 6 insertions(+), 1 deletion(-)
[+Cc Mark]
arm64 is also starting to rely on noinstr working properly.
Sure,
Will, Catalin, other arm64 folks:
Any thoughts on requiring GCC 7.1+/Clang 13.0+ for GCOV support? That
way we can better guarantee that GCOV (and eventually, PGO) don't
touch noinstr functions?
If that's ok, I'll add modify the above like:
+ depends on !ARM64 || (ARM64 && CC_HAS_NO_PROFILE_FN_ATTR)
Wouldn't "!ARM64 || CC_HAS_NO_PROFILE_FN_ATTR" be more succinct?
We need to be able to express via Kconfig "GCOV should not be enabled
for architectures that use noinstr when the toolchain does not support
__attribute__((no_profile_instrument_function))."
Where "architectures that use noinstr" are currently arm64, s390, and
x86. So I guess we could do:
+ depends on !ARM64 || !S390 || !X86 || CC_HAS_NO_PROFILE_FN_ATTR
(We could add a Kconfig for ARCH_WANTS_NO_INSTR, which might be more
informative than listed out architectures which might be non-obvious
to passers-by).
It would be most succinct to raise the requirements to: "GCOV should
not be enabled when the toolchain does not support
__attribute__((no_profile_instrument_function))." Then we could do:
+ depends on CC_HAS_NO_PROFILE_FN_ATTR
Assuming no one has the requirement to support GCOV on PPC with GCC <
7.1, for example.
quoted
to the above hunk in v2. Oh, looks like arch/s390 also uses noinstr.
Same question applies then:
+ depends on !S390 || (S390 && CC_HAS_NO_PROFILE_FN_ATTR)
Or, we could just do
+ depends on CC_HAS_NO_PROFILE_FN_ATTR
Though that will penalize architectures not using noinstr, that still
would like to use GCOV with versions of GCC older than 7.1. Perhaps
there are no such such users, or they should consider upgrading their
tools to we can stick with the simpler Kconfig? Thoughts?
quoted
This should probably be a 'select ARCH_HAS_GCOV_PROFILE_ALL if
CC_HAS_NO_PROFILE_FN_ATTR' in the relevant arch/../Kconfig.
Alternatively, using:
https://lkml.kernel.org/r/YMcssV/n5IBGv4f0@hirez.programming.kicks-ass.net
But I'd probably not overcomplicate things at this point and just use
ARCH_HAS_GCOV_PROFILE_ALL, because GCOV seems to be a) rarely used,
and b) if someone decides to selectively instrument stuff like entry
code, we can just say it's user error.
On Mon, Jun 21, 2021 at 11:50 AM Bill Wendling [off-list ref] wrote:
quoted
On Mon, Jun 21, 2021 at 11:22 AM Nick Desaulniers
[off-list ref] wrote:
quoted
On Fri, Jun 18, 2021 at 11:23 PM Marco Elver [off-list ref] wrote:
quoted
On Sat, 19 Jun 2021 at 01:30, Nick Desaulniers [off-list ref] wrote:
quoted
We don't want compiler instrumentation to touch noinstr functions, which
are annotated with the no_profile function attribute. Add a Kconfig test
for this and make PGO and GCOV depend on it.
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
Link: https://lore.kernel.org/lkml/YMTn9yjuemKFLbws@hirez.programming.kicks-ass.net/
Link: https://lore.kernel.org/lkml/YMcssV%2Fn5IBGv4f0@hirez.programming.kicks-ass.net/
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
init/Kconfig | 3 +++
kernel/gcov/Kconfig | 1 +
kernel/pgo/Kconfig | 3 ++-
3 files changed, 6 insertions(+), 1 deletion(-)
[+Cc Mark]
arm64 is also starting to rely on noinstr working properly.
Sure,
Will, Catalin, other arm64 folks:
Any thoughts on requiring GCC 7.1+/Clang 13.0+ for GCOV support? That
way we can better guarantee that GCOV (and eventually, PGO) don't
touch noinstr functions?
If that's ok, I'll add modify the above like:
+ depends on !ARM64 || (ARM64 && CC_HAS_NO_PROFILE_FN_ATTR)
Wouldn't "!ARM64 || CC_HAS_NO_PROFILE_FN_ATTR" be more succinct?
We need to be able to express via Kconfig "GCOV should not be enabled
for architectures that use noinstr when the toolchain does not support
__attribute__((no_profile_instrument_function))."
Where "architectures that use noinstr" are currently arm64, s390, and
x86. So I guess we could do:
+ depends on !ARM64 || !S390 || !X86 || CC_HAS_NO_PROFILE_FN_ATTR
(We could add a Kconfig for ARCH_WANTS_NO_INSTR, which might be more
informative than listed out architectures which might be non-obvious
to passers-by).
I agree that spelling this out might be nicer for the future, in case
instances like this crop up again. ARCH_REQUIRES_NO_INSTR might be a
better wording?
It would be most succinct to raise the requirements to: "GCOV should
not be enabled when the toolchain does not support
__attribute__((no_profile_instrument_function))." Then we could do:
+ depends on CC_HAS_NO_PROFILE_FN_ATTR
Then this could become
depends on !ARCH_REQUIRES_NO_INSTR || (ARCH_REQUIRES_NO_INSTR &&
CC_HAS_NO_PROFILE_FN_ATTR)
(sorry for the potential wrap).
Cheers,
Nathan
Assuming no one has the requirement to support GCOV on PPC with GCC <
7.1, for example.
quoted
quoted
to the above hunk in v2. Oh, looks like arch/s390 also uses noinstr.
Same question applies then:
+ depends on !S390 || (S390 && CC_HAS_NO_PROFILE_FN_ATTR)
Or, we could just do
+ depends on CC_HAS_NO_PROFILE_FN_ATTR
Though that will penalize architectures not using noinstr, that still
would like to use GCOV with versions of GCC older than 7.1. Perhaps
there are no such such users, or they should consider upgrading their
tools to we can stick with the simpler Kconfig? Thoughts?
quoted
This should probably be a 'select ARCH_HAS_GCOV_PROFILE_ALL if
CC_HAS_NO_PROFILE_FN_ATTR' in the relevant arch/../Kconfig.
Alternatively, using:
https://lkml.kernel.org/r/YMcssV/n5IBGv4f0@hirez.programming.kicks-ass.net
But I'd probably not overcomplicate things at this point and just use
ARCH_HAS_GCOV_PROFILE_ALL, because GCOV seems to be a) rarely used,
and b) if someone decides to selectively instrument stuff like entry
code, we can just say it's user error.
On Mon, Jun 21, 2021 at 01:43:54PM -0700, Nick Desaulniers wrote:
On Mon, Jun 21, 2021 at 11:50 AM Bill Wendling [off-list ref] wrote:
quoted
On Mon, Jun 21, 2021 at 11:22 AM Nick Desaulniers
[off-list ref] wrote:
quoted
On Fri, Jun 18, 2021 at 11:23 PM Marco Elver [off-list ref] wrote:
quoted
On Sat, 19 Jun 2021 at 01:30, Nick Desaulniers [off-list ref] wrote:
quoted
We don't want compiler instrumentation to touch noinstr functions, which
are annotated with the no_profile function attribute. Add a Kconfig test
for this and make PGO and GCOV depend on it.
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
Link: https://lore.kernel.org/lkml/YMTn9yjuemKFLbws@hirez.programming.kicks-ass.net/
Link: https://lore.kernel.org/lkml/YMcssV%2Fn5IBGv4f0@hirez.programming.kicks-ass.net/
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
init/Kconfig | 3 +++
kernel/gcov/Kconfig | 1 +
kernel/pgo/Kconfig | 3 ++-
3 files changed, 6 insertions(+), 1 deletion(-)
[+Cc Mark]
arm64 is also starting to rely on noinstr working properly.
Sure,
Will, Catalin, other arm64 folks:
Any thoughts on requiring GCC 7.1+/Clang 13.0+ for GCOV support? That
way we can better guarantee that GCOV (and eventually, PGO) don't
touch noinstr functions?
If that's ok, I'll add modify the above like:
+ depends on !ARM64 || (ARM64 && CC_HAS_NO_PROFILE_FN_ATTR)
Wouldn't "!ARM64 || CC_HAS_NO_PROFILE_FN_ATTR" be more succinct?
We need to be able to express via Kconfig "GCOV should not be enabled
for architectures that use noinstr when the toolchain does not support
__attribute__((no_profile_instrument_function))."
Where "architectures that use noinstr" are currently arm64, s390, and
x86. So I guess we could do:
+ depends on !ARM64 || !S390 || !X86 || CC_HAS_NO_PROFILE_FN_ATTR
I think you want:
depends on !(ARM64 || S390 || X86) || CC_HAS_NO_PROFILE_FN_ATTR
(We could add a Kconfig for ARCH_WANTS_NO_INSTR, which might be more
informative than listed out architectures which might be non-obvious
to passers-by).
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-06-22 09:29:50
On Tue, Jun 22, 2021 at 10:25:34AM +0100, Catalin Marinas wrote:
On Mon, Jun 21, 2021 at 01:43:54PM -0700, Nick Desaulniers wrote:
quoted
We need to be able to express via Kconfig "GCOV should not be enabled
for architectures that use noinstr when the toolchain does not support
__attribute__((no_profile_instrument_function))."
Where "architectures that use noinstr" are currently arm64, s390, and
x86. So I guess we could do:
+ depends on !ARM64 || !S390 || !X86 || CC_HAS_NO_PROFILE_FN_ATTR
I think you want:
depends on !(ARM64 || S390 || X86) || CC_HAS_NO_PROFILE_FN_ATTR
quoted
(We could add a Kconfig for ARCH_WANTS_NO_INSTR, which might be more
informative than listed out architectures which might be non-obvious
to passers-by).
On Tue, Jun 22, 2021 at 10:29:37AM +0100, Mark Rutland wrote:
On Tue, Jun 22, 2021 at 10:25:34AM +0100, Catalin Marinas wrote:
quoted
On Mon, Jun 21, 2021 at 01:43:54PM -0700, Nick Desaulniers wrote:
quoted
We need to be able to express via Kconfig "GCOV should not be enabled
for architectures that use noinstr when the toolchain does not support
__attribute__((no_profile_instrument_function))."
Where "architectures that use noinstr" are currently arm64, s390, and
x86. So I guess we could do:
+ depends on !ARM64 || !S390 || !X86 || CC_HAS_NO_PROFILE_FN_ATTR
I think you want:
depends on !(ARM64 || S390 || X86) || CC_HAS_NO_PROFILE_FN_ATTR
quoted
(We could add a Kconfig for ARCH_WANTS_NO_INSTR, which might be more
informative than listed out architectures which might be non-obvious
to passers-by).