All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] sched/features: Fix !CONFIG_JUMP_LABEL case
@ 2020-10-13  5:31 Juri Lelli
  2020-10-13  8:26 ` Patrick Bellasi
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Juri Lelli @ 2020-10-13  5:31 UTC (permalink / raw)
  To: peterz, mingo
  Cc: linux-kernel, bristot, patrick.bellasi, vincent.guittot,
	dietmar.eggemann, chris.redpath, rostedt, bsegall, mgorman,
	valentin.schneider, Juri Lelli

Commit 765cc3a4b224e ("sched/core: Optimize sched_feat() for
!CONFIG_SCHED_DEBUG builds") made sched features static for
!CONFIG_SCHED_DEBUG configurations, but overlooked the CONFIG_
SCHED_DEBUG enabled and !CONFIG_JUMP_LABEL cases. For the latter echoing
changes to /sys/kernel/debug/sched_features has the nasty effect of
effectively changing what sched_features reports, but without actually
changing the scheduler behaviour (since different translation units get
different sysctl_sched_features).

Fix CONFIG_SCHED_DEBUG and !CONFIG_JUMP_LABEL configurations by properly
restructuring ifdefs.

Fixes: 765cc3a4b224e ("sched/core: Optimize sched_feat() for !CONFIG_SCHED_DEBUG builds")
Co-developed-by: Daniel Bristot de Oliveira <bristot@redhat.com>
Signed-off-by: Daniel Bristot de Oliveira <bristot@redhat.com>
Signed-off-by: Juri Lelli <juri.lelli@redhat.com>

---
v1->v2
 - use CONFIG_JUMP_LABEL (and not the old HAVE_JUMP_LABEL) [Valentin]
---
 kernel/sched/core.c  |  2 +-
 kernel/sched/sched.h | 13 ++++++++++---
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 3dc415f58bd7..a7949e3ed7e7 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -44,7 +44,7 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(sched_update_nr_running_tp);
 
 DEFINE_PER_CPU_SHARED_ALIGNED(struct rq, runqueues);
 
-#if defined(CONFIG_SCHED_DEBUG) && defined(CONFIG_JUMP_LABEL)
+#ifdef CONFIG_SCHED_DEBUG
 /*
  * Debugging: various feature bits
  *
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 28709f6b0975..8d1ca65db3b0 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1629,7 +1629,7 @@ enum {
 
 #undef SCHED_FEAT
 
-#if defined(CONFIG_SCHED_DEBUG) && defined(CONFIG_JUMP_LABEL)
+#ifdef CONFIG_SCHED_DEBUG
 
 /*
  * To support run-time toggling of sched features, all the translation units
@@ -1637,6 +1637,7 @@ enum {
  */
 extern const_debug unsigned int sysctl_sched_features;
 
+#ifdef CONFIG_JUMP_LABEL
 #define SCHED_FEAT(name, enabled)					\
 static __always_inline bool static_branch_##name(struct static_key *key) \
 {									\
@@ -1649,7 +1650,13 @@ static __always_inline bool static_branch_##name(struct static_key *key) \
 extern struct static_key sched_feat_keys[__SCHED_FEAT_NR];
 #define sched_feat(x) (static_branch_##x(&sched_feat_keys[__SCHED_FEAT_##x]))
 
-#else /* !(SCHED_DEBUG && CONFIG_JUMP_LABEL) */
+#else /* !CONFIG_JUMP_LABEL */
+
+#define sched_feat(x) (sysctl_sched_features & (1UL << __SCHED_FEAT_##x))
+
+#endif /* CONFIG_JUMP_LABEL */
+
+#else /* !SCHED_DEBUG */
 
 /*
  * Each translation unit has its own copy of sysctl_sched_features to allow
@@ -1665,7 +1672,7 @@ static const_debug __maybe_unused unsigned int sysctl_sched_features =
 
 #define sched_feat(x) !!(sysctl_sched_features & (1UL << __SCHED_FEAT_##x))
 
-#endif /* SCHED_DEBUG && CONFIG_JUMP_LABEL */
+#endif /* SCHED_DEBUG */
 
 extern struct static_key_false sched_numa_balancing;
 extern struct static_key_false sched_schedstats;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] sched/features: Fix !CONFIG_JUMP_LABEL case
  2020-10-13  5:31 [PATCH v2] sched/features: Fix !CONFIG_JUMP_LABEL case Juri Lelli
@ 2020-10-13  8:26 ` Patrick Bellasi
  2020-10-13 10:48   ` Juri Lelli
  2020-10-13 10:02 ` [tip: sched/urgent] " tip-bot2 for Juri Lelli
  2020-10-14 17:58 ` tip-bot2 for Juri Lelli
  2 siblings, 1 reply; 5+ messages in thread
From: Patrick Bellasi @ 2020-10-13  8:26 UTC (permalink / raw)
  To: Juri Lelli
  Cc: peterz, mingo, linux-kernel, bristot, vincent.guittot,
	dietmar.eggemann, chris.redpath, rostedt, bsegall, mgorman,
	valentin.schneider


On Tue, Oct 13, 2020 at 07:31:14 +0200, Juri Lelli <juri.lelli@redhat.com> wrote...

> Commit 765cc3a4b224e ("sched/core: Optimize sched_feat() for
> !CONFIG_SCHED_DEBUG builds") made sched features static for
> !CONFIG_SCHED_DEBUG configurations, but overlooked the CONFIG_
> SCHED_DEBUG enabled and !CONFIG_JUMP_LABEL cases. For the latter echoing
> changes to /sys/kernel/debug/sched_features has the nasty effect of
> effectively changing what sched_features reports, but without actually
> changing the scheduler behaviour (since different translation units get
> different sysctl_sched_features).

Hops, yes, I think I missed to properly check that config :/
Good spot!

> Fix CONFIG_SCHED_DEBUG and !CONFIG_JUMP_LABEL configurations by properly
> restructuring ifdefs.
>
> Fixes: 765cc3a4b224e ("sched/core: Optimize sched_feat() for !CONFIG_SCHED_DEBUG builds")
> Co-developed-by: Daniel Bristot de Oliveira <bristot@redhat.com>
> Signed-off-by: Daniel Bristot de Oliveira <bristot@redhat.com>
> Signed-off-by: Juri Lelli <juri.lelli@redhat.com>

(did you get some wrong formatting for the changelog above?)

> ---
> v1->v2
>  - use CONFIG_JUMP_LABEL (and not the old HAVE_JUMP_LABEL) [Valentin]
> ---
>  kernel/sched/core.c  |  2 +-
>  kernel/sched/sched.h | 13 ++++++++++---
>  2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 3dc415f58bd7..a7949e3ed7e7 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -44,7 +44,7 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(sched_update_nr_running_tp);
>  
>  DEFINE_PER_CPU_SHARED_ALIGNED(struct rq, runqueues);
>  
> -#if defined(CONFIG_SCHED_DEBUG) && defined(CONFIG_JUMP_LABEL)
> +#ifdef CONFIG_SCHED_DEBUG
>  /*
>   * Debugging: various feature bits
>   *
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 28709f6b0975..8d1ca65db3b0 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -1629,7 +1629,7 @@ enum {
>  
>  #undef SCHED_FEAT
>  
> -#if defined(CONFIG_SCHED_DEBUG) && defined(CONFIG_JUMP_LABEL)
> +#ifdef CONFIG_SCHED_DEBUG
>  
>  /*
>   * To support run-time toggling of sched features, all the translation units
> @@ -1637,6 +1637,7 @@ enum {
>   */
>  extern const_debug unsigned int sysctl_sched_features;
>  
> +#ifdef CONFIG_JUMP_LABEL
>  #define SCHED_FEAT(name, enabled)					\
>  static __always_inline bool static_branch_##name(struct static_key *key) \
>  {									\
> @@ -1649,7 +1650,13 @@ static __always_inline bool static_branch_##name(struct static_key *key) \
>  extern struct static_key sched_feat_keys[__SCHED_FEAT_NR];
>  #define sched_feat(x) (static_branch_##x(&sched_feat_keys[__SCHED_FEAT_##x]))
>  
> -#else /* !(SCHED_DEBUG && CONFIG_JUMP_LABEL) */
> +#else /* !CONFIG_JUMP_LABEL */
> +
> +#define sched_feat(x) (sysctl_sched_features & (1UL << __SCHED_FEAT_##x))
> +
> +#endif /* CONFIG_JUMP_LABEL */
> +
> +#else /* !SCHED_DEBUG */
>  
>  /*
>   * Each translation unit has its own copy of sysctl_sched_features to allow
> @@ -1665,7 +1672,7 @@ static const_debug __maybe_unused unsigned int sysctl_sched_features =
>  
>  #define sched_feat(x) !!(sysctl_sched_features & (1UL << __SCHED_FEAT_##x))
>  
> -#endif /* SCHED_DEBUG && CONFIG_JUMP_LABEL */
> +#endif /* SCHED_DEBUG */
>  
>  extern struct static_key_false sched_numa_balancing;
>  extern struct static_key_false sched_schedstats;


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tip: sched/urgent] sched/features: Fix !CONFIG_JUMP_LABEL case
  2020-10-13  5:31 [PATCH v2] sched/features: Fix !CONFIG_JUMP_LABEL case Juri Lelli
  2020-10-13  8:26 ` Patrick Bellasi
@ 2020-10-13 10:02 ` tip-bot2 for Juri Lelli
  2020-10-14 17:58 ` tip-bot2 for Juri Lelli
  2 siblings, 0 replies; 5+ messages in thread
From: tip-bot2 for Juri Lelli @ 2020-10-13 10:02 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Daniel Bristot de Oliveira, Juri Lelli, Ingo Molnar,
	Patrick Bellasi, x86, LKML

The following commit has been merged into the sched/urgent branch of tip:

Commit-ID:     da912c29a4a552588cbfa895487d9d5523b6faa7
Gitweb:        https://git.kernel.org/tip/da912c29a4a552588cbfa895487d9d5523b6faa7
Author:        Juri Lelli <juri.lelli@redhat.com>
AuthorDate:    Tue, 13 Oct 2020 07:31:14 +02:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Tue, 13 Oct 2020 11:33:08 +02:00

sched/features: Fix !CONFIG_JUMP_LABEL case

Commit:

  765cc3a4b224e ("sched/core: Optimize sched_feat() for !CONFIG_SCHED_DEBUG builds")

made sched features static for !CONFIG_SCHED_DEBUG configurations, but
overlooked the CONFIG_SCHED_DEBUG=y and !CONFIG_JUMP_LABEL cases.

For the latter echoing changes to /sys/kernel/debug/sched_features has
the nasty effect of effectively changing what sched_features reports,
but without actually changing the scheduler behaviour (since different
translation units get different sysctl_sched_features).

Fix CONFIG_SCHED_DEBUG=y and !CONFIG_JUMP_LABEL configurations by properly
restructuring ifdefs.

Fixes: 765cc3a4b224e ("sched/core: Optimize sched_feat() for !CONFIG_SCHED_DEBUG builds")
Co-developed-by: Daniel Bristot de Oliveira <bristot@redhat.com>
Signed-off-by: Daniel Bristot de Oliveira <bristot@redhat.com>
Signed-off-by: Juri Lelli <juri.lelli@redhat.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Acked-by: Patrick Bellasi <patrick.bellasi@matbug.net>
Link: https://lore.kernel.org/r/20201013053114.160628-1-juri.lelli@redhat.com
---
 kernel/sched/core.c  |  2 +-
 kernel/sched/sched.h | 13 ++++++++++---
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 8160ab5..d2003a7 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -44,7 +44,7 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(sched_update_nr_running_tp);
 
 DEFINE_PER_CPU_SHARED_ALIGNED(struct rq, runqueues);
 
-#if defined(CONFIG_SCHED_DEBUG) && defined(CONFIG_JUMP_LABEL)
+#ifdef CONFIG_SCHED_DEBUG
 /*
  * Debugging: various feature bits
  *
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 28709f6..8d1ca65 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1629,7 +1629,7 @@ enum {
 
 #undef SCHED_FEAT
 
-#if defined(CONFIG_SCHED_DEBUG) && defined(CONFIG_JUMP_LABEL)
+#ifdef CONFIG_SCHED_DEBUG
 
 /*
  * To support run-time toggling of sched features, all the translation units
@@ -1637,6 +1637,7 @@ enum {
  */
 extern const_debug unsigned int sysctl_sched_features;
 
+#ifdef CONFIG_JUMP_LABEL
 #define SCHED_FEAT(name, enabled)					\
 static __always_inline bool static_branch_##name(struct static_key *key) \
 {									\
@@ -1649,7 +1650,13 @@ static __always_inline bool static_branch_##name(struct static_key *key) \
 extern struct static_key sched_feat_keys[__SCHED_FEAT_NR];
 #define sched_feat(x) (static_branch_##x(&sched_feat_keys[__SCHED_FEAT_##x]))
 
-#else /* !(SCHED_DEBUG && CONFIG_JUMP_LABEL) */
+#else /* !CONFIG_JUMP_LABEL */
+
+#define sched_feat(x) (sysctl_sched_features & (1UL << __SCHED_FEAT_##x))
+
+#endif /* CONFIG_JUMP_LABEL */
+
+#else /* !SCHED_DEBUG */
 
 /*
  * Each translation unit has its own copy of sysctl_sched_features to allow
@@ -1665,7 +1672,7 @@ static const_debug __maybe_unused unsigned int sysctl_sched_features =
 
 #define sched_feat(x) !!(sysctl_sched_features & (1UL << __SCHED_FEAT_##x))
 
-#endif /* SCHED_DEBUG && CONFIG_JUMP_LABEL */
+#endif /* SCHED_DEBUG */
 
 extern struct static_key_false sched_numa_balancing;
 extern struct static_key_false sched_schedstats;

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] sched/features: Fix !CONFIG_JUMP_LABEL case
  2020-10-13  8:26 ` Patrick Bellasi
@ 2020-10-13 10:48   ` Juri Lelli
  0 siblings, 0 replies; 5+ messages in thread
From: Juri Lelli @ 2020-10-13 10:48 UTC (permalink / raw)
  To: Patrick Bellasi
  Cc: peterz, mingo, linux-kernel, bristot, vincent.guittot,
	dietmar.eggemann, chris.redpath, rostedt, bsegall, mgorman,
	valentin.schneider

On 13/10/20 10:26, Patrick Bellasi wrote:
> 
> On Tue, Oct 13, 2020 at 07:31:14 +0200, Juri Lelli <juri.lelli@redhat.com> wrote...
> 
> > Commit 765cc3a4b224e ("sched/core: Optimize sched_feat() for
> > !CONFIG_SCHED_DEBUG builds") made sched features static for
> > !CONFIG_SCHED_DEBUG configurations, but overlooked the CONFIG_
> > SCHED_DEBUG enabled and !CONFIG_JUMP_LABEL cases. For the latter echoing
> > changes to /sys/kernel/debug/sched_features has the nasty effect of
> > effectively changing what sched_features reports, but without actually
> > changing the scheduler behaviour (since different translation units get
> > different sysctl_sched_features).
> 
> Hops, yes, I think I missed to properly check that config :/
> Good spot!
> 
> > Fix CONFIG_SCHED_DEBUG and !CONFIG_JUMP_LABEL configurations by properly
> > restructuring ifdefs.
> >
> > Fixes: 765cc3a4b224e ("sched/core: Optimize sched_feat() for !CONFIG_SCHED_DEBUG builds")
> > Co-developed-by: Daniel Bristot de Oliveira <bristot@redhat.com>
> > Signed-off-by: Daniel Bristot de Oliveira <bristot@redhat.com>
> > Signed-off-by: Juri Lelli <juri.lelli@redhat.com>
> 
> (did you get some wrong formatting for the changelog above?)

Hummm, what you mean?

I intended to follow

https://elixir.bootlin.com/linux/latest/source/Documentation/process/submitting-patches.rst#L566


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tip: sched/urgent] sched/features: Fix !CONFIG_JUMP_LABEL case
  2020-10-13  5:31 [PATCH v2] sched/features: Fix !CONFIG_JUMP_LABEL case Juri Lelli
  2020-10-13  8:26 ` Patrick Bellasi
  2020-10-13 10:02 ` [tip: sched/urgent] " tip-bot2 for Juri Lelli
@ 2020-10-14 17:58 ` tip-bot2 for Juri Lelli
  2 siblings, 0 replies; 5+ messages in thread
From: tip-bot2 for Juri Lelli @ 2020-10-14 17:58 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Daniel Bristot de Oliveira, Juri Lelli, Ingo Molnar,
	Patrick Bellasi, Valentin Schneider, x86, LKML

The following commit has been merged into the sched/urgent branch of tip:

Commit-ID:     a73f863af4ce9730795eab7097fb2102e6854365
Gitweb:        https://git.kernel.org/tip/a73f863af4ce9730795eab7097fb2102e6854365
Author:        Juri Lelli <juri.lelli@redhat.com>
AuthorDate:    Tue, 13 Oct 2020 07:31:14 +02:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 14 Oct 2020 19:55:46 +02:00

sched/features: Fix !CONFIG_JUMP_LABEL case

Commit:

  765cc3a4b224e ("sched/core: Optimize sched_feat() for !CONFIG_SCHED_DEBUG builds")

made sched features static for !CONFIG_SCHED_DEBUG configurations, but
overlooked the CONFIG_SCHED_DEBUG=y and !CONFIG_JUMP_LABEL cases.

For the latter echoing changes to /sys/kernel/debug/sched_features has
the nasty effect of effectively changing what sched_features reports,
but without actually changing the scheduler behaviour (since different
translation units get different sysctl_sched_features).

Fix CONFIG_SCHED_DEBUG=y and !CONFIG_JUMP_LABEL configurations by properly
restructuring ifdefs.

Fixes: 765cc3a4b224e ("sched/core: Optimize sched_feat() for !CONFIG_SCHED_DEBUG builds")
Co-developed-by: Daniel Bristot de Oliveira <bristot@redhat.com>
Signed-off-by: Daniel Bristot de Oliveira <bristot@redhat.com>
Signed-off-by: Juri Lelli <juri.lelli@redhat.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Acked-by: Patrick Bellasi <patrick.bellasi@matbug.net>
Reviewed-by: Valentin Schneider <valentin.schneider@arm.com>
Link: https://lore.kernel.org/r/20201013053114.160628-1-juri.lelli@redhat.com
---
 kernel/sched/core.c  |  2 +-
 kernel/sched/sched.h | 13 ++++++++++---
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 8160ab5..d2003a7 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -44,7 +44,7 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(sched_update_nr_running_tp);
 
 DEFINE_PER_CPU_SHARED_ALIGNED(struct rq, runqueues);
 
-#if defined(CONFIG_SCHED_DEBUG) && defined(CONFIG_JUMP_LABEL)
+#ifdef CONFIG_SCHED_DEBUG
 /*
  * Debugging: various feature bits
  *
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 648f023..df80bfc 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1629,7 +1629,7 @@ enum {
 
 #undef SCHED_FEAT
 
-#if defined(CONFIG_SCHED_DEBUG) && defined(CONFIG_JUMP_LABEL)
+#ifdef CONFIG_SCHED_DEBUG
 
 /*
  * To support run-time toggling of sched features, all the translation units
@@ -1637,6 +1637,7 @@ enum {
  */
 extern const_debug unsigned int sysctl_sched_features;
 
+#ifdef CONFIG_JUMP_LABEL
 #define SCHED_FEAT(name, enabled)					\
 static __always_inline bool static_branch_##name(struct static_key *key) \
 {									\
@@ -1649,7 +1650,13 @@ static __always_inline bool static_branch_##name(struct static_key *key) \
 extern struct static_key sched_feat_keys[__SCHED_FEAT_NR];
 #define sched_feat(x) (static_branch_##x(&sched_feat_keys[__SCHED_FEAT_##x]))
 
-#else /* !(SCHED_DEBUG && CONFIG_JUMP_LABEL) */
+#else /* !CONFIG_JUMP_LABEL */
+
+#define sched_feat(x) (sysctl_sched_features & (1UL << __SCHED_FEAT_##x))
+
+#endif /* CONFIG_JUMP_LABEL */
+
+#else /* !SCHED_DEBUG */
 
 /*
  * Each translation unit has its own copy of sysctl_sched_features to allow
@@ -1665,7 +1672,7 @@ static const_debug __maybe_unused unsigned int sysctl_sched_features =
 
 #define sched_feat(x) !!(sysctl_sched_features & (1UL << __SCHED_FEAT_##x))
 
-#endif /* SCHED_DEBUG && CONFIG_JUMP_LABEL */
+#endif /* SCHED_DEBUG */
 
 extern struct static_key_false sched_numa_balancing;
 extern struct static_key_false sched_schedstats;

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-10-14 17:59 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-13  5:31 [PATCH v2] sched/features: Fix !CONFIG_JUMP_LABEL case Juri Lelli
2020-10-13  8:26 ` Patrick Bellasi
2020-10-13 10:48   ` Juri Lelli
2020-10-13 10:02 ` [tip: sched/urgent] " tip-bot2 for Juri Lelli
2020-10-14 17:58 ` tip-bot2 for Juri Lelli

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.