Re: [RFC PATCH 00/86] Make the kernel preemptible

[Ankur Arora]

Peter Zijlstra <peterz@xxxxxxxxxxxxx> writes:

> On Tue, Nov 07, 2023 at 01:56:46PM -0800, Ankur Arora wrote:
>> Hi,
>>
>> We have two models of preemption: voluntary and full
>
> 3, also none (RT is not actually a preemption model).

I think we are just using preemption models differently.

I was trying to distinguish between how preemption happens: thus
voluntary (based on voluntary preemption points), and full
(based on preemption state.)

>> (and RT which is
>> a fuller form of full preemption.)
>
> It is not in fact a different preemption model, it is the same full
> preemption, the difference with RT is that it makes a lot more stuff
> preemptible, but the fundamental preemption model is the same -- full.
>
>> In this series -- which is based
>> on Thomas' PoC (see [1]), we try to unify the two by letting the
>> scheduler enforce policy for the voluntary preemption models as well.
>
> Well, you've also taken out preempt_dynamic for some obscure reason :/

Sorry about that :). I didn't mention it because I was using preemption
model in the sense I describe above. And to my mind preempt_dynamic is
a clever mechanism that switches between other preemption models.

>> Please review.
>
>> Ankur Arora (86):
>>   Revert "riscv: support PREEMPT_DYNAMIC with static keys"
>>   Revert "sched/core: Make sched_dynamic_mutex static"
>>   Revert "ftrace: Use preemption model accessors for trace header
>>     printout"
>>   Revert "preempt/dynamic: Introduce preemption model accessors"
>>   Revert "kcsan: Use preemption model accessors"
>>   Revert "entry: Fix compile error in
>>     dynamic_irqentry_exit_cond_resched()"
>>   Revert "livepatch,sched: Add livepatch task switching to
>>     cond_resched()"
>>   Revert "arm64: Support PREEMPT_DYNAMIC"
>>   Revert "sched/preempt: Add PREEMPT_DYNAMIC using static keys"
>>   Revert "sched/preempt: Decouple HAVE_PREEMPT_DYNAMIC from
>>     GENERIC_ENTRY"
>>   Revert "sched/preempt: Simplify irqentry_exit_cond_resched() callers"
>>   Revert "sched/preempt: Refactor sched_dynamic_update()"
>>   Revert "sched/preempt: Move PREEMPT_DYNAMIC logic later"
>>   Revert "preempt/dynamic: Fix setup_preempt_mode() return value"
>>   Revert "preempt: Restore preemption model selection configs"
>>   Revert "sched: Provide Kconfig support for default dynamic preempt
>>     mode"
>>   sched/preempt: remove PREEMPT_DYNAMIC from the build version
>>   Revert "preempt/dynamic: Fix typo in macro conditional statement"
>>   Revert "sched,preempt: Move preempt_dynamic to debug.c"
>>   Revert "static_call: Relax static_call_update() function argument
>>     type"
>>   Revert "sched/core: Use -EINVAL in sched_dynamic_mode()"
>>   Revert "sched/core: Stop using magic values in sched_dynamic_mode()"
>>   Revert "sched,x86: Allow !PREEMPT_DYNAMIC"
>>   Revert "sched: Harden PREEMPT_DYNAMIC"
>>   Revert "sched: Add /debug/sched_preempt"
>>   Revert "preempt/dynamic: Support dynamic preempt with preempt= boot
>>     option"
>>   Revert "preempt/dynamic: Provide irqentry_exit_cond_resched() static
>>     call"
>>   Revert "preempt/dynamic: Provide preempt_schedule[_notrace]() static
>>     calls"
>>   Revert "preempt/dynamic: Provide cond_resched() and might_resched()
>>     static calls"
>>   Revert "preempt: Introduce CONFIG_PREEMPT_DYNAMIC"
>
> NAK
>
> Even if you were to remove PREEMPT_NONE, which should be a separate
> series, but that isn't on the table at all afaict, removing
> preempt_dynamic doesn't make sense.

Agreed. I don't intend to remove PREEMPT_NONE. And, obviously you
do want preempt_dynamic like toggling abilities.

> You still want the preempt= boot time argument and the
> /debug/sched/preempt things to dynamically switch between the models.

Also, yes.

> Please, focus on the voluntary thing, gut that and then replace it with
> the lazy thing, but leave everything else in place.
>
> Re dynamic preempt, gutting the current voluntary preemption model means
> getting rid of the cond_resched and might_resched toggles but you'll
> gain a switch to kill the lazy stuff.

Yes. I think I mostly agree with you.

And, I should have thought this whole revert thing through.
Reverting it wasn't really the plan. The plan was to revert these
patches temporarily, put in the changes you see in this series, and
then pull in the relevant bits of the preempt_dynamic.

Only I decided to push that to later. Sigh.

On your NAK, what about these patches for instance:

>>   Revert "riscv: support PREEMPT_DYNAMIC with static keys"
>>   Revert "livepatch,sched: Add livepatch task switching to
>>     cond_resched()"
>>   Revert "arm64: Support PREEMPT_DYNAMIC"
>>   Revert "sched/preempt: Add PREEMPT_DYNAMIC using static keys"

What's the best way to handle these? With the lazy bit, cond_resched()
and might_resched() are gone. So we don't need all of the static
key inftrastructure for toggling etc.

The part of preempt_dynamic that makes sense to me is the one that
switches dynamically between none/voluntary/full. Here it would need
to be wired onto controls of the lazy bit.
(Right now the preemption policy is controlled by sched_feat in
patches 43, and 44 but sched/preempt is a much better interface.)

--
ankur