Re: [tip:sched/core] sched/fair: Limit sched_cfs_period_timer loop to avoid hard lockup
On Tue, Apr 09, 2019 at 03:05:27PM +0200 Peter Zijlstra wrote: > On Tue, Apr 09, 2019 at 08:48:16AM -0400, Phil Auld wrote: > > Hi Ingo, Peter, > > > > On Wed, Apr 03, 2019 at 01:38:39AM -0700 tip-bot for Phil Auld wrote: > > > Commit-ID: 06ec5d30e8d57b820d44df6340dcb25010d6d0fa > > > Gitweb: > > > https://git.kernel.org/tip/06ec5d30e8d57b820d44df6340dcb25010d6d0fa > > > Author: Phil Auld > > > AuthorDate: Tue, 19 Mar 2019 09:00:05 -0400 > > > Committer: Ingo Molnar > > > CommitDate: Wed, 3 Apr 2019 09:50:23 +0200 > > > > This commit seems to have gotten lost. It's not in tip and now the > > direct gitweb link is also showing bad commit reference. > > > > Did this fall victim to a reset or something? > > It had (trivial) builds fails on 32 bit. I have a fixed up version > around somewhere, but that hasn't made it back in yet. Thank you :) I'll post the follow up version for the stable trees soon. --
Re: [tip:sched/core] sched/fair: Limit sched_cfs_period_timer loop to avoid hard lockup
On Tue, Apr 09, 2019 at 03:05:27PM +0200 Peter Zijlstra wrote: > On Tue, Apr 09, 2019 at 08:48:16AM -0400, Phil Auld wrote: > > Hi Ingo, Peter, > > > > On Wed, Apr 03, 2019 at 01:38:39AM -0700 tip-bot for Phil Auld wrote: > > > Commit-ID: 06ec5d30e8d57b820d44df6340dcb25010d6d0fa > > > Gitweb: > > > https://git.kernel.org/tip/06ec5d30e8d57b820d44df6340dcb25010d6d0fa > > > Author: Phil Auld > > > AuthorDate: Tue, 19 Mar 2019 09:00:05 -0400 > > > Committer: Ingo Molnar > > > CommitDate: Wed, 3 Apr 2019 09:50:23 +0200 > > > > This commit seems to have gotten lost. It's not in tip and now the > > direct gitweb link is also showing bad commit reference. > > > > Did this fall victim to a reset or something? > > It had (trivial) builds fails on 32 bit. I have a fixed up version > around somewhere, but that hasn't made it back in yet. Friendly ping... Thanks, Phil --
Re: [tip:sched/core] sched/fair: Limit sched_cfs_period_timer loop to avoid hard lockup
On Tue, Apr 09, 2019 at 03:05:27PM +0200 Peter Zijlstra wrote: > On Tue, Apr 09, 2019 at 08:48:16AM -0400, Phil Auld wrote: > > Hi Ingo, Peter, > > > > On Wed, Apr 03, 2019 at 01:38:39AM -0700 tip-bot for Phil Auld wrote: > > > Commit-ID: 06ec5d30e8d57b820d44df6340dcb25010d6d0fa > > > Gitweb: > > > https://git.kernel.org/tip/06ec5d30e8d57b820d44df6340dcb25010d6d0fa > > > Author: Phil Auld > > > AuthorDate: Tue, 19 Mar 2019 09:00:05 -0400 > > > Committer: Ingo Molnar > > > CommitDate: Wed, 3 Apr 2019 09:50:23 +0200 > > > > This commit seems to have gotten lost. It's not in tip and now the > > direct gitweb link is also showing bad commit reference. > > > > Did this fall victim to a reset or something? > > It had (trivial) builds fails on 32 bit. I have a fixed up version > around somewhere, but that hasn't made it back in yet. Drat. Sorry about that. At least I'm not going crazy... Do you want me to fix it and push it back or will you do that? Thanks, Phil --
Re: [tip:sched/core] sched/fair: Limit sched_cfs_period_timer loop to avoid hard lockup
On Tue, Apr 09, 2019 at 08:48:16AM -0400, Phil Auld wrote: > Hi Ingo, Peter, > > On Wed, Apr 03, 2019 at 01:38:39AM -0700 tip-bot for Phil Auld wrote: > > Commit-ID: 06ec5d30e8d57b820d44df6340dcb25010d6d0fa > > Gitweb: > > https://git.kernel.org/tip/06ec5d30e8d57b820d44df6340dcb25010d6d0fa > > Author: Phil Auld > > AuthorDate: Tue, 19 Mar 2019 09:00:05 -0400 > > Committer: Ingo Molnar > > CommitDate: Wed, 3 Apr 2019 09:50:23 +0200 > > This commit seems to have gotten lost. It's not in tip and now the > direct gitweb link is also showing bad commit reference. > > Did this fall victim to a reset or something? It had (trivial) builds fails on 32 bit. I have a fixed up version around somewhere, but that hasn't made it back in yet.
Re: [tip:sched/core] sched/fair: Limit sched_cfs_period_timer loop to avoid hard lockup
Hi Ingo, Peter,
On Wed, Apr 03, 2019 at 01:38:39AM -0700 tip-bot for Phil Auld wrote:
> Commit-ID: 06ec5d30e8d57b820d44df6340dcb25010d6d0fa
> Gitweb:
> https://git.kernel.org/tip/06ec5d30e8d57b820d44df6340dcb25010d6d0fa
> Author: Phil Auld
> AuthorDate: Tue, 19 Mar 2019 09:00:05 -0400
> Committer: Ingo Molnar
> CommitDate: Wed, 3 Apr 2019 09:50:23 +0200
This commit seems to have gotten lost. It's not in tip and now the
direct gitweb link is also showing bad commit reference.
Did this fall victim to a reset or something?
Thanks,
Phil
>
> sched/fair: Limit sched_cfs_period_timer loop to avoid hard lockup
>
> With extremely short cfs_period_us setting on a parent task group with a large
> number of children the for loop in sched_cfs_period_timer can run until the
> watchdog fires. There is no guarantee that the call to hrtimer_forward_now()
> will ever return 0. The large number of children can make
> do_sched_cfs_period_timer() take longer than the period.
>
> NMI watchdog: Watchdog detected hard LOCKUP on cpu 24
> RIP: 0010:tg_nop+0x0/0x10
>
> walk_tg_tree_from+0x29/0xb0
> unthrottle_cfs_rq+0xe0/0x1a0
> distribute_cfs_runtime+0xd3/0xf0
> sched_cfs_period_timer+0xcb/0x160
> ? sched_cfs_slack_timer+0xd0/0xd0
> __hrtimer_run_queues+0xfb/0x270
> hrtimer_interrupt+0x122/0x270
> smp_apic_timer_interrupt+0x6a/0x140
> apic_timer_interrupt+0xf/0x20
>
>
> To prevent this we add protection to the loop that detects when the loop has
> run
> too many times and scales the period and quota up, proportionally, so that
> the timer
> can complete before then next period expires. This preserves the relative
> runtime
> quota while preventing the hard lockup.
>
> A warning is issued reporting this state and the new values.
>
> Signed-off-by: Phil Auld
> Signed-off-by: Peter Zijlstra (Intel)
> Cc: Anton Blanchard
> Cc: Ben Segall
> Cc: Linus Torvalds
> Cc: Mike Galbraith
> Cc: Peter Zijlstra
> Cc: Thomas Gleixner
> Cc:
> Link: https://lkml.kernel.org/r/20190319130005.25492-1-pa...@redhat.com
> Signed-off-by: Ingo Molnar
> ---
> kernel/sched/fair.c | 25 +
> 1 file changed, 25 insertions(+)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 40bd1e27b1b7..d4cce633eac8 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -4885,6 +4885,8 @@ static enum hrtimer_restart
> sched_cfs_slack_timer(struct hrtimer *timer)
> return HRTIMER_NORESTART;
> }
>
> +extern const u64 max_cfs_quota_period;
> +
> static enum hrtimer_restart sched_cfs_period_timer(struct hrtimer *timer)
> {
> struct cfs_bandwidth *cfs_b =
> @@ -4892,6 +4894,7 @@ static enum hrtimer_restart
> sched_cfs_period_timer(struct hrtimer *timer)
> unsigned long flags;
> int overrun;
> int idle = 0;
> + int count = 0;
>
> raw_spin_lock_irqsave(&cfs_b->lock, flags);
> for (;;) {
> @@ -4899,6 +4902,28 @@ static enum hrtimer_restart
> sched_cfs_period_timer(struct hrtimer *timer)
> if (!overrun)
> break;
>
> + if (++count > 3) {
> + u64 new, old = ktime_to_ns(cfs_b->period);
> +
> + new = (old * 147) / 128; /* ~115% */
> + new = min(new, max_cfs_quota_period);
> +
> + cfs_b->period = ns_to_ktime(new);
> +
> + /* since max is 1s, this is limited to 1e9^2, which
> fits in u64 */
> + cfs_b->quota *= new;
> + cfs_b->quota /= old;
> +
> + pr_warn_ratelimited(
> + "cfs_period_timer[cpu%d]: period too short, scaling up (new
> cfs_period_us %lld, cfs_quota_us = %lld)\n",
> + smp_processor_id(),
> + new/NSEC_PER_USEC,
> + cfs_b->quota/NSEC_PER_USEC);
> +
> + /* reset count so we don't come right back in here */
> + count = 0;
> + }
> +
> idle = do_sched_cfs_period_timer(cfs_b, overrun, flags);
> }
> if (idle)
--
[tip:sched/core] sched/fair: Limit sched_cfs_period_timer loop to avoid hard lockup
Commit-ID: 06ec5d30e8d57b820d44df6340dcb25010d6d0fa
Gitweb: https://git.kernel.org/tip/06ec5d30e8d57b820d44df6340dcb25010d6d0fa
Author: Phil Auld
AuthorDate: Tue, 19 Mar 2019 09:00:05 -0400
Committer: Ingo Molnar
CommitDate: Wed, 3 Apr 2019 09:50:23 +0200
sched/fair: Limit sched_cfs_period_timer loop to avoid hard lockup
With extremely short cfs_period_us setting on a parent task group with a large
number of children the for loop in sched_cfs_period_timer can run until the
watchdog fires. There is no guarantee that the call to hrtimer_forward_now()
will ever return 0. The large number of children can make
do_sched_cfs_period_timer() take longer than the period.
NMI watchdog: Watchdog detected hard LOCKUP on cpu 24
RIP: 0010:tg_nop+0x0/0x10
walk_tg_tree_from+0x29/0xb0
unthrottle_cfs_rq+0xe0/0x1a0
distribute_cfs_runtime+0xd3/0xf0
sched_cfs_period_timer+0xcb/0x160
? sched_cfs_slack_timer+0xd0/0xd0
__hrtimer_run_queues+0xfb/0x270
hrtimer_interrupt+0x122/0x270
smp_apic_timer_interrupt+0x6a/0x140
apic_timer_interrupt+0xf/0x20
To prevent this we add protection to the loop that detects when the loop has run
too many times and scales the period and quota up, proportionally, so that the
timer
can complete before then next period expires. This preserves the relative
runtime
quota while preventing the hard lockup.
A warning is issued reporting this state and the new values.
Signed-off-by: Phil Auld
Signed-off-by: Peter Zijlstra (Intel)
Cc: Anton Blanchard
Cc: Ben Segall
Cc: Linus Torvalds
Cc: Mike Galbraith
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Cc:
Link: https://lkml.kernel.org/r/20190319130005.25492-1-pa...@redhat.com
Signed-off-by: Ingo Molnar
---
kernel/sched/fair.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 40bd1e27b1b7..d4cce633eac8 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -4885,6 +4885,8 @@ static enum hrtimer_restart sched_cfs_slack_timer(struct
hrtimer *timer)
return HRTIMER_NORESTART;
}
+extern const u64 max_cfs_quota_period;
+
static enum hrtimer_restart sched_cfs_period_timer(struct hrtimer *timer)
{
struct cfs_bandwidth *cfs_b =
@@ -4892,6 +4894,7 @@ static enum hrtimer_restart sched_cfs_period_timer(struct
hrtimer *timer)
unsigned long flags;
int overrun;
int idle = 0;
+ int count = 0;
raw_spin_lock_irqsave(&cfs_b->lock, flags);
for (;;) {
@@ -4899,6 +4902,28 @@ static enum hrtimer_restart
sched_cfs_period_timer(struct hrtimer *timer)
if (!overrun)
break;
+ if (++count > 3) {
+ u64 new, old = ktime_to_ns(cfs_b->period);
+
+ new = (old * 147) / 128; /* ~115% */
+ new = min(new, max_cfs_quota_period);
+
+ cfs_b->period = ns_to_ktime(new);
+
+ /* since max is 1s, this is limited to 1e9^2, which
fits in u64 */
+ cfs_b->quota *= new;
+ cfs_b->quota /= old;
+
+ pr_warn_ratelimited(
+ "cfs_period_timer[cpu%d]: period too short, scaling up (new
cfs_period_us %lld, cfs_quota_us = %lld)\n",
+ smp_processor_id(),
+ new/NSEC_PER_USEC,
+ cfs_b->quota/NSEC_PER_USEC);
+
+ /* reset count so we don't come right back in here */
+ count = 0;
+ }
+
idle = do_sched_cfs_period_timer(cfs_b, overrun, flags);
}
if (idle)
