Re: [PATCH] [sched] redundant reschedule when set_user_nice() boosts a prio of a task from the "expired" array

["Dmitry Adamushko" <dmitry.adamushko@gmail.com>]

On 07/04/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Wed, 4 Apr 2007 22:05:40 +0200 "Dmitry Adamushko"
> > [...]
> >
> > o  Make TASK_PREEMPTS_CURR(task, rq) return "true" only if the task's
> > prio is higher than the current's one and the task is in the "active"
> > array.
> > This ensures we don't make redundant resched_task() calls when the
> > task is in the "expired" array (as may happen now in set_user_prio(),
> > rt_mutex_setprio() and pull_task() ) ;
> >
> > o  generilise conditions for a call to resched_task() in
> > set_user_nice(), rt_mutex_setprio() and sched_setscheduler()
> >
>
> grief.  This patch conflicts seriously with the staircase scheduler in -mm.
> So to merge it I need to
>
> - apply it
> - then apply a revert-it-again patch
> - then apply staircase
> - then ask Con to cook up a staircase-based equivalent of your change.

I'll make a SD-based version and send it to Con.


> so
>
> - your code only gets publically tested in its against-staircase version
>
> - the against-mainline version will get merged without having been
>   publically tested outside of staircase
>
> which is probably all OK for a 2.6.22-rc1 thing, provided Ingo can give a
> confident ack.

Ok, thanks.

btw, just out of curiosity. The very first approach I was thinking of
- was to move a task from the "expired" to the "active" array when its
priority is boosted (like rt_mutex_setprio() does for rt tasks).

Reasoning: getting a higher static_prio means getting an additional
quota of timeslice which still could be used during this rotation.

delta = task_timeslice(p->static_prio) - task_timeslice(old_static_prio)

Aha.. /here I'm looking at the mainline now/ another funny thing is
that a time_slice is not immediately affected by the change of
static_prio in set_user_nice(). If a task is in the expired array, it
will run the next rotation with the *old* time_slice (i.e. calculated
in task_running_tick() before putting the task in the expired array
and based on the *old* static_prio).
In theory, set_user_nice() could adjust a p->time_slice with "delta"
being calculated as shown above.. But ok, it's not more than a minor
inconsistency (of course, if I'm not missing something).


>
> > --- linux-2.6.21-rc5/kernel/sched-orig.c        2007-04-04
> > 18:26:19.000000000 +0200
> > +++ linux-2.6.21-rc5/kernel/sched.c     2007-04-04 18:26:43.000000000 +0200
> > @@ -168,7 +168,7 @@ unsigned long long __attribute__((weak))
> >                 (MAX_BONUS / 2 + DELTA((p)) + 1) / MAX_BONUS - 1))
> >
> >  #define TASK_PREEMPTS_CURR(p, rq) \
> > -       ((p)->prio < (rq)->curr->prio)
> > +       (((p)->prio < (rq)->curr->prio) && ((p)->array == (rq)->active))
>
> Your patch was wordwrapped and had its tabs replaced with spaces.  Please
> fix your email client.

I apologize for this. Will fix.


-- 
Best regards,
Dmitry Adamushko