It is not necessary to queue work item to run refresh_vm_stats
on a remote CPU if that CPU has no dirty stats and no per-CPU
allocations for remote nodes.
This fixes sosreport hang (which uses vmstat_refresh) with
spinning SCHED_FIFO process.
Signed-off-by: Marcelo Tosatti <[email protected]>
---
mm/vmstat.c | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 44 insertions(+), 5 deletions(-)
Index: linux-2.6/mm/vmstat.c
===================================================================
--- linux-2.6.orig/mm/vmstat.c
+++ linux-2.6/mm/vmstat.c
@@ -1851,6 +1851,31 @@ static bool need_update(int cpu)
}
#ifdef CONFIG_PROC_FS
+static bool need_drain_remote_zones(int cpu)
+{
+#ifdef CONFIG_NUMA
+ struct zone *zone;
+
+ for_each_populated_zone(zone) {
+ struct per_cpu_pages *pcp;
+
+ pcp = per_cpu_ptr(zone->per_cpu_pageset, cpu);
+ if (!pcp->count)
+ continue;
+
+ if (!pcp->expire)
+ continue;
+
+ if (zone_to_nid(zone) == cpu_to_node(cpu))
+ continue;
+
+ return true;
+ }
+#endif
+
+ return false;
+}
+
static void refresh_vm_stats(struct work_struct *work)
{
refresh_cpu_vm_stats(true);
@@ -1860,8 +1885,12 @@ int vmstat_refresh(struct ctl_table *tab
void *buffer, size_t *lenp, loff_t *ppos)
{
long val;
- int err;
- int i;
+ int i, cpu;
+ struct work_struct __percpu *works;
+
+ works = alloc_percpu(struct work_struct);
+ if (!works)
+ return -ENOMEM;
/*
* The regular update, every sysctl_stat_interval, may come later
@@ -1875,9 +1904,19 @@ int vmstat_refresh(struct ctl_table *tab
* transiently negative values, report an error here if any of
* the stats is negative, so we know to go looking for imbalance.
*/
- err = schedule_on_each_cpu(refresh_vm_stats);
- if (err)
- return err;
+ get_online_cpus();
+ for_each_online_cpu(cpu) {
+ struct work_struct *work = per_cpu_ptr(works, cpu);
+
+ INIT_WORK(work, refresh_vm_stats);
+ if (need_update(cpu) || need_drain_remote_zones(cpu))
+ schedule_work_on(cpu, work);
+ }
+ for_each_online_cpu(cpu)
+ flush_work(per_cpu_ptr(works, cpu));
+ put_online_cpus();
+ free_percpu(works);
+
for (i = 0; i < NR_VM_ZONE_STAT_ITEMS; i++) {
/*
* Skip checking stats known to go negative occasionally.
On Tue, 24 Aug 2021, Marcelo Tosatti wrote:
> It is not necessary to queue work item to run refresh_vm_stats
> on a remote CPU if that CPU has no dirty stats and no per-CPU
> allocations for remote nodes.
The issue in the past was whether the effort to check is adding overhead
that is comparable to run refresh_vm_stats. YMMV.
> This fixes sosreport hang (which uses vmstat_refresh) with
> spinning SCHED_FIFO process.
Ughhh.. SCHED_FIFO is evil....
> #ifdef CONFIG_PROC_FS
> +static bool need_drain_remote_zones(int cpu)
Well this is not related to vm stats but per cpu pages of the page
allocator. Maybe call this need_drain_remote_pcp or something?
> @@ -1860,8 +1885,12 @@ int vmstat_refresh(struct ctl_table *tab
> void *buffer, size_t *lenp, loff_t *ppos)
> {
> long val;
> - int err;
> - int i;
> + int i, cpu;
> + struct work_struct __percpu *works;
> +
> + works = alloc_percpu(struct work_struct);
Do malloc instead? Using the percpu allocator frequently in a function to
allocator temporary variables can cause needless fragmentation there. The
percpu allocator does not have the frag management features of the slab
allocators.
On Wed, Sep 01, 2021 at 02:32:04PM -0300, Marcelo Tosatti wrote:
> On Wed, Sep 01, 2021 at 09:05:55AM -0400, Nitesh Lal wrote:
> > Hi Marcelo,
> >
> > On Tue, Aug 24, 2021 at 11:42 AM Marcelo Tosatti <[email protected]> wrote:
> > >
> > > It is not necessary to queue work item to run refresh_vm_stats
> > > on a remote CPU if that CPU has no dirty stats and no per-CPU
> > > allocations for remote nodes.
> > >
> > > This fixes sosreport hang (which uses vmstat_refresh) with
> > > spinning SCHED_FIFO process.
> > >
> >
> > I was still able to reproduce the sosreport hang with this patchset and I
> > am wondering if that is because right now we do vmstat_sync and then cancel
> > any pending jobs on a CPU in the context of one task.
>
> Hi Nitesh,
>
> Did you use chisol (with proper flags) and the modified oslat?
>
> Tested with "echo 1 > /proc/sys/vmstat_refresh" and it was successful
> (no hangs).
>
> > However, while this task is running another process can come in and can
> > dirty the stats resulting in vmstat job getting placed on CPUs running
> > SCHED_FIFO tasks.
> > Am I missing something?
> > What we can probably do is to communicate that a CPU is running on task
> > isolation mode to any other process that is trying to run and schedule
> > jobs there.
>
> No, that can happen. Can use sched notifiers to handle this problem.
> Good point.
Well, actually: the goal is an isolated CPU.
So task A (latency sensitive, in task isolated mode, polling) executing on
an isolated CPU will require something like stalld to execute this other
task that dirties the vmstats.
Therefore stalld can also sched-in/sched-out vmstat_worker, with similar
impact to latency.
Hi Marcelo,
On Tue, Aug 24, 2021 at 11:42 AM Marcelo Tosatti <[email protected]> wrote:
>
> It is not necessary to queue work item to run refresh_vm_stats
> on a remote CPU if that CPU has no dirty stats and no per-CPU
> allocations for remote nodes.
>
> This fixes sosreport hang (which uses vmstat_refresh) with
> spinning SCHED_FIFO process.
>
I was still able to reproduce the sosreport hang with this patchset and I
am wondering if that is because right now we do vmstat_sync and then cancel
any pending jobs on a CPU in the context of one task.
However, while this task is running another process can come in and can
dirty the stats resulting in vmstat job getting placed on CPUs running
SCHED_FIFO tasks.
Am I missing something?
What we can probably do is to communicate that a CPU is running on task
isolation mode to any other process that is trying to run and schedule
jobs there.
--
Thanks
Nitesh
On Wed, Sep 01, 2021 at 09:05:55AM -0400, Nitesh Lal wrote:
> Hi Marcelo,
>
> On Tue, Aug 24, 2021 at 11:42 AM Marcelo Tosatti <[email protected]> wrote:
> >
> > It is not necessary to queue work item to run refresh_vm_stats
> > on a remote CPU if that CPU has no dirty stats and no per-CPU
> > allocations for remote nodes.
> >
> > This fixes sosreport hang (which uses vmstat_refresh) with
> > spinning SCHED_FIFO process.
> >
>
> I was still able to reproduce the sosreport hang with this patchset and I
> am wondering if that is because right now we do vmstat_sync and then cancel
> any pending jobs on a CPU in the context of one task.
Hi Nitesh,
Did you use chisol (with proper flags) and the modified oslat?
Tested with "echo 1 > /proc/sys/vmstat_refresh" and it was successful
(no hangs).
> However, while this task is running another process can come in and can
> dirty the stats resulting in vmstat job getting placed on CPUs running
> SCHED_FIFO tasks.
> Am I missing something?
> What we can probably do is to communicate that a CPU is running on task
> isolation mode to any other process that is trying to run and schedule
> jobs there.
No, that can happen. Can use sched notifiers to handle this problem.
Good point.
On Wed, Sep 1, 2021 at 2:34 PM Marcelo Tosatti <[email protected]> wrote:
>
> On Wed, Sep 01, 2021 at 02:32:04PM -0300, Marcelo Tosatti wrote:
> > On Wed, Sep 01, 2021 at 09:05:55AM -0400, Nitesh Lal wrote:
> > > Hi Marcelo,
> > >
> > > On Tue, Aug 24, 2021 at 11:42 AM Marcelo Tosatti <[email protected]> wrote:
> > > >
> > > > It is not necessary to queue work item to run refresh_vm_stats
> > > > on a remote CPU if that CPU has no dirty stats and no per-CPU
> > > > allocations for remote nodes.
> > > >
> > > > This fixes sosreport hang (which uses vmstat_refresh) with
> > > > spinning SCHED_FIFO process.
> > > >
> > >
> > > I was still able to reproduce the sosreport hang with this patchset and I
> > > am wondering if that is because right now we do vmstat_sync and then cancel
> > > any pending jobs on a CPU in the context of one task.
> >
> > Hi Nitesh,
> >
> > Did you use chisol (with proper flags) and the modified oslat?
> >
Yes, I used your patches.
This is the command that I used:
chisol -q vmstat_sync -I conf ./oslat -f 1 -c 5,6,7,8,9,10,11,12,13,14,15 -D 15m
> > Tested with "echo 1 > /proc/sys/vmstat_refresh" and it was successful
> > (no hangs).
I see, I tried with "sos report --batch", which should have a similar effect.
--
Thanks
Nitesh