* [PATCH] sched/rt: Fix rq->clock_update_flags < RQCF_ACT_SKIP warning
@ 2018-04-02 16:49 Davidlohr Bueso
2018-04-03 12:39 ` Matt Fleming
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Davidlohr Bueso @ 2018-04-02 16:49 UTC (permalink / raw)
To: peterz, mingo; +Cc: efault, rostedt, matt, dave, linux-kernel, Davidlohr Bueso
While running rt-tests' pi_stress program I got the following splat:
444.884597] ------------[ cut here ]------------
[ 444.894784] rq->clock_update_flags < RQCF_ACT_SKIP
[ 444.894798] WARNING: CPU: 27 PID: 0 at kernel/sched/sched.h:960 assert_clock_updated.isra.38.part.39+0x13/0x20
[ 444.927419] Modules linked in: ...
[ 445.141336] CPU: 27 PID: 0 Comm: swapper/27 Kdump: loaded Tainted: G E 4.16.0-rc7-next-20180329-13.5-default+ #1
[ 445.168197] Hardware name: QCI QSSC-S4R/QSSC-S4R, BIOS QSSC-S4R.QCI.01.00.S013.032920111005 03/29/2011
[ 445.188728] RIP: 0010:assert_clock_updated.isra.38.part.39+0x13/0x20
[ 445.202739] RSP: 0018:ffff9af37fb03e98 EFLAGS: 00010092
[ 445.214258] RAX: 0000000000000026 RBX: ffff9af37faa1bc0 RCX: ffffffffbd064a68
[ 445.229994] RDX: 0000000000000001 RSI: 0000000000000092 RDI: 0000000000000083
[ 445.245732] RBP: 0000000000000017 R08: 00000000000005f1 R09: ffffffffbd864ca0
[ 445.261468] R10: 000000003cdf0977 R11: 0000000000000000 R12: ffff9af186178180
[ 445.277206] R13: ffffffffbd2396a0 R14: ffff9af37faa1bc0 R15: 0000000000000017
[ 445.292944] FS: 0000000000000000(0000) GS:ffff9af37fb00000(0000) knlGS:0000000000000000
[ 445.310790] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 445.323457] CR2: 0000556773e17968 CR3: 0000000b3d00a002 CR4: 00000000000206e0
[ 445.339193] Call Trace:
[ 445.344578] <IRQ>
[ 445.349001] enqueue_top_rt_rq+0xf4/0x150
[ 445.357839] ? cpufreq_dbs_governor_start+0x170/0x170
[ 445.368974] sched_rt_rq_enqueue+0x65/0x80
[ 445.378000] sched_rt_period_timer+0x156/0x360
[ 445.387792] ? sched_rt_rq_enqueue+0x80/0x80
[ 445.397204] __hrtimer_run_queues+0xfa/0x260
[ 445.406614] hrtimer_interrupt+0xcb/0x220
[ 445.415451] smp_apic_timer_interrupt+0x62/0x120
[ 445.425629] apic_timer_interrupt+0xf/0x20
[ 445.434654] </IRQ>
[ 445.439269] RIP: 0010:cpuidle_enter_state+0x9d/0x2b0
[ 445.450212] RSP: 0018:ffffac274642fec0 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
[ 445.466908] RAX: ffff9af37fb21bc0 RBX: 00000067952f9fb7 RCX: 000000000000001f
[ 445.482645] RDX: 00000067952f9fb7 RSI: 00000000389cb571 RDI: 0000000000000000
[ 445.498380] RBP: 0000000000000004 R08: 000000000000bc01 R09: 000000000000c04c
[ 445.514116] R10: ffffac274642fea0 R11: 0000000000000eed R12: ffffcc273fb10ff0
[ 445.529853] R13: ffffffffbd1813b8 R14: 0000000000000000 R15: 00000067923bdc5a
[ 445.545595] do_idle+0x183/0x1e0
[ 445.552703] cpu_startup_entry+0x5f/0x70
[ 445.561350] start_secondary+0x192/0x1d0
[ 445.569995] secondary_startup_64+0xa5/0xb0
[ 445.579212] Code: 00 48 0f a3 05 9f 85 17 01 0f 83 74 ff ff ff e9 d4 b6 fe ff 0f 1f 40 00 48 c7 c7 f0 fd e6 bc c6 05 2e 08 14 01 01 e8 cd 2b fc ff <0f> 0b c3 66 2e 0f 1f 84 00 00 00 00 00 66 66 66 66 90 83 ea 08
[ 445.620864] ---[ end trace 0183b70f54a7f4ba ]---
We can get rid of it be the "traditional" means of adding an update_rq_clock() call
after acquiring the rq->lock in do_sched_rt_period_timer().
The case for the rt task throttling (which this workload also hits) can be ignored in
that the skip_update call is actually bogus and quite the contrary (the request bits
are removed/reverted). By setting RQCF_UPDATED we really don't care if the skip is
happening or not and will therefore make the assert_clock_updated() check happy.
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
kernel/sched/rt.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 86b77987435e..ad13e6242481 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -839,6 +839,8 @@ static int do_sched_rt_period_timer(struct rt_bandwidth *rt_b, int overrun)
continue;
raw_spin_lock(&rq->lock);
+ update_rq_clock(rq);
+
if (rt_rq->rt_time) {
u64 runtime;
--
2.13.6
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] sched/rt: Fix rq->clock_update_flags < RQCF_ACT_SKIP warning
2018-04-02 16:49 [PATCH] sched/rt: Fix rq->clock_update_flags < RQCF_ACT_SKIP warning Davidlohr Bueso
@ 2018-04-03 12:39 ` Matt Fleming
2018-04-04 15:27 ` Peter Zijlstra
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Matt Fleming @ 2018-04-03 12:39 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: peterz, mingo, efault, rostedt, linux-kernel, Davidlohr Bueso
On Mon, 02 Apr, at 09:49:54AM, Davidlohr Bueso wrote:
>
> We can get rid of it be the "traditional" means of adding an update_rq_clock() call
> after acquiring the rq->lock in do_sched_rt_period_timer().
>
> The case for the rt task throttling (which this workload also hits) can be ignored in
> that the skip_update call is actually bogus and quite the contrary (the request bits
> are removed/reverted). By setting RQCF_UPDATED we really don't care if the skip is
> happening or not and will therefore make the assert_clock_updated() check happy.
>
> Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
> ---
> kernel/sched/rt.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> index 86b77987435e..ad13e6242481 100644
> --- a/kernel/sched/rt.c
> +++ b/kernel/sched/rt.c
> @@ -839,6 +839,8 @@ static int do_sched_rt_period_timer(struct rt_bandwidth *rt_b, int overrun)
> continue;
>
> raw_spin_lock(&rq->lock);
> + update_rq_clock(rq);
> +
> if (rt_rq->rt_time) {
> u64 runtime;
Looks good to me.
Reviewed-by: Matt Fleming <matt@codeblueprint.co.uk>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sched/rt: Fix rq->clock_update_flags < RQCF_ACT_SKIP warning
2018-04-02 16:49 [PATCH] sched/rt: Fix rq->clock_update_flags < RQCF_ACT_SKIP warning Davidlohr Bueso
2018-04-03 12:39 ` Matt Fleming
@ 2018-04-04 15:27 ` Peter Zijlstra
2018-04-04 16:15 ` Davidlohr Bueso
2018-04-05 9:16 ` [tip:sched/urgent] sched/rt: Fix rq->clock_update_flags < RQCF_ACT_SKIP warning tip-bot for Davidlohr Bueso
3 siblings, 0 replies; 8+ messages in thread
From: Peter Zijlstra @ 2018-04-04 15:27 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: mingo, efault, rostedt, matt, linux-kernel, Davidlohr Bueso
On Mon, Apr 02, 2018 at 09:49:54AM -0700, Davidlohr Bueso wrote:
> While running rt-tests' pi_stress program I got the following splat:
>
> 444.884597] ------------[ cut here ]------------
> [ 444.894784] rq->clock_update_flags < RQCF_ACT_SKIP
> [ 444.894798] WARNING: CPU: 27 PID: 0 at kernel/sched/sched.h:960 assert_clock_updated.isra.38.part.39+0x13/0x20
> [ 444.927419] Modules linked in: ...
> [ 445.141336] CPU: 27 PID: 0 Comm: swapper/27 Kdump: loaded Tainted: G E 4.16.0-rc7-next-20180329-13.5-default+ #1
> [ 445.168197] Hardware name: QCI QSSC-S4R/QSSC-S4R, BIOS QSSC-S4R.QCI.01.00.S013.032920111005 03/29/2011
> [ 445.188728] RIP: 0010:assert_clock_updated.isra.38.part.39+0x13/0x20
> [ 445.202739] RSP: 0018:ffff9af37fb03e98 EFLAGS: 00010092
> [ 445.214258] RAX: 0000000000000026 RBX: ffff9af37faa1bc0 RCX: ffffffffbd064a68
> [ 445.229994] RDX: 0000000000000001 RSI: 0000000000000092 RDI: 0000000000000083
> [ 445.245732] RBP: 0000000000000017 R08: 00000000000005f1 R09: ffffffffbd864ca0
> [ 445.261468] R10: 000000003cdf0977 R11: 0000000000000000 R12: ffff9af186178180
> [ 445.277206] R13: ffffffffbd2396a0 R14: ffff9af37faa1bc0 R15: 0000000000000017
> [ 445.292944] FS: 0000000000000000(0000) GS:ffff9af37fb00000(0000) knlGS:0000000000000000
> [ 445.310790] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 445.323457] CR2: 0000556773e17968 CR3: 0000000b3d00a002 CR4: 00000000000206e0
> [ 445.339193] Call Trace:
> [ 445.344578] <IRQ>
> [ 445.349001] enqueue_top_rt_rq+0xf4/0x150
> [ 445.357839] ? cpufreq_dbs_governor_start+0x170/0x170
> [ 445.368974] sched_rt_rq_enqueue+0x65/0x80
> [ 445.378000] sched_rt_period_timer+0x156/0x360
> [ 445.387792] ? sched_rt_rq_enqueue+0x80/0x80
> [ 445.397204] __hrtimer_run_queues+0xfa/0x260
> [ 445.406614] hrtimer_interrupt+0xcb/0x220
> [ 445.415451] smp_apic_timer_interrupt+0x62/0x120
> [ 445.425629] apic_timer_interrupt+0xf/0x20
> [ 445.434654] </IRQ>
> [ 445.439269] RIP: 0010:cpuidle_enter_state+0x9d/0x2b0
> [ 445.450212] RSP: 0018:ffffac274642fec0 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
> [ 445.466908] RAX: ffff9af37fb21bc0 RBX: 00000067952f9fb7 RCX: 000000000000001f
> [ 445.482645] RDX: 00000067952f9fb7 RSI: 00000000389cb571 RDI: 0000000000000000
> [ 445.498380] RBP: 0000000000000004 R08: 000000000000bc01 R09: 000000000000c04c
> [ 445.514116] R10: ffffac274642fea0 R11: 0000000000000eed R12: ffffcc273fb10ff0
> [ 445.529853] R13: ffffffffbd1813b8 R14: 0000000000000000 R15: 00000067923bdc5a
> [ 445.545595] do_idle+0x183/0x1e0
> [ 445.552703] cpu_startup_entry+0x5f/0x70
> [ 445.561350] start_secondary+0x192/0x1d0
> [ 445.569995] secondary_startup_64+0xa5/0xb0
> [ 445.579212] Code: 00 48 0f a3 05 9f 85 17 01 0f 83 74 ff ff ff e9 d4 b6 fe ff 0f 1f 40 00 48 c7 c7 f0 fd e6 bc c6 05 2e 08 14 01 01 e8 cd 2b fc ff <0f> 0b c3 66 2e 0f 1f 84 00 00 00 00 00 66 66 66 66 90 83 ea 08
> [ 445.620864] ---[ end trace 0183b70f54a7f4ba ]---
>
> We can get rid of it be the "traditional" means of adding an update_rq_clock() call
> after acquiring the rq->lock in do_sched_rt_period_timer().
>
> The case for the rt task throttling (which this workload also hits) can be ignored in
> that the skip_update call is actually bogus and quite the contrary (the request bits
> are removed/reverted). By setting RQCF_UPDATED we really don't care if the skip is
> happening or not and will therefore make the assert_clock_updated() check happy.
>
> Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Ingo, please merge.
> ---
> kernel/sched/rt.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> index 86b77987435e..ad13e6242481 100644
> --- a/kernel/sched/rt.c
> +++ b/kernel/sched/rt.c
> @@ -839,6 +839,8 @@ static int do_sched_rt_period_timer(struct rt_bandwidth *rt_b, int overrun)
> continue;
>
> raw_spin_lock(&rq->lock);
> + update_rq_clock(rq);
> +
> if (rt_rq->rt_time) {
> u64 runtime;
>
> --
> 2.13.6
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sched/rt: Fix rq->clock_update_flags < RQCF_ACT_SKIP warning
2018-04-02 16:49 [PATCH] sched/rt: Fix rq->clock_update_flags < RQCF_ACT_SKIP warning Davidlohr Bueso
2018-04-03 12:39 ` Matt Fleming
2018-04-04 15:27 ` Peter Zijlstra
@ 2018-04-04 16:15 ` Davidlohr Bueso
2018-04-04 16:39 ` Peter Zijlstra
2018-04-05 9:17 ` [tip:sched/urgent] sched/core: Simplify helpers for rq clock update skip requests tip-bot for Davidlohr Bueso
2018-04-05 9:16 ` [tip:sched/urgent] sched/rt: Fix rq->clock_update_flags < RQCF_ACT_SKIP warning tip-bot for Davidlohr Bueso
3 siblings, 2 replies; 8+ messages in thread
From: Davidlohr Bueso @ 2018-04-04 16:15 UTC (permalink / raw)
To: peterz, mingo; +Cc: efault, rostedt, matt, linux-kernel, Davidlohr Bueso
On Mon, 02 Apr 2018, Davidlohr Bueso wrote:
>The case for the rt task throttling (which this workload also hits) can be ignored in
>that the skip_update call is actually bogus and quite the contrary (the request bits
>are removed/reverted).
While at it, how about this trivial patch?
----8<--------------------------------------------------------
[PATCH 2/1] sched: Simplify helpers for rq clock update skip requests
By renaming the functions we can get rid of the skip parameter
and have better code redability. It makes zero sense to have
things such as:
rq_clock_skip_update(rq, false)
When the skip request is in fact not going to happen. Ever. Rename
things such that we end up with:
rq_clock_skip_update(rq)
rq_clock_cancel_skipupdate(rq)
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
kernel/sched/core.c | 2 +-
kernel/sched/deadline.c | 2 +-
kernel/sched/fair.c | 2 +-
kernel/sched/rt.c | 2 +-
kernel/sched/sched.h | 17 ++++++++++++-----
5 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index de440456f15c..c4334a3350c5 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -874,7 +874,7 @@ void check_preempt_curr(struct rq *rq, struct task_struct *p, int flags)
* this case, we can save a useless back to back clock update.
*/
if (task_on_rq_queued(rq->curr) && test_tsk_need_resched(rq->curr))
- rq_clock_skip_update(rq, true);
+ rq_clock_skip_update(rq);
}
#ifdef CONFIG_SMP
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index d1c7bf7c7e5b..e7b3008b85bb 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -1560,7 +1560,7 @@ static void yield_task_dl(struct rq *rq)
* so we don't do microscopic update in schedule()
* and double the fastpath cost.
*/
- rq_clock_skip_update(rq, true);
+ rq_clock_skip_update(rq);
}
#ifdef CONFIG_SMP
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 0951d1c58d2f..54dc31e7ab9b 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7089,7 +7089,7 @@ static void yield_task_fair(struct rq *rq)
* so we don't do microscopic update in schedule()
* and double the fastpath cost.
*/
- rq_clock_skip_update(rq, true);
+ rq_clock_skip_update(rq);
}
set_skip_buddy(se);
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index ad13e6242481..7aef6b4e885a 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -861,7 +861,7 @@ static int do_sched_rt_period_timer(struct rt_bandwidth *rt_b, int overrun)
* 'runtime'.
*/
if (rt_rq->rt_nr_running && rq->curr == rq->idle)
- rq_clock_skip_update(rq, false);
+ rq_clock_cancel_skipupdate(rq);
}
if (rt_rq->rt_time || rt_rq->rt_nr_running)
idle = 0;
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index c3deaee7a7a2..15750c222ca2 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -976,13 +976,20 @@ static inline u64 rq_clock_task(struct rq *rq)
return rq->clock_task;
}
-static inline void rq_clock_skip_update(struct rq *rq, bool skip)
+static inline void rq_clock_skip_update(struct rq *rq)
{
lockdep_assert_held(&rq->lock);
- if (skip)
- rq->clock_update_flags |= RQCF_REQ_SKIP;
- else
- rq->clock_update_flags &= ~RQCF_REQ_SKIP;
+ rq->clock_update_flags |= RQCF_REQ_SKIP;
+}
+
+/*
+ * See rt task throttoling, which is the only time a skip
+ * request is cancelled.
+ */
+static inline void rq_clock_cancel_skipupdate(struct rq *rq)
+{
+ lockdep_assert_held(&rq->lock);
+ rq->clock_update_flags &= ~RQCF_REQ_SKIP;
}
struct rq_flags {
--
2.13.6
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] sched/rt: Fix rq->clock_update_flags < RQCF_ACT_SKIP warning
2018-04-04 16:15 ` Davidlohr Bueso
@ 2018-04-04 16:39 ` Peter Zijlstra
2018-04-05 7:21 ` Ingo Molnar
2018-04-05 9:17 ` [tip:sched/urgent] sched/core: Simplify helpers for rq clock update skip requests tip-bot for Davidlohr Bueso
1 sibling, 1 reply; 8+ messages in thread
From: Peter Zijlstra @ 2018-04-04 16:39 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: mingo, efault, rostedt, matt, linux-kernel, Davidlohr Bueso
On Wed, Apr 04, 2018 at 09:15:39AM -0700, Davidlohr Bueso wrote:
> On Mon, 02 Apr 2018, Davidlohr Bueso wrote:
>
> > The case for the rt task throttling (which this workload also hits) can be ignored in
> > that the skip_update call is actually bogus and quite the contrary (the request bits
> > are removed/reverted).
>
> While at it, how about this trivial patch?
>
> ----8<--------------------------------------------------------
> [PATCH 2/1] sched: Simplify helpers for rq clock update skip requests
>
> By renaming the functions we can get rid of the skip parameter
> and have better code redability. It makes zero sense to have
> things such as:
>
> rq_clock_skip_update(rq, false)
>
> When the skip request is in fact not going to happen. Ever. Rename
> things such that we end up with:
>
> rq_clock_skip_update(rq)
> rq_clock_cancel_skipupdate(rq)
>
> Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
Works for me :-)
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sched/rt: Fix rq->clock_update_flags < RQCF_ACT_SKIP warning
2018-04-04 16:39 ` Peter Zijlstra
@ 2018-04-05 7:21 ` Ingo Molnar
0 siblings, 0 replies; 8+ messages in thread
From: Ingo Molnar @ 2018-04-05 7:21 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Davidlohr Bueso, efault, rostedt, matt, linux-kernel, Davidlohr Bueso
* Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, Apr 04, 2018 at 09:15:39AM -0700, Davidlohr Bueso wrote:
> > On Mon, 02 Apr 2018, Davidlohr Bueso wrote:
> >
> > > The case for the rt task throttling (which this workload also hits) can be ignored in
> > > that the skip_update call is actually bogus and quite the contrary (the request bits
> > > are removed/reverted).
> >
> > While at it, how about this trivial patch?
> >
> > ----8<--------------------------------------------------------
> > [PATCH 2/1] sched: Simplify helpers for rq clock update skip requests
> >
> > By renaming the functions we can get rid of the skip parameter
> > and have better code redability. It makes zero sense to have
> > things such as:
> >
> > rq_clock_skip_update(rq, false)
> >
> > When the skip request is in fact not going to happen. Ever. Rename
> > things such that we end up with:
> >
> > rq_clock_skip_update(rq)
> > rq_clock_cancel_skipupdate(rq)
> >
> > Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
>
> Works for me :-)
>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
I have applied both patches, thanks guys!
Ingo
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tip:sched/urgent] sched/rt: Fix rq->clock_update_flags < RQCF_ACT_SKIP warning
2018-04-02 16:49 [PATCH] sched/rt: Fix rq->clock_update_flags < RQCF_ACT_SKIP warning Davidlohr Bueso
` (2 preceding siblings ...)
2018-04-04 16:15 ` Davidlohr Bueso
@ 2018-04-05 9:16 ` tip-bot for Davidlohr Bueso
3 siblings, 0 replies; 8+ messages in thread
From: tip-bot for Davidlohr Bueso @ 2018-04-05 9:16 UTC (permalink / raw)
To: linux-tip-commits
Cc: matt, dbueso, mingo, linux-kernel, torvalds, peterz, efault,
dave, tglx, hpa
Commit-ID: d29a20645d5e929aa7e8616f28e5d8e1c49263ec
Gitweb: https://git.kernel.org/tip/d29a20645d5e929aa7e8616f28e5d8e1c49263ec
Author: Davidlohr Bueso <dave@stgolabs.net>
AuthorDate: Mon, 2 Apr 2018 09:49:54 -0700
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 5 Apr 2018 09:20:46 +0200
sched/rt: Fix rq->clock_update_flags < RQCF_ACT_SKIP warning
While running rt-tests' pi_stress program I got the following splat:
rq->clock_update_flags < RQCF_ACT_SKIP
WARNING: CPU: 27 PID: 0 at kernel/sched/sched.h:960 assert_clock_updated.isra.38.part.39+0x13/0x20
[...]
<IRQ>
enqueue_top_rt_rq+0xf4/0x150
? cpufreq_dbs_governor_start+0x170/0x170
sched_rt_rq_enqueue+0x65/0x80
sched_rt_period_timer+0x156/0x360
? sched_rt_rq_enqueue+0x80/0x80
__hrtimer_run_queues+0xfa/0x260
hrtimer_interrupt+0xcb/0x220
smp_apic_timer_interrupt+0x62/0x120
apic_timer_interrupt+0xf/0x20
</IRQ>
[...]
do_idle+0x183/0x1e0
cpu_startup_entry+0x5f/0x70
start_secondary+0x192/0x1d0
secondary_startup_64+0xa5/0xb0
We can get rid of it be the "traditional" means of adding an
update_rq_clock() call after acquiring the rq->lock in
do_sched_rt_period_timer().
The case for the RT task throttling (which this workload also hits)
can be ignored in that the skip_update call is actually bogus and
quite the contrary (the request bits are removed/reverted).
By setting RQCF_UPDATED we really don't care if the skip is happening
or not and will therefore make the assert_clock_updated() check happy.
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
Reviewed-by: Matt Fleming <matt@codeblueprint.co.uk>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: dave@stgolabs.net
Cc: linux-kernel@vger.kernel.org
Cc: rostedt@goodmis.org
Link: http://lkml.kernel.org/r/20180402164954.16255-1-dave@stgolabs.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/sched/rt.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 86b77987435e..ad13e6242481 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -839,6 +839,8 @@ static int do_sched_rt_period_timer(struct rt_bandwidth *rt_b, int overrun)
continue;
raw_spin_lock(&rq->lock);
+ update_rq_clock(rq);
+
if (rt_rq->rt_time) {
u64 runtime;
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [tip:sched/urgent] sched/core: Simplify helpers for rq clock update skip requests
2018-04-04 16:15 ` Davidlohr Bueso
2018-04-04 16:39 ` Peter Zijlstra
@ 2018-04-05 9:17 ` tip-bot for Davidlohr Bueso
1 sibling, 0 replies; 8+ messages in thread
From: tip-bot for Davidlohr Bueso @ 2018-04-05 9:17 UTC (permalink / raw)
To: linux-tip-commits
Cc: dave, dbueso, torvalds, hpa, linux-kernel, peterz, efault, mingo, tglx
Commit-ID: adcc8da8859bee9548bb6d323b1e8de8a7252acd
Gitweb: https://git.kernel.org/tip/adcc8da8859bee9548bb6d323b1e8de8a7252acd
Author: Davidlohr Bueso <dave@stgolabs.net>
AuthorDate: Wed, 4 Apr 2018 09:15:39 -0700
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 5 Apr 2018 09:20:46 +0200
sched/core: Simplify helpers for rq clock update skip requests
By renaming the functions we can get rid of the skip parameter
and have better code redability. It makes zero sense to have
things such as:
rq_clock_skip_update(rq, false)
When the skip request is in fact not going to happen. Ever. Rename
things such that we end up with:
rq_clock_skip_update(rq)
rq_clock_cancel_skipupdate(rq)
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel@vger.kernel.org
Cc: matt@codeblueprint.co.uk
Cc: rostedt@goodmis.org
Link: http://lkml.kernel.org/r/20180404161539.nhadkff2aats74jh@linux-n805
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/sched/core.c | 2 +-
kernel/sched/deadline.c | 2 +-
kernel/sched/fair.c | 2 +-
kernel/sched/rt.c | 2 +-
kernel/sched/sched.h | 17 ++++++++++++-----
5 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 28b68995a417..550a07f648b6 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -874,7 +874,7 @@ void check_preempt_curr(struct rq *rq, struct task_struct *p, int flags)
* this case, we can save a useless back to back clock update.
*/
if (task_on_rq_queued(rq->curr) && test_tsk_need_resched(rq->curr))
- rq_clock_skip_update(rq, true);
+ rq_clock_skip_update(rq);
}
#ifdef CONFIG_SMP
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index d1c7bf7c7e5b..e7b3008b85bb 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -1560,7 +1560,7 @@ static void yield_task_dl(struct rq *rq)
* so we don't do microscopic update in schedule()
* and double the fastpath cost.
*/
- rq_clock_skip_update(rq, true);
+ rq_clock_skip_update(rq);
}
#ifdef CONFIG_SMP
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 0951d1c58d2f..54dc31e7ab9b 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7089,7 +7089,7 @@ static void yield_task_fair(struct rq *rq)
* so we don't do microscopic update in schedule()
* and double the fastpath cost.
*/
- rq_clock_skip_update(rq, true);
+ rq_clock_skip_update(rq);
}
set_skip_buddy(se);
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index ad13e6242481..7aef6b4e885a 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -861,7 +861,7 @@ static int do_sched_rt_period_timer(struct rt_bandwidth *rt_b, int overrun)
* 'runtime'.
*/
if (rt_rq->rt_nr_running && rq->curr == rq->idle)
- rq_clock_skip_update(rq, false);
+ rq_clock_cancel_skipupdate(rq);
}
if (rt_rq->rt_time || rt_rq->rt_nr_running)
idle = 0;
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index c3deaee7a7a2..15750c222ca2 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -976,13 +976,20 @@ static inline u64 rq_clock_task(struct rq *rq)
return rq->clock_task;
}
-static inline void rq_clock_skip_update(struct rq *rq, bool skip)
+static inline void rq_clock_skip_update(struct rq *rq)
{
lockdep_assert_held(&rq->lock);
- if (skip)
- rq->clock_update_flags |= RQCF_REQ_SKIP;
- else
- rq->clock_update_flags &= ~RQCF_REQ_SKIP;
+ rq->clock_update_flags |= RQCF_REQ_SKIP;
+}
+
+/*
+ * See rt task throttoling, which is the only time a skip
+ * request is cancelled.
+ */
+static inline void rq_clock_cancel_skipupdate(struct rq *rq)
+{
+ lockdep_assert_held(&rq->lock);
+ rq->clock_update_flags &= ~RQCF_REQ_SKIP;
}
struct rq_flags {
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-04-05 9:17 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-02 16:49 [PATCH] sched/rt: Fix rq->clock_update_flags < RQCF_ACT_SKIP warning Davidlohr Bueso
2018-04-03 12:39 ` Matt Fleming
2018-04-04 15:27 ` Peter Zijlstra
2018-04-04 16:15 ` Davidlohr Bueso
2018-04-04 16:39 ` Peter Zijlstra
2018-04-05 7:21 ` Ingo Molnar
2018-04-05 9:17 ` [tip:sched/urgent] sched/core: Simplify helpers for rq clock update skip requests tip-bot for Davidlohr Bueso
2018-04-05 9:16 ` [tip:sched/urgent] sched/rt: Fix rq->clock_update_flags < RQCF_ACT_SKIP warning tip-bot for Davidlohr Bueso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).