linux-kernel - Re: [PATCH v2 13/35] sched: allow runtime config for PREEMPT

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Sat, 08 Jun 2024 17:46:26 -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 13/35] sched: allow runtime config for PREEMPT_AUTO


Peter Zijlstra <peterz@...radead.org> writes:

> On Thu, Jun 06, 2024 at 08:11:41AM -0700, Ankur Arora wrote:
>>
>> Peter Zijlstra <peterz@...radead.org> writes:
>>
>> > On Thu, May 30, 2024 at 02:29:45AM -0700, Ankur Arora wrote:
>> >>
>> >> Peter Zijlstra <peterz@...radead.org> writes:
>> >>
>> >> > On Mon, May 27, 2024 at 05:34:59PM -0700, Ankur Arora wrote:
>> >> >> Reuse sched_dynamic_update() and related logic to enable choosing
>> >> >> the preemption model at boot or runtime for PREEMPT_AUTO.
>> >> >>
>> >> >> The interface is identical to PREEMPT_DYNAMIC.
>> >> >
>> >> > Colour me confused, why?!? What are you doing and why aren't just just
>> >> > adding AUTO to the existing DYNAMIC thing?
>> >>
>> >> You mean have a single __sched_dynamic_update()? AUTO doesn't use any
>> >> of the static_call/static_key stuff so I'm not sure how that would work.
>> >
>> > *sigh*... see the below, seems to work.
>>
>> Sorry, didn't mean for you to have to do all that work to prove the
>> point.
>
> Well, for a large part it was needed for me to figure out what your
> patches were actually doing anyway. Peel away all the layers and this is
> what remains.
>
>> I phrased it badly. I do understand how lazy can be folded in as
>> you do here:
>>
>> > +	case preempt_dynamic_lazy:
>> > +		if (!klp_override)
>> > +			preempt_dynamic_disable(cond_resched);
>> > +		preempt_dynamic_disable(might_resched);
>> > +		preempt_dynamic_enable(preempt_schedule);
>> > +		preempt_dynamic_enable(preempt_schedule_notrace);
>> > +		preempt_dynamic_enable(irqentry_exit_cond_resched);
>> > +		preempt_dynamic_key_enable(preempt_lazy);
>> > +		if (mode != preempt_dynamic_mode)
>> > +			pr_info("Dynamic Preempt: lazy\n");
>> > +		break;
>> >  	}
>>
>> But, if the long term goal (at least as I understand it) is to get rid
>> of cond_resched() -- to allow optimizations that needing to call cond_resched()
>> makes impossible -- does it make sense to pull all of these together?
>
> It certainly doesn't make sense to add yet another configurable thing. We
> have one, so yes add it here.
>
>> Say, eventually preempt_dynamic_lazy and preempt_dynamic_full are the
>> only two models left. Then we will have (modulo figuring out how to
>> switch over klp from cond_resched() to a different unwinding technique):
>>
>> static void __sched_dynamic_update(int mode)
>> {
>>         preempt_dynamic_enable(preempt_schedule);
>>         preempt_dynamic_enable(preempt_schedule_notrace);
>>         preempt_dynamic_enable(irqentry_exit_cond_resched);
>>
>>         switch (mode) {
>>         case preempt_dynamic_full:
>>                 preempt_dynamic_key_disable(preempt_lazy);
>>                 if (mode != preempt_dynamic_mode)
>>                         pr_info("%s: full\n", PREEMPT_MODE);
>>                 break;
>>
>> 	case preempt_dynamic_lazy:
>> 		preempt_dynamic_key_enable(preempt_lazy);
>> 		if (mode != preempt_dynamic_mode)
>> 			pr_info("Dynamic Preempt: lazy\n");
>> 		break;
>>         }
>>
>>         preempt_dynamic_mode = mode;
>> }
>>
>> Which is pretty similar to what the PREEMPT_AUTO code was doing.
>
> Right, but without duplicating all that stuff in the interim.

Yeah, that makes sense. Joel had suggested something on these lines
earlier [1], to which I was resistant.

However, the duplication (and the fact that the voluntary model
was quite thin) should have told me that (AUTO, preempt=voluntary)
should just be folded under PREEMPT_DYNAMIC.

I'll rework the series to do that.

That should also simplify RCU related choices which I think Paul will
like. Given that the lazy model is meant to eventually replace
none/voluntary, so PREEMPT_RCU configuration can just be:

--- a/kernel/rcu/Kconfig
+++ b/kernel/rcu/Kconfig
@@ -18,7 +18,7 @@ config TREE_RCU

 config PREEMPT_RCU
        bool
-       default y if PREEMPTION
+       default y if PREEMPTION && !PREEMPT_LAZY


Or, maybe we should instead have this:

--- a/kernel/rcu/Kconfig
+++ b/kernel/rcu/Kconfig
@@ -18,7 +18,7 @@ config TREE_RCU

 config PREEMPT_RCU
        bool
-       default y if PREEMPTION
+       default y if PREEMPT || PREEMPT_RT
        select TREE_RCU

Though this would be a change in behaviour for current PREEMPT_DYNAMIC
users.

[1] https://lore.kernel.org/lkml/fd48ea5c-bc74-4914-a621-d12c9741c014@joelfernandes.org/

Thanks
--
ankur