Re: [PATCH v2 12/35] sched: separate PREEMPT_DYNAMIC config logic

From: Peter Zijlstra
Date: Tue May 28 2024 - 12:26:39 EST


On Mon, May 27, 2024 at 05:34:58PM -0700, Ankur Arora wrote:
> Pull out the PREEMPT_DYNAMIC setup logic to allow other preemption
> models to dynamically configure preemption.

Uh what ?!? What's the point of creating back-to-back #ifdef sections ?

> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Juri Lelli <juri.lelli@xxxxxxxxxx>
> Cc: Vincent Guittot <vincent.guittot@xxxxxxxxxx>
> Signed-off-by: Ankur Arora <ankur.a.arora@xxxxxxxxxx>
> ---
> kernel/sched/core.c | 165 +++++++++++++++++++++++---------------------
> 1 file changed, 86 insertions(+), 79 deletions(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 0c26b60c1101..349f6257fdcd 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -8713,6 +8713,89 @@ int __cond_resched_rwlock_write(rwlock_t *lock)
> }
> EXPORT_SYMBOL(__cond_resched_rwlock_write);
>
> +#if defined(CONFIG_PREEMPT_DYNAMIC)
> +
> +#define PREEMPT_MODE "Dynamic Preempt"
> +
> +enum {
> + preempt_dynamic_undefined = -1,
> + preempt_dynamic_none,
> + preempt_dynamic_voluntary,
> + preempt_dynamic_full,
> +};
> +
> +int preempt_dynamic_mode = preempt_dynamic_undefined;
> +static DEFINE_MUTEX(sched_dynamic_mutex);
> +
> +int sched_dynamic_mode(const char *str)
> +{
> + if (!strcmp(str, "none"))
> + return preempt_dynamic_none;
> +
> + if (!strcmp(str, "voluntary"))
> + return preempt_dynamic_voluntary;
> +
> + if (!strcmp(str, "full"))
> + return preempt_dynamic_full;
> +
> + return -EINVAL;
> +}
> +
> +static void __sched_dynamic_update(int mode);
> +void sched_dynamic_update(int mode)
> +{
> + mutex_lock(&sched_dynamic_mutex);
> + __sched_dynamic_update(mode);
> + mutex_unlock(&sched_dynamic_mutex);
> +}
> +
> +static void __init preempt_dynamic_init(void)
> +{
> + if (preempt_dynamic_mode == preempt_dynamic_undefined) {
> + if (IS_ENABLED(CONFIG_PREEMPT_NONE)) {
> + sched_dynamic_update(preempt_dynamic_none);
> + } else if (IS_ENABLED(CONFIG_PREEMPT_VOLUNTARY)) {
> + sched_dynamic_update(preempt_dynamic_voluntary);
> + } else {
> + /* Default static call setting, nothing to do */
> + WARN_ON_ONCE(!IS_ENABLED(CONFIG_PREEMPT));
> + preempt_dynamic_mode = preempt_dynamic_full;
> + pr_info("%s: full\n", PREEMPT_MODE);
> + }
> + }
> +}
> +
> +static int __init setup_preempt_mode(char *str)
> +{
> + int mode = sched_dynamic_mode(str);
> + if (mode < 0) {
> + pr_warn("%s: unsupported mode: %s\n", PREEMPT_MODE, str);
> + return 0;
> + }
> +
> + sched_dynamic_update(mode);
> + return 1;
> +}
> +__setup("preempt=", setup_preempt_mode);
> +
> +#define PREEMPT_MODEL_ACCESSOR(mode) \
> + bool preempt_model_##mode(void) \
> + { \
> + WARN_ON_ONCE(preempt_dynamic_mode == preempt_dynamic_undefined); \
> + return preempt_dynamic_mode == preempt_dynamic_##mode; \
> + } \
> + EXPORT_SYMBOL_GPL(preempt_model_##mode)
> +
> +PREEMPT_MODEL_ACCESSOR(none);
> +PREEMPT_MODEL_ACCESSOR(voluntary);
> +PREEMPT_MODEL_ACCESSOR(full);
> +
> +#else /* !CONFIG_PREEMPT_DYNAMIC */
> +
> +static inline void preempt_dynamic_init(void) { }
> +
> +#endif /* !CONFIG_PREEMPT_DYNAMIC */
> +
> #ifdef CONFIG_PREEMPT_DYNAMIC
>
> #ifdef CONFIG_GENERIC_ENTRY
> @@ -8749,29 +8832,6 @@ EXPORT_SYMBOL(__cond_resched_rwlock_write);
> * irqentry_exit_cond_resched <- irqentry_exit_cond_resched
> */
>
> -enum {
> - preempt_dynamic_undefined = -1,
> - preempt_dynamic_none,
> - preempt_dynamic_voluntary,
> - preempt_dynamic_full,
> -};
> -
> -int preempt_dynamic_mode = preempt_dynamic_undefined;
> -
> -int sched_dynamic_mode(const char *str)
> -{
> - if (!strcmp(str, "none"))
> - return preempt_dynamic_none;
> -
> - if (!strcmp(str, "voluntary"))
> - return preempt_dynamic_voluntary;
> -
> - if (!strcmp(str, "full"))
> - return preempt_dynamic_full;
> -
> - return -EINVAL;
> -}
> -
> #if defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
> #define preempt_dynamic_enable(f) static_call_update(f, f##_dynamic_enabled)
> #define preempt_dynamic_disable(f) static_call_update(f, f##_dynamic_disabled)
> @@ -8782,7 +8842,6 @@ int sched_dynamic_mode(const char *str)
> #error "Unsupported PREEMPT_DYNAMIC mechanism"
> #endif
>
> -static DEFINE_MUTEX(sched_dynamic_mutex);
> static bool klp_override;
>
> static void __sched_dynamic_update(int mode)
> @@ -8807,7 +8866,7 @@ static void __sched_dynamic_update(int mode)
> preempt_dynamic_disable(preempt_schedule_notrace);
> preempt_dynamic_disable(irqentry_exit_cond_resched);
> if (mode != preempt_dynamic_mode)
> - pr_info("Dynamic Preempt: none\n");
> + pr_info("%s: none\n", PREEMPT_MODE);
> break;
>
> case preempt_dynamic_voluntary:
> @@ -8818,7 +8877,7 @@ static void __sched_dynamic_update(int mode)
> preempt_dynamic_disable(preempt_schedule_notrace);
> preempt_dynamic_disable(irqentry_exit_cond_resched);
> if (mode != preempt_dynamic_mode)
> - pr_info("Dynamic Preempt: voluntary\n");
> + pr_info("%s: voluntary\n", PREEMPT_MODE);
> break;
>
> case preempt_dynamic_full:
> @@ -8829,20 +8888,13 @@ static void __sched_dynamic_update(int mode)
> preempt_dynamic_enable(preempt_schedule_notrace);
> preempt_dynamic_enable(irqentry_exit_cond_resched);
> if (mode != preempt_dynamic_mode)
> - pr_info("Dynamic Preempt: full\n");
> + pr_info("%s: full\n", PREEMPT_MODE);
> break;
> }
>
> preempt_dynamic_mode = mode;
> }
>
> -void sched_dynamic_update(int mode)
> -{
> - mutex_lock(&sched_dynamic_mutex);
> - __sched_dynamic_update(mode);
> - mutex_unlock(&sched_dynamic_mutex);
> -}
> -
> #ifdef CONFIG_HAVE_PREEMPT_DYNAMIC_CALL
>
> static int klp_cond_resched(void)
> @@ -8873,51 +8925,6 @@ void sched_dynamic_klp_disable(void)
>
> #endif /* CONFIG_HAVE_PREEMPT_DYNAMIC_CALL */
>
> -static int __init setup_preempt_mode(char *str)
> -{
> - int mode = sched_dynamic_mode(str);
> - if (mode < 0) {
> - pr_warn("Dynamic Preempt: unsupported mode: %s\n", str);
> - return 0;
> - }
> -
> - sched_dynamic_update(mode);
> - return 1;
> -}
> -__setup("preempt=", setup_preempt_mode);
> -
> -static void __init preempt_dynamic_init(void)
> -{
> - if (preempt_dynamic_mode == preempt_dynamic_undefined) {
> - if (IS_ENABLED(CONFIG_PREEMPT_NONE)) {
> - sched_dynamic_update(preempt_dynamic_none);
> - } else if (IS_ENABLED(CONFIG_PREEMPT_VOLUNTARY)) {
> - sched_dynamic_update(preempt_dynamic_voluntary);
> - } else {
> - /* Default static call setting, nothing to do */
> - WARN_ON_ONCE(!IS_ENABLED(CONFIG_PREEMPT));
> - preempt_dynamic_mode = preempt_dynamic_full;
> - pr_info("Dynamic Preempt: full\n");
> - }
> - }
> -}
> -
> -#define PREEMPT_MODEL_ACCESSOR(mode) \
> - bool preempt_model_##mode(void) \
> - { \
> - WARN_ON_ONCE(preempt_dynamic_mode == preempt_dynamic_undefined); \
> - return preempt_dynamic_mode == preempt_dynamic_##mode; \
> - } \
> - EXPORT_SYMBOL_GPL(preempt_model_##mode)
> -
> -PREEMPT_MODEL_ACCESSOR(none);
> -PREEMPT_MODEL_ACCESSOR(voluntary);
> -PREEMPT_MODEL_ACCESSOR(full);
> -
> -#else /* !CONFIG_PREEMPT_DYNAMIC */
> -
> -static inline void preempt_dynamic_init(void) { }
> -
> #endif /* #ifdef CONFIG_PREEMPT_DYNAMIC */
>
> /**
> --
> 2.31.1
>