* Re: [PATCH] sched: Remove WAKEUP_PREEMPT feature check in entity_tick
2011-07-29 7:56 ` Peter Zijlstra
@ 2011-07-29 8:18 ` Yong Zhang
2011-07-29 8:20 ` Peter Zijlstra
2011-07-29 8:20 ` [RFC PATCH] sched: Kill WAKEUP_PREEMPT Yong Zhang
2011-07-29 9:04 ` [PATCH] sched: Remove WAKEUP_PREEMPT feature check in entity_tick Mike Galbraith
2 siblings, 1 reply; 20+ messages in thread
From: Yong Zhang @ 2011-07-29 8:18 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: Lin Ming, mingo, lkml, Mike Galbraith
On Fri, Jul 29, 2011 at 09:56:58AM +0200, Peter Zijlstra wrote:
> On Fri, 2011-07-29 at 15:46 +0800, Yong Zhang wrote:
> > On Fri, Jul 29, 2011 at 03:36:15PM +0800, Lin Ming wrote:
> > > >From another point of view, below !sched_feat(WAKEUP_PREEMPT) still
> > > looks like duplicated.
> > >
> > > if (cfs_rq->nr_running > 1 || !sched_feat(WAKEUP_PREEMPT))
> > > check_preempt_tick(cfs_rq, curr);
> > >
> > > if "!sched_feat(WAKEUP_PREEMPT)" is run,
> > > that implies cfs_rq->nr_running == 1.
> >
> > That's true.
> >
> > >
> > > Why do we need to call check_preempt_tick when there is only 1 task
> > > runnable?
> >
> > Just set_tsk_need_resched(p) if p's slice is over, thus:
> >
> > (n tick) ---> (n+1 tick)
> > set_tsk_need_resched(p);
> > another task Q is awaked
> >
> > If we don't have !sched_feat(WAKEUP_PREEMPT), Q maybe will wait
> > for tick coming to get scheduled. If we have
> > !sched_feat(WAKEUP_PREEMPT), Q will get scheduled when some event
> > happen, like IRQ.
>
> Nah, if there is 1 runnable task it will always run, preemption simply
> doesn't matter. There's nothing to preempt it with.
Hmmm, so the newly waked task could be scheduled a little later.
That means schedule tick judge everything.
Thanks,
Yong
>
> I've queued Lin's patch as I don't see the point of this thing either,
> normally WAKEUP_PREEMPT is enabled so it says || 0 which is kinda
> useless :-)
>
> And I'm starting to think we should just kill all of WAKEUP_PREEMPT I
> don't think we ever want to disable it anyway..
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
Only stand for myself
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] sched: Remove WAKEUP_PREEMPT feature check in entity_tick
2011-07-29 8:18 ` Yong Zhang
@ 2011-07-29 8:20 ` Peter Zijlstra
2011-07-29 8:44 ` Lin Ming
2011-07-29 8:46 ` Yong Zhang
0 siblings, 2 replies; 20+ messages in thread
From: Peter Zijlstra @ 2011-07-29 8:20 UTC (permalink / raw)
To: Yong Zhang; +Cc: Lin Ming, mingo, lkml, Mike Galbraith
On Fri, 2011-07-29 at 16:18 +0800, Yong Zhang wrote:
> > Nah, if there is 1 runnable task it will always run, preemption simply
> > doesn't matter. There's nothing to preempt it with.
>
> Hmmm, so the newly waked task could be scheduled a little later.
> That means schedule tick judge everything.
Oh, are you referring to the case where a task gets woken on an idle
remote cpu?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] sched: Remove WAKEUP_PREEMPT feature check in entity_tick
2011-07-29 8:20 ` Peter Zijlstra
@ 2011-07-29 8:44 ` Lin Ming
2011-07-29 8:46 ` Yong Zhang
1 sibling, 0 replies; 20+ messages in thread
From: Lin Ming @ 2011-07-29 8:44 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: Yong Zhang, mingo, lkml, Mike Galbraith
On Fri, 2011-07-29 at 16:20 +0800, Peter Zijlstra wrote:
> On Fri, 2011-07-29 at 16:18 +0800, Yong Zhang wrote:
> > > Nah, if there is 1 runnable task it will always run, preemption simply
> > > doesn't matter. There's nothing to preempt it with.
> >
> > Hmmm, so the newly waked task could be scheduled a little later.
> > That means schedule tick judge everything.
>
> Oh, are you referring to the case where a task gets woken on an idle
> remote cpu?
Yong means the case below.
At (n tick) there is only 1 task, then another task is woken before
(n+1) tick.
On Fri, 2011-07-29 at 15:46 +0800, Yong Zhang wrote:
>
> Just set_tsk_need_resched(p) if p's slice is over, thus:
>
> (n tick) ---> (n+1 tick)
> set_tsk_need_resched(p);
> another task Q is awaked
>
> If we don't have !sched_feat(WAKEUP_PREEMPT), Q maybe will wait
> for tick coming to get scheduled. If we have
> !sched_feat(WAKEUP_PREEMPT), Q will get scheduled when some event
> happen, like IRQ.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] sched: Remove WAKEUP_PREEMPT feature check in entity_tick
2011-07-29 8:20 ` Peter Zijlstra
2011-07-29 8:44 ` Lin Ming
@ 2011-07-29 8:46 ` Yong Zhang
2011-07-29 11:45 ` Peter Zijlstra
1 sibling, 1 reply; 20+ messages in thread
From: Yong Zhang @ 2011-07-29 8:46 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: Lin Ming, mingo, lkml, Mike Galbraith
On Fri, Jul 29, 2011 at 10:20:50AM +0200, Peter Zijlstra wrote:
> On Fri, 2011-07-29 at 16:18 +0800, Yong Zhang wrote:
> > > Nah, if there is 1 runnable task it will always run, preemption simply
> > > doesn't matter. There's nothing to preempt it with.
> >
> > Hmmm, so the newly waked task could be scheduled a little later.
> > That means schedule tick judge everything.
>
> Oh, are you referring to the case where a task gets woken on an idle
> remote cpu?
Not really.
Let's take UP for example, we have cpu-hug task A and threadirq B.
n tick ---> n+1 tick
set_tsk_need_resched(A);
B comes in and
wake up thread-B;
So for system on which we disable WAKEUP_PREEMPT,
if we don't have that check, thread-B will wait until n+1 tick comes
to get to run.
But if we have that check, thread-B will get to run after IRQ-B returns.
Thanks,
Yong
--
Only stand for myself
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] sched: Remove WAKEUP_PREEMPT feature check in entity_tick
2011-07-29 8:46 ` Yong Zhang
@ 2011-07-29 11:45 ` Peter Zijlstra
2011-08-01 1:33 ` Yong Zhang
0 siblings, 1 reply; 20+ messages in thread
From: Peter Zijlstra @ 2011-07-29 11:45 UTC (permalink / raw)
To: Yong Zhang; +Cc: Lin Ming, mingo, lkml, Mike Galbraith
On Fri, 2011-07-29 at 16:46 +0800, Yong Zhang wrote:
> Let's take UP for example, we have cpu-hug task A and threadirq B.
>
> n tick ---> n+1 tick
> set_tsk_need_resched(A);
> B comes in and
> wake up thread-B;
>
> So for system on which we disable WAKEUP_PREEMPT,
> if we don't have that check, thread-B will wait until n+1 tick comes
> to get to run.
> But if we have that check, thread-B will get to run after IRQ-B returns.
But that's exactly what wakeup preemption is about, waking tasks don't
get to preempt running tasks. So no doing that preemption is exactly
right for !WAKEUP_PREEMPT.
Anyway, I've queued the removal patch since that removes all
confusion ;-)
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] sched: Remove WAKEUP_PREEMPT feature check in entity_tick
2011-07-29 11:45 ` Peter Zijlstra
@ 2011-08-01 1:33 ` Yong Zhang
0 siblings, 0 replies; 20+ messages in thread
From: Yong Zhang @ 2011-08-01 1:33 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: Lin Ming, mingo, lkml, Mike Galbraith
On Fri, Jul 29, 2011 at 01:45:33PM +0200, Peter Zijlstra wrote:
> On Fri, 2011-07-29 at 16:46 +0800, Yong Zhang wrote:
> > Let's take UP for example, we have cpu-hug task A and threadirq B.
> >
> > n tick ---> n+1 tick
> > set_tsk_need_resched(A);
> > B comes in and
> > wake up thread-B;
> >
> > So for system on which we disable WAKEUP_PREEMPT,
> > if we don't have that check, thread-B will wait until n+1 tick comes
> > to get to run.
> > But if we have that check, thread-B will get to run after IRQ-B returns.
>
> But that's exactly what wakeup preemption is about, waking tasks don't
> get to preempt running tasks. So no doing that preemption is exactly
> right for !WAKEUP_PREEMPT.
>
> Anyway, I've queued the removal patch since that removes all
> confusion ;-)
Yup, that sounds good. Thanks Peter.
-Yong
--
Only stand for myself
^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC PATCH] sched: Kill WAKEUP_PREEMPT
2011-07-29 7:56 ` Peter Zijlstra
2011-07-29 8:18 ` Yong Zhang
@ 2011-07-29 8:20 ` Yong Zhang
2011-07-29 8:30 ` Lin Ming
2011-08-14 15:57 ` [tip:sched/core] " tip-bot for Yong Zhang
2011-07-29 9:04 ` [PATCH] sched: Remove WAKEUP_PREEMPT feature check in entity_tick Mike Galbraith
2 siblings, 2 replies; 20+ messages in thread
From: Yong Zhang @ 2011-07-29 8:20 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: Lin Ming, mingo, lkml, Mike Galbraith
On Fri, Jul 29, 2011 at 09:56:58AM +0200, Peter Zijlstra wrote:
> I've queued Lin's patch as I don't see the point of this thing either,
> normally WAKEUP_PREEMPT is enabled so it says || 0 which is kinda
> useless :-)
>
> And I'm starting to think we should just kill all of WAKEUP_PREEMPT I
> don't think we ever want to disable it anyway..
Someting like this?
---
From: Yong Zhang <yong.zhang0@gmail.com>
Subject: [RFC PATCH] sched: Kill WAKEUP_PREEMPT
Per Peter Zijlstra:
> And I'm starting to think we should just kill all of WAKEUP_PREEMPT I
> don't think we ever want to disable it anyway..
Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
---
kernel/sched_fair.c | 8 +-------
kernel/sched_features.h | 5 -----
2 files changed, 1 insertions(+), 12 deletions(-)
diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
index 46b7855..3c58042 100644
--- a/kernel/sched_fair.c
+++ b/kernel/sched_fair.c
@@ -1114,9 +1114,6 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
* narrow margin doesn't have to wait for a full slice.
* This also mitigates buddy induced latencies under load.
*/
- if (!sched_feat(WAKEUP_PREEMPT))
- return;
-
if (delta_exec < sysctl_sched_min_granularity)
return;
@@ -1252,7 +1249,7 @@ entity_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr, int queued)
return;
#endif
- if (cfs_rq->nr_running > 1 || !sched_feat(WAKEUP_PREEMPT))
+ if (cfs_rq->nr_running > 1)
check_preempt_tick(cfs_rq, curr);
}
@@ -1918,9 +1915,6 @@ static void check_preempt_wakeup(struct rq *rq, struct task_struct *p, int wake_
return;
- if (!sched_feat(WAKEUP_PREEMPT))
- return;
-
update_curr(cfs_rq);
find_matching_se(&se, &pse);
BUG_ON(!pse);
diff --git a/kernel/sched_features.h b/kernel/sched_features.h
index 1e7066d..878a5f9 100644
--- a/kernel/sched_features.h
+++ b/kernel/sched_features.h
@@ -12,11 +12,6 @@ SCHED_FEAT(GENTLE_FAIR_SLEEPERS, 1)
SCHED_FEAT(START_DEBIT, 1)
/*
- * Should wakeups try to preempt running tasks.
- */
-SCHED_FEAT(WAKEUP_PREEMPT, 1)
-
-/*
* Based on load and program behaviour, see if it makes sense to place
* a newly woken task on the same cpu as the task that woke it --
* improve cache locality. Typically used with SYNC wakeups as
--
1.7.4.1
--
Only stand for myself
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [RFC PATCH] sched: Kill WAKEUP_PREEMPT
2011-07-29 8:20 ` [RFC PATCH] sched: Kill WAKEUP_PREEMPT Yong Zhang
@ 2011-07-29 8:30 ` Lin Ming
2011-07-29 8:32 ` Lin Ming
2011-08-14 15:57 ` [tip:sched/core] " tip-bot for Yong Zhang
1 sibling, 1 reply; 20+ messages in thread
From: Lin Ming @ 2011-07-29 8:30 UTC (permalink / raw)
To: Yong Zhang; +Cc: Peter Zijlstra, mingo, lkml, Mike Galbraith
On Fri, 2011-07-29 at 16:20 +0800, Yong Zhang wrote:
> On Fri, Jul 29, 2011 at 09:56:58AM +0200, Peter Zijlstra wrote:
> > I've queued Lin's patch as I don't see the point of this thing either,
> > normally WAKEUP_PREEMPT is enabled so it says || 0 which is kinda
> > useless :-)
> >
> > And I'm starting to think we should just kill all of WAKEUP_PREEMPT I
> > don't think we ever want to disable it anyway..
>
> Someting like this?
>
> ---
> From: Yong Zhang <yong.zhang0@gmail.com>
> Subject: [RFC PATCH] sched: Kill WAKEUP_PREEMPT
>
> Per Peter Zijlstra:
> > And I'm starting to think we should just kill all of WAKEUP_PREEMPT I
> > don't think we ever want to disable it anyway..
>
> Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
> ---
> kernel/sched_fair.c | 8 +-------
> kernel/sched_features.h | 5 -----
> 2 files changed, 1 insertions(+), 12 deletions(-)
>
> diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
> index 46b7855..3c58042 100644
> --- a/kernel/sched_fair.c
> +++ b/kernel/sched_fair.c
> @@ -1114,9 +1114,6 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
> * narrow margin doesn't have to wait for a full slice.
> * This also mitigates buddy induced latencies under load.
> */
> - if (!sched_feat(WAKEUP_PREEMPT))
> - return;
> -
- /*
- * Ensure that a task that missed wakeup preemption by a
- * narrow margin doesn't have to wait for a full slice.
- * This also mitigates buddy induced latencies under load.
- */
- if (!sched_feat(WAKEUP_PREEMPT))
- return;
-
Then remove the comments also.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH] sched: Kill WAKEUP_PREEMPT
2011-07-29 8:30 ` Lin Ming
@ 2011-07-29 8:32 ` Lin Ming
0 siblings, 0 replies; 20+ messages in thread
From: Lin Ming @ 2011-07-29 8:32 UTC (permalink / raw)
To: Yong Zhang; +Cc: Peter Zijlstra, mingo, lkml, Mike Galbraith
On Fri, 2011-07-29 at 16:30 +0800, Lin Ming wrote:
> On Fri, 2011-07-29 at 16:20 +0800, Yong Zhang wrote:
> > On Fri, Jul 29, 2011 at 09:56:58AM +0200, Peter Zijlstra wrote:
> > > I've queued Lin's patch as I don't see the point of this thing either,
> > > normally WAKEUP_PREEMPT is enabled so it says || 0 which is kinda
> > > useless :-)
> > >
> > > And I'm starting to think we should just kill all of WAKEUP_PREEMPT I
> > > don't think we ever want to disable it anyway..
> >
> > Someting like this?
> >
> > ---
> > From: Yong Zhang <yong.zhang0@gmail.com>
> > Subject: [RFC PATCH] sched: Kill WAKEUP_PREEMPT
> >
> > Per Peter Zijlstra:
> > > And I'm starting to think we should just kill all of WAKEUP_PREEMPT I
> > > don't think we ever want to disable it anyway..
> >
> > Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
> > ---
> > kernel/sched_fair.c | 8 +-------
> > kernel/sched_features.h | 5 -----
> > 2 files changed, 1 insertions(+), 12 deletions(-)
> >
> > diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
> > index 46b7855..3c58042 100644
> > --- a/kernel/sched_fair.c
> > +++ b/kernel/sched_fair.c
> > @@ -1114,9 +1114,6 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
> > * narrow margin doesn't have to wait for a full slice.
> > * This also mitigates buddy induced latencies under load.
> > */
> > - if (!sched_feat(WAKEUP_PREEMPT))
> > - return;
> > -
>
> - /*
> - * Ensure that a task that missed wakeup preemption by a
> - * narrow margin doesn't have to wait for a full slice.
> - * This also mitigates buddy induced latencies under load.
> - */
> - if (!sched_feat(WAKEUP_PREEMPT))
> - return;
> -
>
> Then remove the comments also.
Sorry, don't remove that comments.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [tip:sched/core] sched: Kill WAKEUP_PREEMPT
2011-07-29 8:20 ` [RFC PATCH] sched: Kill WAKEUP_PREEMPT Yong Zhang
2011-07-29 8:30 ` Lin Ming
@ 2011-08-14 15:57 ` tip-bot for Yong Zhang
1 sibling, 0 replies; 20+ messages in thread
From: tip-bot for Yong Zhang @ 2011-08-14 15:57 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, hpa, mingo, a.p.zijlstra, efault, yong.zhang0, tglx, mingo
Commit-ID: 2c2efaed9bc973e3aeab1385c618017b56c8f6d7
Gitweb: http://git.kernel.org/tip/2c2efaed9bc973e3aeab1385c618017b56c8f6d7
Author: Yong Zhang <yong.zhang0@gmail.com>
AuthorDate: Fri, 29 Jul 2011 16:20:33 +0800
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Sun, 14 Aug 2011 12:00:41 +0200
sched: Kill WAKEUP_PREEMPT
Remove the WAKEUP_PREEMPT feature, disabling it doesn't make any sense
and its outlived its use by a long long while.
Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
Acked-by: Mike Galbraith <efault@gmx.de>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20110729082033.GB12106@zhy
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
kernel/sched_fair.c | 9 +--------
kernel/sched_features.h | 5 -----
2 files changed, 1 insertions(+), 13 deletions(-)
diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
index bc8ee99..241fc86 100644
--- a/kernel/sched_fair.c
+++ b/kernel/sched_fair.c
@@ -1095,9 +1095,6 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
* narrow margin doesn't have to wait for a full slice.
* This also mitigates buddy induced latencies under load.
*/
- if (!sched_feat(WAKEUP_PREEMPT))
- return;
-
if (delta_exec < sysctl_sched_min_granularity)
return;
@@ -1233,7 +1230,7 @@ entity_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr, int queued)
return;
#endif
- if (cfs_rq->nr_running > 1 || !sched_feat(WAKEUP_PREEMPT))
+ if (cfs_rq->nr_running > 1)
check_preempt_tick(cfs_rq, curr);
}
@@ -1899,10 +1896,6 @@ static void check_preempt_wakeup(struct rq *rq, struct task_struct *p, int wake_
if (unlikely(p->policy != SCHED_NORMAL))
return;
-
- if (!sched_feat(WAKEUP_PREEMPT))
- return;
-
find_matching_se(&se, &pse);
update_curr(cfs_rq_of(se));
BUG_ON(!pse);
diff --git a/kernel/sched_features.h b/kernel/sched_features.h
index 2e74677..efa0a7b 100644
--- a/kernel/sched_features.h
+++ b/kernel/sched_features.h
@@ -12,11 +12,6 @@ SCHED_FEAT(GENTLE_FAIR_SLEEPERS, 1)
SCHED_FEAT(START_DEBIT, 1)
/*
- * Should wakeups try to preempt running tasks.
- */
-SCHED_FEAT(WAKEUP_PREEMPT, 1)
-
-/*
* Based on load and program behaviour, see if it makes sense to place
* a newly woken task on the same cpu as the task that woke it --
* improve cache locality. Typically used with SYNC wakeups as
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] sched: Remove WAKEUP_PREEMPT feature check in entity_tick
2011-07-29 7:56 ` Peter Zijlstra
2011-07-29 8:18 ` Yong Zhang
2011-07-29 8:20 ` [RFC PATCH] sched: Kill WAKEUP_PREEMPT Yong Zhang
@ 2011-07-29 9:04 ` Mike Galbraith
2 siblings, 0 replies; 20+ messages in thread
From: Mike Galbraith @ 2011-07-29 9:04 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: Yong Zhang, Lin Ming, mingo, lkml
On Fri, 2011-07-29 at 09:56 +0200, Peter Zijlstra wrote:
> And I'm starting to think we should just kill all of WAKEUP_PREEMPT I
> don't think we ever want to disable it anyway..
Agreed. You have to be really into pain to flip that switch.
-Mike
^ permalink raw reply [flat|nested] 20+ messages in thread