Or is the line:
p->prio = effective_prio(p);
in wake_up_new_task() an expensive no op.
As far as I can tell from reading the code, it will always be the case
that EITHER rt_prio(p->prio) is true OR p->prio == p->normal_prio when
this call is made and, in either case, the value of p->prio will be
unchanged. In addition, when this call is made p->normal_prio is
already equal to to normal_prio(p), so the side effects of the function
(setting p->normal_prio) are also unnecessary.
Am I correct or have I missed something?
Peter
--
Peter Williams [email protected]
"Learning, n. The kind of ignorance distinguishing the studious."
-- Ambrose Bierce
On Mon, 2009-09-21 at 23:22 +1000, Peter Williams wrote:
> Or is the line:
>
> p->prio = effective_prio(p);
>
> in wake_up_new_task() an expensive no op.
>
> As far as I can tell from reading the code, it will always be the case
> that EITHER rt_prio(p->prio) is true OR p->prio == p->normal_prio when
> this call is made and, in either case, the value of p->prio will be
> unchanged. In addition, when this call is made p->normal_prio is
> already equal to to normal_prio(p), so the side effects of the function
> (setting p->normal_prio) are also unnecessary.
>
> Am I correct or have I missed something?
Yuck @ all that prio code..
I think you're right, sched_fork() resets the prio, so poking at it in
wake_up_new_task() seems superfluous.
I've been meaning to re-write most of the PI code one of these days, but
so far I've not had time to.
My initial goal is to replace plist with a rb-tree and fix some of the
boost paths to be inside the scheduler. That is, we currently have the
fun situation that we boost a lock owner, which becomes runnable, gets
pushed to another cpu, then current blocks and reschedules, leaving this
cpu to again sort out work.
It would be much easier if we'd first dequeue current, then boost and
then select the owner. Saves a bit of bouncing around.
The rb-tree is needed for things like PI on CFS (yes, you can do a form
of PI on proportional schedulers), and we're going to look at doing a
full sporadic task model deadline scheduler, which needs both deadline
inheritance and bandwidth inheritance.
On 21/09/09 23:53, Peter Zijlstra wrote:
> On Mon, 2009-09-21 at 23:22 +1000, Peter Williams wrote:
>> Or is the line:
>>
>> p->prio = effective_prio(p);
>>
>> in wake_up_new_task() an expensive no op.
>>
>> As far as I can tell from reading the code, it will always be the case
>> that EITHER rt_prio(p->prio) is true OR p->prio == p->normal_prio when
>> this call is made and, in either case, the value of p->prio will be
>> unchanged. In addition, when this call is made p->normal_prio is
>> already equal to to normal_prio(p), so the side effects of the function
>> (setting p->normal_prio) are also unnecessary.
>>
>> Am I correct or have I missed something?
>
> Yuck @ all that prio code..
>
> I think you're right, sched_fork() resets the prio, so poking at it in
> wake_up_new_task() seems superfluous.
After more thought, I also think it would be dangerous if it did
actually change the value from/to a real time priority to/from a non
real time priority as it runs the risk of leaving p with an
inappropriate sched_class if CONFIG_RT_MUTEXES is defined. It seems to
me that if CONFIG_RT_MUTEXES is defined then any changes to a task's
prio field needs to be accompanied by code to ensure the task has the
correct sched_class value as well.
>
> I've been meaning to re-write most of the PI code one of these days, but
> so far I've not had time to.
>
> My initial goal is to replace plist with a rb-tree and fix some of the
> boost paths to be inside the scheduler. That is, we currently have the
> fun situation that we boost a lock owner, which becomes runnable, gets
> pushed to another cpu, then current blocks and reschedules, leaving this
> cpu to again sort out work.
>
> It would be much easier if we'd first dequeue current, then boost and
> then select the owner. Saves a bit of bouncing around.
>
> The rb-tree is needed for things like PI on CFS (yes, you can do a form
> of PI on proportional schedulers), and we're going to look at doing a
> full sporadic task model deadline scheduler, which needs both deadline
> inheritance and bandwidth inheritance.
I think that the __normal_prio(), normal_prio() and effective_prio()
code are inefficient remnants of the old cpu scheduler that missed out
on being cleaned up during the switch to CFS. I think that they can be
cleaned up a bit independently of the changes to the PI code that you
mention. I'll look at it further and see if I can come up with a patch.
Peter
--
Peter Williams [email protected]
"Learning, n. The kind of ignorance distinguishing the studious."
-- Ambrose Bierce
On 21/09/09 23:53, Peter Zijlstra wrote:
> On Mon, 2009-09-21 at 23:22 +1000, Peter Williams wrote:
>> Or is the line:
>>
>> p->prio = effective_prio(p);
>>
>> in wake_up_new_task() an expensive no op.
>>
>> As far as I can tell from reading the code, it will always be the case
>> that EITHER rt_prio(p->prio) is true OR p->prio == p->normal_prio when
>> this call is made and, in either case, the value of p->prio will be
>> unchanged. In addition, when this call is made p->normal_prio is
>> already equal to to normal_prio(p), so the side effects of the function
>> (setting p->normal_prio) are also unnecessary.
The recent sched_reset_on_fork changes to sched_fork() invalidate my
statement (above) about p->normal_prio as they do not set it to the
correct value (in all cases). But it now means that the only useful
work done by the above line in wake_up_new_task() is the side effect of
setting p->normal_prio. I think it would be better if it was set
properly in sched_fork(). I'll send a patch tomorrow.
>>
>> Am I correct or have I missed something?
>
> Yuck @ all that prio code..
>
> I think you're right, sched_fork() resets the prio, so poking at it in
> wake_up_new_task() seems superfluous.
>
> I've been meaning to re-write most of the PI code one of these days, but
> so far I've not had time to.
>
> My initial goal is to replace plist with a rb-tree and fix some of the
> boost paths to be inside the scheduler. That is, we currently have the
> fun situation that we boost a lock owner, which becomes runnable, gets
> pushed to another cpu, then current blocks and reschedules, leaving this
> cpu to again sort out work.
>
> It would be much easier if we'd first dequeue current, then boost and
> then select the owner. Saves a bit of bouncing around.
>
> The rb-tree is needed for things like PI on CFS (yes, you can do a form
> of PI on proportional schedulers), and we're going to look at doing a
> full sporadic task model deadline scheduler, which needs both deadline
> inheritance and bandwidth inheritance.
Peter
--
Peter Williams [email protected]
"Learning, n. The kind of ignorance distinguishing the studious."
-- Ambrose Bierce