Messages in this thread | | | From | Valentin Schneider <> | Subject | Re: [PATCH 2/4] sched: set new prio after checking schedule policy | Date | Thu, 30 Apr 2020 15:18:41 +0100 |
| |
On 30/04/20 15:06, Dietmar Eggemann wrote: >>>> + newprio = NICE_TO_PRIO(attr->sched_nice); >>> >>> This is new, however AFAICT it doesn't change anything for CFS (or about to >>> be) tasks since what matters is calling check_class_changed() further down. >> >> Yes it's only used by rt_effective_prio(). >> > > Looks like changing a SCHED_NORMAL to a SCHED_BATCH task will create a different > queue_flags value. > > # chrt -p $$ > pid 2803's current scheduling policy: SCHED_OTHER > pid 2803's current scheduling priority: 0 > > # chrt -b -p 0 $$ > > ... > [bash 2803] policy=3 oldprio=120 newprio=[99->120] new_effective_prio=[99->120] queue_flags=[0xe->0xa] > [bash 2803] queued=0 running=0 > ... > > But since in this example 'queued=0' it has no further effect here. > > Why is SCHED_NORMAL/SCHED_BATCH (fair_policy()) now treated differently than SCHED_IDLE? > > # chrt -i -p 0 $$ > > ... > [bash 2803] policy=5 newprio=99 oldprio=120 new_effective_prio=99 queue_flags=0xe > [bash 2803] queued=0 running=0 > ...
Good catch; I suppose we'll want to special case SCHED_IDLE (IIRC should map to nice 20).
As you pointed out, right now the newprio computation for CFS tasks is kinda bonkers, so it seems we'll almost always clear DEQUEUE_MOVE from queue_flags for them.
For CFS, not having DEQUEUE_MOVE here would lead to not calling update_min_vruntime() on the dequeue. I'm not sure how much it matters in this one case - I don't expect sched_setscheduler() calls to be *too* frequent, and that oughta be fixed by the next entity_tick()) - but that is an actual change.
| |