Assumably a BUG in Linux Kernel (scheduler part)
Andrey Gelman
Assumably, I've discovered a bug in Linux kernel (version 2.6.16), at:
kernel\sched.c function set_user_nice()
Problem description:
After you execute nice() system call, the dynamic priority is set to the new
value of the static priority, instead of being adjusted by a difference
between new and old nice values.
In other words:
What you have before you execute nice(): p->prio == static_prio - bonus
After you execute nice(): p->prio == "a new" static_prio (no bonus)
BUG Fix:
Here I paste the whole of function set_user_nice() (thanks god, it's short)
with the BUG highlighted and fixed:
void set_user_nice(task_t *p, long nice)
{
unsigned long flags;
prio_array_t *array;
runqueue_t *rq;
int old_prio, new_prio, delta;
if (TASK_NICE(p) == nice || nice < -20 || nice > 19)
return;
/*
* We have to be careful, if called from sys_setpriority(),
* the task might be in the middle of scheduling on another CPU.
*/
rq = task_rq_lock(p, &flags);
/*
* The RT priorities are set via sched_setscheduler(), but we still
* allow the 'normal' nice value to be set - but as expected
* it wont have any effect on scheduling until the task is
* not SCHED_NORMAL/SCHED_BATCH:
*/
if (rt_task(p)) {
p->static_prio = NICE_TO_PRIO(nice);
goto out_unlock;
}
array = p->array;
if (array)
dequeue_task(p, array);
//-------------------------------------------------
/*
//BUGGED FORMULA : 5 lines
old_prio = p->prio;
new_prio = NICE_TO_PRIO(nice);
delta = new_prio - old_prio;
p->static_prio = NICE_TO_PRIO(nice);
p->prio += delta;
*/
//BUG FIX : 5 lines
old_prio = p->static_prio;
new_prio = NICE_TO_PRIO(nice);
delta = new_prio - old_prio;
p->static_prio = new_prio;
p->prio += delta;
//-------------------------------------------------
if (array) {
enqueue_task(p, array);
/*
* If the task increased its priority or is running and
* lowered its priority, then reschedule its CPU:
*/
if (delta < 0 || (delta > 0 && task_running(rq, p)))
resched_task(rq->curr);
}
out_unlock:
task_rq_unlock(rq, &flags);
}
Thank you,
Andrey Gelman,
Haifa, ISRAEL
9-Jun-2006
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Ingo Molnar
* Andrey Gelman <age...@012.net.il> wrote:
> Hello there !
> Assumably, I've discovered a bug in Linux kernel (version 2.6.16), at:
> kernel\sched.c function set_user_nice()
[...]
> //-------------------------------------------------
> /*
> //BUGGED FORMULA : 5 lines
> old_prio = p->prio;
> new_prio = NICE_TO_PRIO(nice);
> delta = new_prio - old_prio;
> p->static_prio = NICE_TO_PRIO(nice);
> p->prio += delta;
> */
> //BUG FIX : 5 lines
> old_prio = p->static_prio;
> new_prio = NICE_TO_PRIO(nice);
> delta = new_prio - old_prio;
> p->static_prio = new_prio;
> p->prio += delta;
> //-------------------------------------------------
you are right, this is a bug in the scheduler - good find.
I did accidentally fix it months ago via one of the PI-scheduling
cleanups, and thus the fix is in the current -mm tree and is scheduled
for 2.6.18 inclusion:
(see the adding of effective_prio() to set_user_nice(), instead of an
open-coded calculation of the priority)
But i did not realize that this also fixed a bug. The effects of the bug
are minor: a user can renice only 19 times (to go from 0 to +19), so
there's a finite amount of extra timeslices a CPU hog might win due to
this. I'd not include the effective_prio() change in 2.6.17 - it touches
quite some code and we are close to the release of 2.6.17. Nevertheless
kudos for finding and pointing out this bug!
Andrew, please add this to the changelog of
pi-futex-scheduler-support-for-pi.patch:
the effective_prio() cleanups also fix a priority-calculation bug
pointed out by Andrey Gelman, in set_user_nice().
Ingo