Messages in this thread | ![/](/images/icornerl.gif) | | From | Ankur Arora <> | Subject | Re: [PATCH 26/30] sched: handle preempt=voluntary under PREEMPT_AUTO | Date | Tue, 05 Mar 2024 00:11:28 -0800 |
| |
Joel Fernandes <joel@joelfernandes.org> writes:
> Hi Anukr, > > On Mon, Feb 12, 2024 at 09:55:50PM -0800, Ankur Arora wrote: >> The default preemption policy for voluntary preemption under >> PREEMPT_AUTO is to schedule eagerly for tasks of higher scheduling >> class, and lazily for well-behaved, non-idle tasks. >> >> This is the same policy as preempt=none, with an eager handling of >> higher priority scheduling classes. > > AFAICS, the meaning of the word 'voluntary' has changed versus the old > CONFIG_PREEMPT_VOLUNTARY, with this patch. > > So the word voluntary does not completely make sense in this context. What is > VOLUNTARY about choosing a higher scheduling class? > > For instance, even in the same scheduling class, there is a notion of higher > priority, not just between classes. Example, higher RT priority within RT, or > earlier deadline within EEVDF (formerly CFS).
Agreed. The higher scheduling class line is pretty fuzzy and after the discussion with Juri, almost non existent: https://lore.kernel.org/lkml/ZeBPXNFkipU9yytp@localhost.localdomain/.
> IMO, just kill 'voluntary' if PREEMPT_AUTO is enabled. There is no > 'voluntary' business because > 1. The behavior vs =none is to allow higher scheduling class to preempt, it > is not about the old voluntary.
What do you think about folding the higher scheduling class preemption logic into preempt=none? As Juri pointed out, prioritization of at least the leftmost deadline task needs to be done for correctness.
(That'll get rid of the current preempt=voluntary model, at least until there's a separate use for it.)
> 2. you are also planning to remove cond_resched()s via this series and leave > it to the scheduler right?
Yeah, under PREEMPT_AUTO, cond_resched() will /almost/ be not there. Gets defined to:
static inline int _cond_resched(void) { klp_sched_try_switch(); return 0; }
Right now, we need cond_resched() to make timely forward progress while doing live-patching.
> Or call it preempt=higher, or something? No one is going to understand the > meaning of voluntary the way it is implied here IMHO.
I don't think there's enough to make it worth adding a new model. For now I'm tending towards moving the correctness parts to preempt=none and making preempt=voluntary identical to preempt=none.
Thanks for the review.
-- ankur
| ![\](/images/icornerr.gif) |