Skip to contentSkip to conversation

sched: stop hrtick timer if running task is switching from fair scheduling class to another

PATCH In review Kirill Tkhai 3 participants 6 messages Dec 22, 2011 – Jan 4, 2012 Scheduler

lore
Share & tools
lore

Kirill Tkhai

author
#0 lore raw

PATCH 1/1 sched: stop hrtick timer if running task is switching from fair scheduling class to another

[PATCH]sched: stop hrtick timer if running task is switching from fair scheduling class to another

We have to stop hrtick timer to avoid excess interrupt. Not-fair tasks are not interested in fair's hrtick. RT class uses its own fixed timeslice (in case of RR), which doesn't depend on current value of hrtick timer.

Kernel tree/version/git: next-20111216

This is resending, nobody answered me in previous time.

Signed-off-by Kirill Tkhai <[email protected]>

---

kernel/sched/core.c +1 -5
index 211cdc5..7733ba7 100644
@@ -368,7 +368,7 @@ static struct rq *this_rq_lock(void)
  * rq->lock.
  */
 
-static void hrtick_clear(struct rq *rq)
+void hrtick_clear(struct rq *rq)
 {
 	if (hrtimer_active(&rq->hrtick_timer))
 		hrtimer_cancel(&rq->hrtick_timer);

@@ -480,10 +480,6 @@ static void init_rq_hrtick(struct rq *rq)
 	rq->hrtick_timer.function = hrtick;
 }
 #else	/* CONFIG_SCHED_HRTICK */
-static inline void hrtick_clear(struct rq *rq)
-{
-}
-
 static inline void init_rq_hrtick(struct rq *rq)
 {
 }
kernel/sched/fair.c +0 -0
index a4d2b7a..b083a91 100644
@@ -5271,6 +5271,13 @@ static void switched_from_fair(struct rq *rq,

struct task_struct *p)
place_entity(cfs_rq, se, 0); se->vruntime -= cfs_rq->min_vruntime;
}
+
+ /*
+ * Other scheduling classes are not interested in fair's hrtick timer.
+ */
+ if (task_current(rq, p) && sched_feat(HRTICK))
+ hrtick_clear(rq);
+
}

/*

kernel/sched/sched.h +6 -0
index d8d3613..f3c177c 100644
@@ -936,6 +936,8 @@ static inline int hrtick_enabled(struct rq *rq)
 	return hrtimer_is_hres_active(&rq->hrtick_timer);
 }
 
+void hrtick_clear(struct rq *rq);
+
 void hrtick_start(struct rq *rq, u64 delay);
 
 #else

@@ -945,6 +947,10 @@ static inline int hrtick_enabled(struct rq *rq)
 	return 0;
 }
 
+static inline void hrtick_clear(struct rq *rq)
+{
+}
+
 #endif /* CONFIG_SCHED_HRTICK */
 
 #ifdef CONFIG_SMP
#1 lore raw
Quoted 8 lines · Kirill Tkhai

[PATCH]sched: stop hrtick timer if running task is switching from fair scheduling class to another

We have to stop hrtick timer to avoid excess interrupt. Not-fair tasks are not interested in fair's hrtick. RT class uses its own fixed timeslice (in case of RR), which doesn't depend on current value of hrtick timer.

Well. I shouldn't try to comment this patch, I do not really understand this code.

But since nobody else replies...

Review static void switched_from_fair(struct rq *rq, · line 5282

Quoted 13 lines
(quoted diff) +7 -0
@@ -5271,6 +5271,13 @@ static void switched_from_fair(struct rq *rq,
 struct task_struct *p)
 		place_entity(cfs_rq, se, 0);
 		se->vruntime -= cfs_rq->min_vruntime;
 	}
+
+	/*
+	 * Other scheduling classes are not interested in fair's hrtick timer.
+	 */
+	if (task_current(rq, p) && sched_feat(HRTICK))
+		hrtick_clear(rq);
+
 }

May be... but in this case, perhaps instead we should teach dequeue_task_fair() or put_prev_task_fair() to do this. Then we can probably remove __schedule()->hrtick_clear().

I simply can't understand the current hrtick logic. For example, why dequeue_task_fair() does hrtick_update() ? OK, probably because "nr_running < sched_nr_latency" can become true. But at least this doesn't make sense to me when p == rq->curr, say, __schedule() path.

Hmm. In any case, how it is possible to do hrtick_start() with rq->lock held? hrtimer_restart() may want to wakeup_softirqd().

Peter, Ingo, does this code really work? SCHED_FEAT(HRTICK) == 0 by default, and afaics you can't change it without CONFIG_SCHED_DEBUG.

Confused...

Oleg.

maintainer
#2 lore raw
Quoted 5 lines · Oleg Nesterov

Peter, Ingo, does this code really work? SCHED_FEAT(HRTICK) == 0 by default, and afaics you can't change it without CONFIG_SCHED_DEBUG.

No its broken. Maybe I should've removed it, but I've always been meaning to fix it since SCHED_DEADLINE actually needs it, but alas. -ENOTIME and no urgency need have conspired against it.

#3 lore raw

SCHED_RR && time_slice

In reply to Kirill Tkhai
Quoted 3 lines · Kirill Tkhai

RT class uses its own fixed
timeslice (in case of RR),

which looks confusing too, btw.

sched_set_scheduler() does not initialize rt.time_slice, and INIT_TASK() sets time_slice = HZ (not DEF_TIMESLICE).

Oleg.

author
#4 lore raw

Re: SCHED_RR && time_slice

Thanks for the answers, Oleg.

Quoted 10 lines · Oleg Nesterov
Quoted 3 lines · Kirill Tkhai

RT class uses its own fixed
timeslice (in case of RR),

which looks confusing too, btw.

sched_set_scheduler() does not initialize rt.time_slice, and INIT_TASK() sets time_slice = HZ (not DEF_TIMESLICE).

I talk about task_tick_rt(). When time_slice is over, it is being renewed. New value is DEF_TIMESLICE. This happens in case of SCHED_RR class.

Kirill

#5 lore raw

Re: SCHED_RR && time_slice

Quoted 9 lines · Kirill Tkhai

Thanks for the answers, Oleg.

Quoted 5 lines · Oleg Nesterov
Quoted 3 lines · Kirill Tkhai

RT class uses its own fixed
timeslice (in case of RR),

Quoted 7 lines
Quoted 5 lines

which looks confusing too, btw.

sched_set_scheduler() does not initialize rt.time_slice, and INIT_TASK() sets time_slice = HZ (not DEF_TIMESLICE).

I talk about task_tick_rt().

me too ;)

Quoted 3 lines

When time_slice is over, it is being
renewed. New value is DEF_TIMESLICE. This happens in case of SCHED_RR class.

Yes, but I was talking about initial value of time_slice, when the task switches to SCHED_RR.

Oleg.