Hi all,
when running some experiments on current git master, I noticed a
regression respect to version 3.18 of the kernel: when invoking
sched_setattr() to change the SCHED_DEADLINE parameters of a task that
is already scheduled by SCHED_DEADLINE, it is possible to crash the
system.
The bug can be reproduced with this testcase:
http://disi.unitn.it/~abeni/reclaiming/bug-test.tgz
Uncompress it, enter the "Bug-Test" directory, and type "make test".
After few cycles, my test machine (a laptop with an intel i7 CPU)
becomes unusable, and freezes.
Since I know that 3.18 is not affected by this bug, I tried a bisect,
that pointed to commit 67dfa1b756f250972bde31d65e3f8fde6aeddc5b
(sched/deadline: Implement cancel_dl_timer() to use in
switched_from_dl()).
By looking at that commit, I suspect the problem is that it removes the
following lines from init_dl_task_timer():
- if (hrtimer_active(timer)) {
- hrtimer_try_to_cancel(timer);
- return;
- }
As a result, when changing the parameters of a SCHED_DEADLINE task
init_dl_task_timer() is invoked again, and it can initialize a pending
timer (not sure why, but this seems to be the cause of the freezes I am
seeing).
So, I modified core.c::__setparam_dl() to invoke init_dl_task_timer()
only if the task is not already scheduled by SCHED_DEADLINE...
Basically, I changed
init_dl_task_timer(dl_se);
into
if (p->sched_class != &dl_sched_class) {
init_dl_task_timer(dl_se);
}
I am not sure if this is the correct fix, but with this change the
kernel survives my test script (mentioned above), and arrives to 500
cycles (without my patch, it crashes after 2 or 3 cycles).
What do you think? Is my patch correct, or should I fix the issue in a
different way?
Thanks,
Luca
On 31/01/2015 09:56, Peter Zijlstra wrote:
> On Fri, Jan 30, 2015 at 10:35:02AM +0000, Juri Lelli wrote:
>> So, we do the safe thing only in case of throttling.
>
> No, even for the !throttle aka running tasks. We only use
> dl_{runtime,deadline,period} for replenishment, until that time we
> observe the old runtime/deadline set by the previous replenishment.
>
Oh, right. We set dl_new in __dl_clear_params(), nice.
Thanks,
- Juri
>> I guess is more than
>> ok for now, while we hopefully find some spare cycle to implement a
>> complete solution :/.
>
> Yeah, I bet the fun part is computing the 0-lag across the entire root
> domain, per-cpu 0-lag isn't correct afaict.
>
Hi Peter,
On 01/31/2015 10:56 AM, Peter Zijlstra wrote:
> On Fri, Jan 30, 2015 at 10:35:02AM +0000, Juri Lelli wrote:
>> So, we do the safe thing only in case of throttling.
>
> No, even for the !throttle aka running tasks. We only use
> dl_{runtime,deadline,period} for replenishment, until that time we
> observe the old runtime/deadline set by the previous replenishment.
>
>> I guess is more than
>> ok for now, while we hopefully find some spare cycle to implement a
>> complete solution :/.
>
> Yeah, I bet the fun part is computing the 0-lag across the entire root
> domain, per-cpu 0-lag isn't correct afaict.
Uhm... This is an interesting problem.
I _suspect_ the 0-lag time does not depend on the number of CPUs. In other
words: I _suspect_ that when you kill a SCHED_DEADLINE task its bandwidth
should released at a time
t0 = scheduling_deadline - current_budget / maximum_budget * period
and this time is not affected by the fact that the task is scheduled by
global EDF or by EDF on a single core.
But I have no proof about this (and I changed my mind on this multiple
times :).
On a side note: as far as I can see, releasing the bandwidth at the end
of the current reservation period (that is, when the time is equal to the
current scheduling deadline) should be safe.
Basically, by doing this we assume that the task already consumed all of
its budget for the current reservation period.
Luca