* [PATCH]sched: stop hrtick timer if running task is switching from fair scheduling class to another
@ 2011-12-22 20:01 Kirill Tkhai
2011-12-26 15:42 ` Oleg Nesterov
2011-12-26 16:59 ` SCHED_RR && time_slice Oleg Nesterov
0 siblings, 2 replies; 6+ messages in thread
From: Kirill Tkhai @ 2011-12-22 20:01 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Peter Zijlstra, Thomas Gleixner, Oleg Nesterov,
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.
Kernel tree/version/git: next-20111216
This is resending, nobody answered me in previous time.
Signed-off-by: Kirill Tkhai <kirill.tkhai@gmail.com>
---
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 211cdc5..7733ba7 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -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)
{
}
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index a4d2b7a..b083a91 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -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);
+
}
/*
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index d8d3613..f3c177c 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH]sched: stop hrtick timer if running task is switching from fair scheduling class to another
2011-12-22 20:01 [PATCH]sched: stop hrtick timer if running task is switching from fair scheduling class to another Kirill Tkhai
@ 2011-12-26 15:42 ` Oleg Nesterov
2012-01-04 12:30 ` Peter Zijlstra
2011-12-26 16:59 ` SCHED_RR && time_slice Oleg Nesterov
1 sibling, 1 reply; 6+ messages in thread
From: Oleg Nesterov @ 2011-12-26 15:42 UTC (permalink / raw)
To: Kirill Tkhai
Cc: linux-kernel, Ingo Molnar, Peter Zijlstra, Thomas Gleixner, Kirill Tkhai
On 12/23, Kirill Tkhai wrote:
>
> [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...
> @@ -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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH]sched: stop hrtick timer if running task is switching from fair scheduling class to another
2011-12-26 15:42 ` Oleg Nesterov
@ 2012-01-04 12:30 ` Peter Zijlstra
0 siblings, 0 replies; 6+ messages in thread
From: Peter Zijlstra @ 2012-01-04 12:30 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Kirill Tkhai, linux-kernel, Ingo Molnar, Thomas Gleixner, Kirill Tkhai
On Mon, 2011-12-26 at 16:42 +0100, Oleg Nesterov wrote:
>
> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* SCHED_RR && time_slice
2011-12-22 20:01 [PATCH]sched: stop hrtick timer if running task is switching from fair scheduling class to another Kirill Tkhai
2011-12-26 15:42 ` Oleg Nesterov
@ 2011-12-26 16:59 ` Oleg Nesterov
2011-12-29 16:43 ` Kirill Tkhai
1 sibling, 1 reply; 6+ messages in thread
From: Oleg Nesterov @ 2011-12-26 16:59 UTC (permalink / raw)
To: Kirill Tkhai
Cc: linux-kernel, Ingo Molnar, Peter Zijlstra, Thomas Gleixner, Kirill Tkhai
On 12/23, Kirill Tkhai wrote:
>
> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: SCHED_RR && time_slice
2011-12-26 16:59 ` SCHED_RR && time_slice Oleg Nesterov
@ 2011-12-29 16:43 ` Kirill Tkhai
2011-12-29 17:12 ` Oleg Nesterov
0 siblings, 1 reply; 6+ messages in thread
From: Kirill Tkhai @ 2011-12-29 16:43 UTC (permalink / raw)
To: Oleg Nesterov; +Cc: linux-kernel, Ingo Molnar, Peter Zijlstra, Thomas Gleixner
Thanks for the answers, Oleg.
On Mon, 2011-12-26 at 17:59 +0100, Oleg Nesterov wrote:
> On 12/23, Kirill Tkhai wrote:
> >
> > 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: SCHED_RR && time_slice
2011-12-29 16:43 ` Kirill Tkhai
@ 2011-12-29 17:12 ` Oleg Nesterov
0 siblings, 0 replies; 6+ messages in thread
From: Oleg Nesterov @ 2011-12-29 17:12 UTC (permalink / raw)
To: Kirill Tkhai; +Cc: linux-kernel, Ingo Molnar, Peter Zijlstra, Thomas Gleixner
On 12/29, Kirill Tkhai wrote:
>
> Thanks for the answers, Oleg.
>
> On Mon, 2011-12-26 at 17:59 +0100, Oleg Nesterov wrote:
> > On 12/23, Kirill Tkhai wrote:
> > >
> > > 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().
me too ;)
> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-01-04 12:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-22 20:01 [PATCH]sched: stop hrtick timer if running task is switching from fair scheduling class to another Kirill Tkhai
2011-12-26 15:42 ` Oleg Nesterov
2012-01-04 12:30 ` Peter Zijlstra
2011-12-26 16:59 ` SCHED_RR && time_slice Oleg Nesterov
2011-12-29 16:43 ` Kirill Tkhai
2011-12-29 17:12 ` Oleg Nesterov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.