| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <87r0dj640e.fsf@oracle.com>
Date: Thu, 30 May 2024 02:30:25 -0700
From: Ankur Arora <ankur.a.arora@...cle.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: Ankur Arora <ankur.a.arora@...cle.com>, linux-kernel@...r.kernel.org,
tglx@...utronix.de, torvalds@...ux-foundation.org, paulmck@...nel.org,
rostedt@...dmis.org, mark.rutland@....com, juri.lelli@...hat.com,
joel@...lfernandes.org, raghavendra.kt@....com, sshegde@...ux.ibm.com,
boris.ostrovsky@...cle.com, konrad.wilk@...cle.com,
Ingo Molnar
<mingo@...hat.com>,
Vincent Guittot <vincent.guittot@...aro.org>
Subject: Re: [PATCH v2 12/35] sched: separate PREEMPT_DYNAMIC config logic
Peter Zijlstra <peterz@...radead.org> writes:
> 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 ?
Now that you mention it, it does seem quite odd.
Assuming I keep the separation maybe it makes sense to make the runtime
configuration it's own configuration option, say CONFIG_PREEMPT_RUNTIME.
And, PREEMPT_AUTO and PREEMPT_DYNAMIC could select it?
>> Cc: Ingo Molnar <mingo@...hat.com>
>> Cc: Peter Zijlstra <peterz@...radead.org>
>> Cc: Juri Lelli <juri.lelli@...hat.com>
>> Cc: Vincent Guittot <vincent.guittot@...aro.org>
>> Signed-off-by: Ankur Arora <ankur.a.arora@...cle.com>
>> ---
>> 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
>>
--
ankur
Powered by blists - more mailing lists