Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1313255 > unrolled thread
| Started by | Michal Hocko <mhocko@kernel.org> |
|---|---|
| First post | 2016-01-20 15:40 +0100 |
| Last post | 2016-01-20 16:30 +0100 |
| Articles | 10 on this page of 50 — 6 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: mm, vmstat: kernel BUG at mm/vmstat.c:1408! Michal Hocko <mhocko@kernel.org> - 2016-01-20 15:40 +0100
Re: mm, vmstat: kernel BUG at mm/vmstat.c:1408! Sasha Levin <sasha.levin@oracle.com> - 2016-01-20 16:00 +0100
Re: mm, vmstat: kernel BUG at mm/vmstat.c:1408! Michal Hocko <mhocko@kernel.org> - 2016-01-20 16:20 +0100
Re: mm, vmstat: kernel BUG at mm/vmstat.c:1408! Christoph Lameter <cl@linux.com> - 2016-01-20 16:30 +0100
Re: mm, vmstat: kernel BUG at mm/vmstat.c:1408! Sasha Levin <sasha.levin@oracle.com> - 2016-01-20 17:00 +0100
Re: mm, vmstat: kernel BUG at mm/vmstat.c:1408! Christoph Lameter <cl@linux.com> - 2016-01-20 17:00 +0100
Re: mm, vmstat: kernel BUG at mm/vmstat.c:1408! Michal Hocko <mhocko@kernel.org> - 2016-01-20 22:30 +0100
Re: mm, vmstat: kernel BUG at mm/vmstat.c:1408! Christoph Lameter <cl@linux.com> - 2016-01-20 23:00 +0100
Re: mm, vmstat: kernel BUG at mm/vmstat.c:1408! Michal Hocko <mhocko@kernel.org> - 2016-01-21 09:30 +0100
Re: mm, vmstat: kernel BUG at mm/vmstat.c:1408! Christoph Lameter <cl@linux.com> - 2016-01-21 16:50 +0100
Re: mm, vmstat: kernel BUG at mm/vmstat.c:1408! Michal Hocko <mhocko@kernel.org> - 2016-01-21 18:00 +0100
Re: mm, vmstat: kernel BUG at mm/vmstat.c:1408! Christoph Lameter <cl@linux.com> - 2016-01-21 18:40 +0100
Re: mm, vmstat: kernel BUG at mm/vmstat.c:1408! Shiraz Hashim <shiraz.linux.kernel@gmail.com> - 2016-01-22 12:10 +0100
Re: mm, vmstat: kernel BUG at mm/vmstat.c:1408! Michal Hocko <mhocko@kernel.org> - 2016-01-22 15:10 +0100
Re: mm, vmstat: kernel BUG at mm/vmstat.c:1408! Christoph Lameter <cl@linux.com> - 2016-01-22 17:10 +0100
Re: mm, vmstat: kernel BUG at mm/vmstat.c:1408! Michal Hocko <mhocko@kernel.org> - 2016-01-22 17:20 +0100
Re: mm, vmstat: kernel BUG at mm/vmstat.c:1408! Christoph Lameter <cl@linux.com> - 2016-01-22 17:50 +0100
Re: mm, vmstat: kernel BUG at mm/vmstat.c:1408! Michal Hocko <mhocko@kernel.org> - 2016-01-22 18:20 +0100
fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Mike Galbraith <umgwanakikbuti@gmail.com> - 2016-01-23 17:30 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Christoph Lameter <cl@linux.com> - 2016-01-24 01:40 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Mike Galbraith <umgwanakikbuti@gmail.com> - 2016-01-24 03:50 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Christoph Lameter <cl@linux.com> - 2016-01-24 04:50 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Mike Galbraith <umgwanakikbuti@gmail.com> - 2016-01-24 06:40 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Michal Hocko <mhocko@kernel.org> - 2016-01-25 18:50 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Christoph Lameter <cl@linux.com> - 2016-01-25 19:10 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Michal Hocko <mhocko@kernel.org> - 2016-01-25 21:20 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Christoph Lameter <cl@linux.com> - 2016-01-26 17:30 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Christoph Lameter <cl@linux.com> - 2016-01-26 19:40 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Mike Galbraith <umgwanakikbuti@gmail.com> - 2016-01-26 19:50 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Christoph Lameter <cl@linux.com> - 2016-01-26 20:30 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Mike Galbraith <umgwanakikbuti@gmail.com> - 2016-01-27 04:20 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Mike Galbraith <umgwanakikbuti@gmail.com> - 2016-01-27 05:20 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Christoph Lameter <cl@linux.com> - 2016-01-27 17:30 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Mike Galbraith <umgwanakikbuti@gmail.com> - 2016-01-26 19:40 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Mike Galbraith <umgwanakikbuti@gmail.com> - 2016-01-26 03:20 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Mike Galbraith <umgwanakikbuti@gmail.com> - 2016-01-26 03:30 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Christoph Lameter <cl@linux.com> - 2016-01-26 17:30 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Mike Galbraith <umgwanakikbuti@gmail.com> - 2016-01-26 18:40 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Christoph Lameter <cl@linux.com> - 2016-01-26 19:20 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Christoph Lameter <cl@linux.com> - 2016-01-26 17:30 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Mike Galbraith <umgwanakikbuti@gmail.com> - 2016-01-26 18:10 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Christoph Lameter <cl@linux.com> - 2016-01-26 19:30 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Mike Galbraith <umgwanakikbuti@gmail.com> - 2016-01-26 20:10 +0100
Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) Christoph Lameter <cl@linux.com> - 2016-01-26 20:30 +0100
[PATCH] mm, vmstat: make quiet_vmstat lighter (was: Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable) again and shut down on idle) Michal Hocko <mhocko@kernel.org> - 2016-01-27 17:50 +0100
Re: [PATCH] mm, vmstat: make quiet_vmstat lighter (was: Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable) again and shut down on idle) Mike Galbraith <umgwanakikbuti@gmail.com> - 2016-01-27 18:10 +0100
Re: [PATCH] mm, vmstat: make quiet_vmstat lighter (was: Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable) again and shut down on idle) Christoph Lameter <cl@linux.com> - 2016-01-27 19:30 +0100
Re: mm, vmstat: kernel BUG at mm/vmstat.c:1408! Linus Torvalds <torvalds@linux-foundation.org> - 2016-01-24 18:00 +0100
Re: mm, vmstat: kernel BUG at mm/vmstat.c:1408! Christoph Lameter <cl@linux.com> - 2016-01-20 16:20 +0100
Re: mm, vmstat: kernel BUG at mm/vmstat.c:1408! Michal Hocko <mhocko@kernel.org> - 2016-01-20 16:30 +0100
Page 3 of 3 — ← Prev page 1 2 [3]
| From | Mike Galbraith <umgwanakikbuti@gmail.com> |
|---|---|
| Date | 2016-01-26 18:10 +0100 |
| Subject | Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) |
| Message-ID | <qVfhV-7dM-31@gated-at.bofh.it> |
| In reply to | #1318120 |
On Tue, 2016-01-26 at 10:26 -0600, Christoph Lameter wrote: > On Tue, 26 Jan 2016, Mike Galbraith wrote: > > > > Why would the deferring cause this overhead? > > > > Because we schedule to idle cores aggressively, thus we may pop in and > > out of idle at high frequency. > > Whats the point of going idle if you have things to do soon? When a task schedules off, how do you know it'll be back at all, much less soon? -Mike
[toc] | [prev] | [next] | [standalone]
| From | Christoph Lameter <cl@linux.com> |
|---|---|
| Date | 2016-01-26 19:30 +0100 |
| Subject | Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) |
| Message-ID | <qVgxl-83o-21@gated-at.bofh.it> |
| In reply to | #1318154 |
On Tue, 26 Jan 2016, Mike Galbraith wrote: > On Tue, 2016-01-26 at 10:26 -0600, Christoph Lameter wrote: > > On Tue, 26 Jan 2016, Mike Galbraith wrote: > > > > > > Why would the deferring cause this overhead? > > > > > > Because we schedule to idle cores aggressively, thus we may pop in and > > > out of idle at high frequency. > > > > Whats the point of going idle if you have things to do soon? > > When a task schedules off, how do you know it'll be back at all, much > less soon? Ok so you are running an artificial benchmark that always gets the system running again when it decides to go idle?
[toc] | [prev] | [next] | [standalone]
| From | Mike Galbraith <umgwanakikbuti@gmail.com> |
|---|---|
| Date | 2016-01-26 20:10 +0100 |
| Subject | Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) |
| Message-ID | <qVha3-8N-45@gated-at.bofh.it> |
| In reply to | #1318224 |
On Tue, 2016-01-26 at 12:22 -0600, Christoph Lameter wrote: > On Tue, 26 Jan 2016, Mike Galbraith wrote: > > > On Tue, 2016-01-26 at 10:26 -0600, Christoph Lameter wrote: > > > On Tue, 26 Jan 2016, Mike Galbraith wrote: > > > > > > > > Why would the deferring cause this overhead? > > > > > > > > Because we schedule to idle cores aggressively, thus we may pop > > > > in and > > > > out of idle at high frequency. > > > > > > Whats the point of going idle if you have things to do soon? > > > > When a task schedules off, how do you know it'll be back at all, > > much > > less soon? > > Ok so you are running an artificial benchmark that always gets the > system running again when it decides to go idle? The benchmark does not alter the cycle expenditure per event. The real world will pay the toll less frequently than the artificial benchmark, yes, but it will pay nonetheless, and for most, needlessly. Or? -Mike
[toc] | [prev] | [next] | [standalone]
| From | Christoph Lameter <cl@linux.com> |
|---|---|
| Date | 2016-01-26 20:30 +0100 |
| Subject | Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable again and shut down on idle) |
| Message-ID | <qVhtq-gQ-85@gated-at.bofh.it> |
| In reply to | #1318267 |
On Tue, 26 Jan 2016, Mike Galbraith wrote: > > Ok so you are running an artificial benchmark that always gets the > > system running again when it decides to go idle? > > The benchmark does not alter the cycle expenditure per event. The real > world will pay the toll less frequently than the artificial benchmark, > yes, but it will pay nonetheless, and for most, needlessly. Or? Well if it does not pay it there then it is going to pay for it later when the vmstat_updater needs to cause a wakeup to fold the data. Also not folding the vmstat updates increases the time that minor updates are deferred and thus impacts the accuracy of the global vm counters. That seems to have been important recently.
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-01-27 17:50 +0100 |
| Subject | [PATCH] mm, vmstat: make quiet_vmstat lighter (was: Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable) again and shut down on idle) |
| Message-ID | <qVBs7-6oZ-53@gated-at.bofh.it> |
| In reply to | #1315680 |
On Sat 23-01-16 17:21:55, Mike Galbraith wrote:
> Hi Christoph,
>
> While you're fixing that commit up, can you perhaps find a better home
> for quiet_vmstat()? It not only munches cycles when switching cross
> -core mightily, for -rt it injects a sleeping lock into the idle task.
>
> 12.89% [kernel] [k] refresh_cpu_vm_stats.isra.12
> 4.75% [kernel] [k] __schedule
> 4.70% [kernel] [k] mutex_unlock
> 3.14% [kernel] [k] __switch_to
What about the following fix?
---
From c74a04c4fdfe1fa67933bb1ac83d3de0532aaab2 Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.com>
Date: Wed, 27 Jan 2016 17:24:22 +0100
Subject: [PATCH] mm, vmstat: make quiet_vmstat lighter
Mike has reported a considerable overhead of refresh_cpu_vm_stats from
the idle entry during pipe test:
12.89% [kernel] [k] refresh_cpu_vm_stats.isra.12
4.75% [kernel] [k] __schedule
4.70% [kernel] [k] mutex_unlock
3.14% [kernel] [k] __switch_to
This is caused by 0eb77e988032 ("vmstat: make vmstat_updater deferrable
again and shut down on idle") which has placed quiet_vmstat into
cpu_idle_loop. The main reason here seems to be that the idle entry has
to get over all zones and perform atomic operations for each vmstat
entry even though there might be no per cpu diffs. This is a pointless
overhead for _each_ idle entry.
Make sure that quiet_vmstat is as light as possible.
First of all it doesn't make any sense to do any local sync if the
current cpu is already set in oncpu_stat_off because vmstat_update puts
itself there only if there is nothing to do.
Then we can check need_update which should be a cheap way to check for
potential per-cpu diffs and only then do refresh_cpu_vm_stats.
The original patch also did cancel_delayed_work which we are not doing
here. There are two reasons for that. Firstly cancel_delayed_work from
idle context will blow up on RT kernels (reported by Mike):
[ 2.279582] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.5.0-rt3 #7
[ 2.280444] Hardware name: MEDION MS-7848/MS-7848, BIOS M7848W08.20C 09/23/2013
[ 2.281316] ffff88040b00d640 ffff88040b01fe10 ffffffff812d20e2 0000000000000000
[ 2.282202] ffff88040b01fe30 ffffffff81081095 ffff88041ec4cee0 ffff88041ec501e0
[ 2.283073] ffff88040b01fe48 ffffffff815ff910 ffff88041ec4cee0 ffff88040b01fe88
[ 2.283941] Call Trace:
[ 2.284797] [<ffffffff812d20e2>] dump_stack+0x49/0x67
[ 2.285658] [<ffffffff81081095>] ___might_sleep+0xf5/0x180
[ 2.286521] [<ffffffff815ff910>] rt_spin_lock+0x20/0x50
[ 2.287382] [<ffffffff81075919>] try_to_grab_pending+0x69/0x240
[ 2.288239] [<ffffffff81075b16>] cancel_delayed_work+0x26/0xe0
[ 2.289094] [<ffffffff8115ec05>] quiet_vmstat+0x75/0xa0
[ 2.289949] [<ffffffff8109ab38>] cpu_idle_loop+0x38/0x3e0
[ 2.290800] [<ffffffff8109aef3>] cpu_startup_entry+0x13/0x20
[ 2.291647] [<ffffffff81036164>] start_secondary+0x114/0x140
And secondly, even on !RT kernels it might add some non trivial overhead
which is not necessary. Even if the vmstat worker wakes up and preempts
idle then it will be most likely a single shot noop because the stats
were already synced and so it would end up on the oncpu_stat_off anyway.
We just need to teach both vmstat_shepherd and vmstat_update to stop
scheduling the worker if there is nothing to do.
Reported-by: Mike Galbraith <umgwanakikbuti@gmail.com>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
mm/vmstat.c | 60 ++++++++++++++++++++++++++++++++++++++++--------------------
1 file changed, 40 insertions(+), 20 deletions(-)
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 40b2c74ddf16..7747f85537b6 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -1396,10 +1396,15 @@ static void vmstat_update(struct work_struct *w)
* Counters were updated so we expect more updates
* to occur in the future. Keep on running the
* update worker thread.
+ * If we were marked on cpu_stat_off clear the flag
+ * so that vmstat_shepherd doesn't schedule us again.
*/
- queue_delayed_work_on(smp_processor_id(), vmstat_wq,
- this_cpu_ptr(&vmstat_work),
- round_jiffies_relative(sysctl_stat_interval));
+ if (!cpumask_test_and_clear_cpu(smp_processor_id(),
+ cpu_stat_off)) {
+ queue_delayed_work_on(smp_processor_id(), vmstat_wq,
+ this_cpu_ptr(&vmstat_work),
+ round_jiffies_relative(sysctl_stat_interval));
+ }
} else {
/*
* We did not update any counters so the app may be in
@@ -1417,18 +1422,6 @@ static void vmstat_update(struct work_struct *w)
* until the diffs stay at zero. The function is used by NOHZ and can only be
* invoked when tick processing is not active.
*/
-void quiet_vmstat(void)
-{
- if (system_state != SYSTEM_RUNNING)
- return;
-
- do {
- if (!cpumask_test_and_set_cpu(smp_processor_id(), cpu_stat_off))
- cancel_delayed_work(this_cpu_ptr(&vmstat_work));
-
- } while (refresh_cpu_vm_stats(false));
-}
-
/*
* Check if the diffs for a certain cpu indicate that
* an update is needed.
@@ -1452,6 +1445,30 @@ static bool need_update(int cpu)
return false;
}
+void quiet_vmstat(void)
+{
+ if (system_state != SYSTEM_RUNNING)
+ return;
+
+ /*
+ * If we are already in hands of the shepherd then there
+ * is nothing for us to do here.
+ */
+ if (cpumask_test_and_set_cpu(smp_processor_id(), cpu_stat_off))
+ return;
+
+ if (!need_update(smp_processor_id()))
+ return;
+
+ /*
+ * Just refresh counters and do not care about the pending delayed
+ * vmstat_update. It doesn't fire that often to matter and canceling
+ * it would be too expensive from this path.
+ * vmstat_shepherd will take care about that for us.
+ */
+ refresh_cpu_vm_stats(false);
+}
+
/*
* Shepherd worker thread that checks the
@@ -1470,11 +1487,14 @@ static void vmstat_shepherd(struct work_struct *w)
get_online_cpus();
/* Check processors whose vmstat worker threads have been disabled */
for_each_cpu(cpu, cpu_stat_off)
- if (need_update(cpu) &&
- cpumask_test_and_clear_cpu(cpu, cpu_stat_off))
-
- queue_delayed_work_on(cpu, vmstat_wq,
- &per_cpu(vmstat_work, cpu), 0);
+ if (need_update(cpu)) {
+ if (cpumask_test_and_clear_cpu(cpu, cpu_stat_off))
+ queue_delayed_work_on(cpu, vmstat_wq,
+ &per_cpu(vmstat_work, cpu), 0);
+ } else {
+ cpumask_set_cpu(cpu, cpu_stat_off);
+ cancel_delayed_work(this_cpu_ptr(&vmstat_work));
+ }
put_online_cpus();
--
2.7.0.rc3
--
Michal Hocko
SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Mike Galbraith <umgwanakikbuti@gmail.com> |
|---|---|
| Date | 2016-01-27 18:10 +0100 |
| Subject | Re: [PATCH] mm, vmstat: make quiet_vmstat lighter (was: Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable) again and shut down on idle) |
| Message-ID | <qVBLs-6NS-9@gated-at.bofh.it> |
| In reply to | #1319153 |
On Wed, 2016-01-27 at 17:48 +0100, Michal Hocko wrote:
> On Sat 23-01-16 17:21:55, Mike Galbraith wrote:
> > Hi Christoph,
> >
> > While you're fixing that commit up, can you perhaps find a better
> > home
> > for quiet_vmstat()? It not only munches cycles when switching
> > cross
> > -core mightily, for -rt it injects a sleeping lock into the idle
> > task.
> >
> > 12.89% [kernel] [k] refresh_cpu_vm_stats.isra.12
> > 4.75% [kernel] [k] __schedule
> > 4.70% [kernel] [k] mutex_unlock
> > 3.14% [kernel] [k] __switch_to
>
> What about the following fix?
I haven't done _extensive_ testing, but 4.5-rc1-rt (with NO_HZ_FULL
enabling hacks restored, as it's otherwise a dead .config item) is
gripe free whether nohz_full CPUs are in use or not, and overhead went
away in it and desktop configured master kernel (PREEMPT_VOLUNTARY).
> ---
> From c74a04c4fdfe1fa67933bb1ac83d3de0532aaab2 Mon Sep 17 00:00:00
> 2001
> From: Michal Hocko <mhocko@suse.com>
> Date: Wed, 27 Jan 2016 17:24:22 +0100
> Subject: [PATCH] mm, vmstat: make quiet_vmstat lighter
>
> Mike has reported a considerable overhead of refresh_cpu_vm_stats
> from
> the idle entry during pipe test:
> 12.89% [kernel] [k] refresh_cpu_vm_stats.isra.12
> 4.75% [kernel] [k] __schedule
> 4.70% [kernel] [k] mutex_unlock
> 3.14% [kernel] [k] __switch_to
>
> This is caused by 0eb77e988032 ("vmstat: make vmstat_updater
> deferrable
> again and shut down on idle") which has placed quiet_vmstat into
> cpu_idle_loop. The main reason here seems to be that the idle entry
> has
> to get over all zones and perform atomic operations for each vmstat
> entry even though there might be no per cpu diffs. This is a
> pointless
> overhead for _each_ idle entry.
>
> Make sure that quiet_vmstat is as light as possible.
>
> First of all it doesn't make any sense to do any local sync if the
> current cpu is already set in oncpu_stat_off because vmstat_update
> puts
> itself there only if there is nothing to do.
>
> Then we can check need_update which should be a cheap way to check
> for
> potential per-cpu diffs and only then do refresh_cpu_vm_stats.
>
> The original patch also did cancel_delayed_work which we are not
> doing
> here. There are two reasons for that. Firstly cancel_delayed_work
> from
> idle context will blow up on RT kernels (reported by Mike):
> [ 2.279582] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.5.0-rt3 #7
> [ 2.280444] Hardware name: MEDION MS-7848/MS-7848, BIOS
> M7848W08.20C 09/23/2013
> [ 2.281316] ffff88040b00d640 ffff88040b01fe10 ffffffff812d20e2
> 0000000000000000
> [ 2.282202] ffff88040b01fe30 ffffffff81081095 ffff88041ec4cee0
> ffff88041ec501e0
> [ 2.283073] ffff88040b01fe48 ffffffff815ff910 ffff88041ec4cee0
> ffff88040b01fe88
> [ 2.283941] Call Trace:
> [ 2.284797] [<ffffffff812d20e2>] dump_stack+0x49/0x67
> [ 2.285658] [<ffffffff81081095>] ___might_sleep+0xf5/0x180
> [ 2.286521] [<ffffffff815ff910>] rt_spin_lock+0x20/0x50
> [ 2.287382] [<ffffffff81075919>] try_to_grab_pending+0x69/0x240
> [ 2.288239] [<ffffffff81075b16>] cancel_delayed_work+0x26/0xe0
> [ 2.289094] [<ffffffff8115ec05>] quiet_vmstat+0x75/0xa0
> [ 2.289949] [<ffffffff8109ab38>] cpu_idle_loop+0x38/0x3e0
> [ 2.290800] [<ffffffff8109aef3>] cpu_startup_entry+0x13/0x20
> [ 2.291647] [<ffffffff81036164>] start_secondary+0x114/0x140
>
> And secondly, even on !RT kernels it might add some non trivial
> overhead
> which is not necessary. Even if the vmstat worker wakes up and
> preempts
> idle then it will be most likely a single shot noop because the stats
> were already synced and so it would end up on the oncpu_stat_off
> anyway.
> We just need to teach both vmstat_shepherd and vmstat_update to stop
> scheduling the worker if there is nothing to do.
>
> Reported-by: Mike Galbraith <umgwanakikbuti@gmail.com>
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
> mm/vmstat.c | 60 ++++++++++++++++++++++++++++++++++++++++-----------
> ---------
> 1 file changed, 40 insertions(+), 20 deletions(-)
>
> diff --git a/mm/vmstat.c b/mm/vmstat.c
> index 40b2c74ddf16..7747f85537b6 100644
> --- a/mm/vmstat.c
> +++ b/mm/vmstat.c
> @@ -1396,10 +1396,15 @@ static void vmstat_update(struct work_struct
> *w)
> * Counters were updated so we expect more updates
> * to occur in the future. Keep on running the
> * update worker thread.
> + * If we were marked on cpu_stat_off clear the flag
> + * so that vmstat_shepherd doesn't schedule us
> again.
> */
> - queue_delayed_work_on(smp_processor_id(), vmstat_wq,
> - this_cpu_ptr(&vmstat_work),
> - round_jiffies_relative(sysctl_stat_interval)
> );
> + if (!cpumask_test_and_clear_cpu(smp_processor_id(),
> + cpu_stat_off)) {
> + queue_delayed_work_on(smp_processor_id(),
> vmstat_wq,
> + this_cpu_ptr(&vmstat_work),
> + round_jiffies_relative(sysctl_stat_i
> nterval));
> + }
> } else {
> /*
> * We did not update any counters so the app may be
> in
> @@ -1417,18 +1422,6 @@ static void vmstat_update(struct work_struct
> *w)
> * until the diffs stay at zero. The function is used by NOHZ and
> can only be
> * invoked when tick processing is not active.
> */
> -void quiet_vmstat(void)
> -{
> - if (system_state != SYSTEM_RUNNING)
> - return;
> -
> - do {
> - if (!cpumask_test_and_set_cpu(smp_processor_id(),
> cpu_stat_off))
> - cancel_delayed_work(this_cpu_ptr(&vmstat_wor
> k));
> -
> - } while (refresh_cpu_vm_stats(false));
> -}
> -
> /*
> * Check if the diffs for a certain cpu indicate that
> * an update is needed.
> @@ -1452,6 +1445,30 @@ static bool need_update(int cpu)
> return false;
> }
>
> +void quiet_vmstat(void)
> +{
> + if (system_state != SYSTEM_RUNNING)
> + return;
> +
> + /*
> + * If we are already in hands of the shepherd then there
> + * is nothing for us to do here.
> + */
> + if (cpumask_test_and_set_cpu(smp_processor_id(),
> cpu_stat_off))
> + return;
> +
> + if (!need_update(smp_processor_id()))
> + return;
> +
> + /*
> + * Just refresh counters and do not care about the pending
> delayed
> + * vmstat_update. It doesn't fire that often to matter and
> canceling
> + * it would be too expensive from this path.
> + * vmstat_shepherd will take care about that for us.
> + */
> + refresh_cpu_vm_stats(false);
> +}
> +
>
> /*
> * Shepherd worker thread that checks the
> @@ -1470,11 +1487,14 @@ static void vmstat_shepherd(struct
> work_struct *w)
> get_online_cpus();
> /* Check processors whose vmstat worker threads have been
> disabled */
> for_each_cpu(cpu, cpu_stat_off)
> - if (need_update(cpu) &&
> - cpumask_test_and_clear_cpu(cpu,
> cpu_stat_off))
> -
> - queue_delayed_work_on(cpu, vmstat_wq,
> - &per_cpu(vmstat_work, cpu), 0);
> + if (need_update(cpu)) {
> + if (cpumask_test_and_clear_cpu(cpu,
> cpu_stat_off))
> + queue_delayed_work_on(cpu,
> vmstat_wq,
> + &per_cpu(vmstat_work, cpu),
> 0);
> + } else {
> + cpumask_set_cpu(cpu, cpu_stat_off);
> + cancel_delayed_work(this_cpu_ptr(&vmstat_wor
> k));
> + }
>
> put_online_cpus();
>
> --
> 2.7.0.rc3
>
[toc] | [prev] | [next] | [standalone]
| From | Christoph Lameter <cl@linux.com> |
|---|---|
| Date | 2016-01-27 19:30 +0100 |
| Subject | Re: [PATCH] mm, vmstat: make quiet_vmstat lighter (was: Re: fast path cycle muncher (vmstat: make vmstat_updater deferrable) again and shut down on idle) |
| Message-ID | <qVD0S-7IE-25@gated-at.bofh.it> |
| In reply to | #1319153 |
On Wed, 27 Jan 2016, Michal Hocko wrote:
> This is caused by 0eb77e988032 ("vmstat: make vmstat_updater deferrable
> again and shut down on idle") which has placed quiet_vmstat into
> cpu_idle_loop. The main reason here seems to be that the idle entry has
> to get over all zones and perform atomic operations for each vmstat
> entry even though there might be no per cpu diffs. This is a pointless
> overhead for _each_ idle entry.
>
> Make sure that quiet_vmstat is as light as possible.
>
> First of all it doesn't make any sense to do any local sync if the
> current cpu is already set in oncpu_stat_off because vmstat_update puts
> itself there only if there is nothing to do.
But there might be something to do that happened in the meantime because
counters were incremented. It needs to be checked. The shepherd can do
that but it will delay the folding of diffs for awhile.
> +void quiet_vmstat(void)
> +{
> + if (system_state != SYSTEM_RUNNING)
> + return;
> +
> + /*
> + * If we are already in hands of the shepherd then there
> + * is nothing for us to do here.
> + */
> + if (cpumask_test_and_set_cpu(smp_processor_id(), cpu_stat_off))
> + return;
> +
> + if (!need_update(smp_processor_id()))
> + return;
> +
> + /*
> + * Just refresh counters and do not care about the pending delayed
> + * vmstat_update. It doesn't fire that often to matter and canceling
> + * it would be too expensive from this path.
> + * vmstat_shepherd will take care about that for us.
> + */
> + refresh_cpu_vm_stats(false);
> +}
The problem here is that there will be an additional tick generated on
idle. This is an issue for power because now the processor has to
needlessly wake up again, do tick processing etc just to effectively do a
cancel_delayed_work().
The cancelling of the work request is required to avoid this additonal
tick.
> @@ -1470,11 +1487,14 @@ static void vmstat_shepherd(struct work_struct *w)
> get_online_cpus();
> /* Check processors whose vmstat worker threads have been disabled */
> for_each_cpu(cpu, cpu_stat_off)
> - if (need_update(cpu) &&
> - cpumask_test_and_clear_cpu(cpu, cpu_stat_off))
> -
> - queue_delayed_work_on(cpu, vmstat_wq,
> - &per_cpu(vmstat_work, cpu), 0);
> + if (need_update(cpu)) {
> + if (cpumask_test_and_clear_cpu(cpu, cpu_stat_off))
> + queue_delayed_work_on(cpu, vmstat_wq,
> + &per_cpu(vmstat_work, cpu), 0);
> + } else {
> + cpumask_set_cpu(cpu, cpu_stat_off);
Umm the flag is already set right? This just causes a bouncing cacheline
since we are accessing a global set of cpus,. Drop this line?
> + cancel_delayed_work(this_cpu_ptr(&vmstat_work));
Ok so this is to move the cancel into the shepherd.
Aside from the two issues mentioned this looks good.
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2016-01-24 18:00 +0100 |
| Message-ID | <qUwb8-7Bw-3@gated-at.bofh.it> |
| In reply to | #1315126 |
On Fri, Jan 22, 2016 at 8:46 AM, Christoph Lameter <cl@linux.com> wrote:
>
> Subject: vmstat: Remove BUG_ON from vmstat_update
>
> If we detect that there is nothing to do just set the flag and do not check
> if it was already set before. [..]
Ok, I am assuming this is in Andrew's queue already, but this bug hit
my machine overnight, so I'm applying it directly..
Linus
[toc] | [prev] | [next] | [standalone]
| From | Christoph Lameter <cl@linux.com> |
|---|---|
| Date | 2016-01-20 16:20 +0100 |
| Message-ID | <qT2Ib-8li-49@gated-at.bofh.it> |
| In reply to | #1313255 |
On Wed, 20 Jan 2016, Michal Hocko wrote: > [CCing Andrew] > > I am just reading through this old discussion again because "vmstat: > make vmstat_updater deferrable again and shut down on idle" which seems > to be the culprit AFAIU has been merged as 0eb77e988032 and I do not see > any follow up fix merged to linus tree Is there any way to reproce this issue? This is running through trinity right? Can we please get the exact syscall that causes this to occur?
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-01-20 16:30 +0100 |
| Message-ID | <qT2RQ-8pt-15@gated-at.bofh.it> |
| In reply to | #1313292 |
On Wed 20-01-16 09:14:06, Christoph Lameter wrote: > On Wed, 20 Jan 2016, Michal Hocko wrote: > > > [CCing Andrew] > > > > I am just reading through this old discussion again because "vmstat: > > make vmstat_updater deferrable again and shut down on idle" which seems > > to be the culprit AFAIU has been merged as 0eb77e988032 and I do not see > > any follow up fix merged to linus tree > > Is there any way to reproce this issue? This is running through trinity > right? Can we please get the exact syscall that causes this to occur? As per the backtrace in the initial report this seems to be time dependent as the crash happens from _kthread_ context. So it doesn't seem to be directly related to any particular syscall. -- Michal Hocko SUSE Labs
[toc] | [prev] | [standalone]
Page 3 of 3 — ← Prev page 1 2 [3]
Back to top | Article view | linux.kernel
csiph-web