Re: [PATCH v5 00/12] fold per-CPU vmstats remotely

Marcelo Tosatti
2023-03-14T19:02:29Z

On Tue, Mar 14, 2023 at 03:31:21PM +0100, Michal Hocko wrote:
> On Tue 14-03-23 09:59:37, Marcelo Tosatti wrote:
> > On Tue, Mar 14, 2023 at 01:25:53PM +0100, Michal Hocko wrote:
> > > On Mon 13-03-23 13:25:07, Marcelo Tosatti wrote:
> > > > This patch series addresses the following two problems:
> > > >
> > > > 1. A customer provided some evidence which indicates that
> > > > the idle tick was stopped; albeit, CPU-specific vmstat
> > > > counters still remained populated.
> > > >
> > > > Thus one can only assume quiet_vmstat() was not
> > > > invoked on return to the idle loop. If I understand
> > > > correctly, I suspect this divergence might erroneously
> > > > prevent a reclaim attempt by kswapd. If the number of
> > > > zone specific free pages are below their per-cpu drift
> > > > value then zone_page_state_snapshot() is used to
> > > > compute a more accurate view of the aforementioned
> > > > statistic. Thus any task blocked on the NUMA node
> > > > specific pfmemalloc_wait queue will be unable to make
> > > > significant progress via direct reclaim unless it is
> > > > killed after being woken up by kswapd
> > > > (see throttle_direct_reclaim())
> > >
> > > I have hard time to follow the actual problem described above. Are you
> > > suggesting that a lack of pcp vmstat counters update has led to
> > > reclaim issues? What is the said "evidence"? Could you share more of the
> > > story please?
> >
> >
> > - The process was trapped in throttle_direct_reclaim().
> > The function wait_event_killable() was called to wait condition
> > allow_direct_reclaim(pgdat) for current node to be true.
> > The allow_direct_reclaim(pgdat) examined the number of free pages
> > on the node by zone_page_state() which just returns value in
> > zone->vm_stat[NR_FREE_PAGES].
> >
> > - On node #1, zone->vm_stat[NR_FREE_PAGES] was 0.
> > However, the freelist on this node was not empty.
> >
> > - This inconsistent of vmstat value was caused by percpu vmstat on
> > nohz_full cpus. Every increment/decrement of vmstat is performed
> > on percpu vmstat counter at first, then pooled diffs are cumulated
> > to the zone's vmstat counter in timely manner. However, on nohz_full
> > cpus (in case of this customer's system, 48 of 52 cpus) these pooled
> > diffs were not cumulated once the cpu had no event on it so that
> > the cpu started sleeping infinitely.
> > I checked percpu vmstat and found there were total 69 counts not
> > cumulated to the zone's vmstat counter yet.
> >
> > - In this situation, kswapd did not help the trapped process.
> > In pgdat_balanced(), zone_wakermark_ok_safe() examined the number
> > of free pages on the node by zone_page_state_snapshot() which
> > checks pending counts on percpu vmstat.
> > Therefore kswapd could know there were 69 free pages correctly.
> > Since zone->_watermark = {8, 20, 32}, kswapd did not work because
> > 69 was greater than 32 as high watermark.
>
> If the imprecision of allow_direct_reclaim is the underlying problem why
> haven't you used zone_page_state_snapshot instead?

It might have dealt with problem #1 for this particular case. However,
looking at the callers of zone_page_state:

5 2227 mm/compaction.c <<compaction_suitable>>
zone_page_state(zone, NR_FREE_PAGES));
6 124 mm/highmem.c <<__nr_free_highpages>>
pages += zone_page_state(zone, NR_FREE_PAGES);
7 283 mm/page-writeback.c <<node_dirtyable_memory>>
nr_pages += zone_page_state(zone, NR_FREE_PAGES);
8 318 mm/page-writeback.c <<highmem_dirtyable_memory>>
nr_pages = zone_page_state(z, NR_FREE_PAGES);
9 321 mm/page-writeback.c <<highmem_dirtyable_memory>>
nr_pages += zone_page_state(z, NR_ZONE_INACTIVE_FILE);
10 322 mm/page-writeback.c <<highmem_dirtyable_memory>>
nr_pages += zone_page_state(z, NR_ZONE_ACTIVE_FILE);
11 3091 mm/page_alloc.c <<__rmqueue>>
zone_page_state(zone, NR_FREE_CMA_PAGES) >
12 3092 mm/page_alloc.c <<__rmqueue>>
zone_page_state(zone, NR_FREE_PAGES) / 2) {

The suggested patchset fixes the problem of where due to nohz_full,
the delayed timer for vmstat_work can be armed but not executed (which means
the per-cpu counters can be out of sync for as long as the cpu is in
idle while in nohz_full mode).

You probably do not want to convert all callers of zone_page_state
into zone_page_state_snapshot (as a justification for the proposed
patchset).

> Anyway, this is kind of information that is really helpful to have in
> the patch description.

Agree: resending a new version with updated commit.

> [...]
> > > > 2. With a SCHED_FIFO task that busy loops on a given CPU,
> > > > and kworker for that CPU at SCHED_OTHER priority,
> > > > queuing work to sync per-vmstats will either cause that
> > > > work to never execute, or stalld (i.e. stall daemon)
> > > > boosts kworker priority which causes a latency
> > > > violation
> > >
> > > Why is that a problem? Out-of-sync stats shouldn't cause major problems.
> > > Or can they?
> >
> > Consider SCHED_FIFO task that is polling the network queue (say
> > testpmd).
> >
> > do {
> > if (net_registers->state & DATA_AVAILABLE) {
> > process_data)();
> > }
> > } while (!stopped);
> >
> > Since this task runs at SCHED_FIFO priority, kworker won't
> > be scheduled to run (therefore per-CPU vmstats won't be
> > flushed to global vmstats).
>
> Yes, that is certainly possible. But my main point is that vmstat
> imprecision shouldn't cause functional problems. That is why we have
> _snapshot readers to get an exact value where it matters for
> consistency.

Understood. Perhaps allow_direct_reclaim should use
zone_page_state_snapshot, as otherwise it is only precise
at sysctl_stat_interval intervals?

>
> > Or, if testpmd runs at SCHED_OTHER, then the work item to
> > flush per-CPU vmstats causes
> >
> > testpmd -> kworker
> > kworker: flush per-CPU vmstats
> > kworker -> testpmd
>
> And this might cause undesired latencies to the packets being
> processed by the testpmd task.

> Right but can you have any latencies expectation in a situation like
> that?

Not sure i understand what you mean. Example:

https://www.gabrieleara.it/assets/documents/papers/conferences/2021-ieee-nfv-sdn.pdf

In general, UDPDK exhibits a much lower
latency than the in-kernel UDP stack used through the POSIX
API (e.g., a 69 % reduction from 95 µs down to 29 µs), thanks
to its ability to bypass the kernel entirely, which in turn
outperforms the in-kernel TCP stack as available through the
POSIX API, as expected.
...
Alternatively, application processes can use UDPDK
with the non-blocking API calls (using the O_NONBLOCK flag)
and perform some other action while waiting for packets to
be ready to be sent/received to/from the UDPDK Process,
instead of performing continuous busy-loops on packet queues.
However, in this case the cost of a single CPU fully busy due
to the UDPDK Process itself is anyway unavoidab