Re: [PATCH 05/16] sched: SCHED_DEADLINE policy implementation.

From: Juri Lelli
Date: Mon Apr 23 2012 - 12:41:15 EST


On 04/23/2012 05:43 PM, Peter Zijlstra wrote:
On Mon, 2012-04-23 at 17:39 +0200, Juri Lelli wrote:
On 04/23/2012 04:35 PM, Peter Zijlstra wrote:
On Fri, 2012-04-06 at 09:14 +0200, Juri Lelli wrote:
+static void init_dl_task_timer(struct sched_dl_entity *dl_se)
+{
+ struct hrtimer *timer =&dl_se->dl_timer;
+
+ if (hrtimer_active(timer)) {
+ hrtimer_try_to_cancel(timer);
+ return;
+ }

Same question I guess, how can it be active here? Also, just letting it
run doesn't seem like the best way out..


Probably s/hrtimer_try_to_cancel/hrtimer_cancel is better.

Yeah, not sure you can do hrtimer_cancel() there though, you're holding
->pi_lock and rq->lock and have IRQs disabled. That sounds like asking
for trouble.

Anyway, if it can't happen, we don't have to fix it.. so lets answer
that first ;-)

The user could call __setparam_dl on a throttled task through
__sched_setscheduler.

BTW, I noticed that we should change this (inside __sched_setscheduler):

/*
* If not changing anything there's no need to proceed further
*/
if (unlikely(policy == p->policy && (!rt_policy(policy) ||
param->sched_priority == p->rt_priority))) {

__task_rq_unlock(rq);
raw_spin_unlock_irqrestore(&p->pi_lock, flags);
return 0;
}

to something like this:

if (unlikely(policy == p->policy && (!rt_policy(policy) ||
param->sched_priority == p->rt_priority) &&
!dl_policy(policy)))

Thanks,

- Juri
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/