Re: [patch 0/2] mm: too_many_isolated can stall due to out of sync VM counters

Marcelo Tosatti
2023-11-22T11:24:25Z

On Tue, Nov 14, 2023 at 01:46:41PM +0100, Michal Hocko wrote:
> On Tue 14-11-23 09:26:53, Marcelo Tosatti wrote:
> > Hi Michal,
> >
> > On Tue, Nov 14, 2023 at 09:20:09AM +0100, Michal Hocko wrote:
> > > On Mon 13-11-23 20:34:20, Marcelo Tosatti wrote:
> > > > A customer reported seeing processes hung at too_many_isolated,
> > > > while analysis indicated that the problem occurred due to out
> > > > of sync per-CPU stats (see below).
> > > >
> > > > Fix is to use node_page_state_snapshot to avoid the out of stale values.
> > > >
> > > > 2136 static unsigned long
> > > > 2137 shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec,
> > > > 2138 struct scan_control *sc, enum lru_list lru)
> > > > 2139 {
> > > > :
> > > > 2145 bool file = is_file_lru(lru);
> > > > :
> > > > 2147 struct pglist_data *pgdat = lruvec_pgdat(lruvec);
> > > > :
> > > > 2150 while (unlikely(too_many_isolated(pgdat, file, sc))) {
> > > > 2151 if (stalled)
> > > > 2152 return 0;
> > > > 2153
> > > > 2154 /* wait a bit for the reclaimer. */
> > > > 2155 msleep(100); <--- some processes were sleeping here, with pending SIGKILL.
> > > > 2156 stalled = true;
> > > > 2157
> > > > 2158 /* We are about to die and free our memory. Return now. */
> > > > 2159 if (fatal_signal_pending(current))
> > > > 2160 return SWAP_CLUSTER_MAX;
> > > > 2161 }
> > > >
> > > > msleep() must be called only when there are too many isolated pages:
> > >
> > > What do you mean here?
> >
> > That msleep() must not be called when
> >
> > isolated > inactive
> >
> > is false.
>
> Well, but the code is structured in a way that this is simply true.
> too_many_isolated might be false positive because it is a very loose
> interface and the number of isolated pages can fluctuate depending on
> the number of direct reclaimers.
>
> > > > 2019 static int too_many_isolated(struct pglist_data *pgdat, int file,
> > > > 2020 struct scan_control *sc)
> > > > 2021 {
> > > > :
> > > > 2030 if (file) {
> > > > 2031 inactive = node_page_state(pgdat, NR_INACTIVE_FILE);
> > > > 2032 isolated = node_page_state(pgdat, NR_ISOLATED_FILE);
> > > > 2033 } else {
> > > > :
> > > > 2046 return isolated > inactive;
> > > >
> > > > The return value was true since:
> > > >
> > > > crash> p ((struct pglist_data *) 0xffff00817fffe580)->vm_stat[NR_INACTIVE_FILE]
> > > > $8 = {
> > > > counter = 1
> > > > }
> > > > crash> p ((struct pglist_data *) 0xffff00817fffe580)->vm_stat[NR_ISOLATED_FILE]
> > > > $9 = {
> > > > counter = 2
> > > >
> > > > while per_cpu stats had:
> > > >
> > > > crash> p ((struct pglist_data *) 0xffff00817fffe580)->per_cpu_nodestats
> > > > $85 = (struct per_cpu_nodestat *) 0xffff8000118832e0
> > > > crash> p/x 0xffff8000118832e0 + __per_cpu_offset[42]
> > > > $86 = 0xffff00917fcc32e0
> > > > crash> p ((struct per_cpu_nodestat *) 0xffff00917fcc32e0)->vm_node_stat_diff[NR_ISOLATED_FILE]
> > > > $87 = -1 '\377'
> > > >
> > > > crash> p/x 0xffff8000118832e0 + __per_cpu_offset[44]
> > > > $89 = 0xffff00917fe032e0
> > > > crash> p ((struct per_cpu_nodestat *) 0xffff00917fe032e0)->vm_node_stat_diff[NR_ISOLATED_FILE]
> > > > $91 = -1 '\377'
> > >
> > > This doesn't really tell much. How much out of sync they really are
> > > cumulatively over all cpus?
> >
> > This is the cumulative value over all CPUs (offsets for other CPUs
> > have been omitted since they are zero).
>
> OK, so that means the NR_ISOLATED_FILE is 0 while NR_INACTIVE_FILE is 1,
> correct? If that is the case then the value is indeed outdated but it
> also means that the NR_INACTIVE_FILE is so small that all but 1 (resp. 2
> as kswapd is never throttled) reclaimers will be stalled anyway. So does
> the exact snapshot really help? Do you have any means to reproduce this
> behavior and see that the patch actually changed the behavior?
>
> [...]
>
> > > With a very low NR_FREE_PAGES and many contending allocation the system
> > > could be easily stuck in reclaim. What are other reclaim
> > > characteristics?
> >
> > I can ask. What information in particular do you want to know?
>
> When I am dealing with issues like this I heavily rely on /proc/vmstat
> counters and pgscan, pgsteal counters to see whether there is any
> progress over time.
>
> > > Is the direct reclaim successful?
> >
> > Processes are stuck in too_many_isolated (unnecessarily). What do you mean when you ask
> > "Is the direct reclaim successful", precisely?
>
> With such a small LRU list it is quite likely that many processes will
> be competing over last pages on the list while rest will be throttled
> because there is nothing to reclaim. It is quite possible that all
> reclaimers will be waiting for a single reclaimer (either kswapd or
> other direct reclaimer). I would like to understand whether the system
> is stuck in unproductive state where everybody just waits until the
> counter is synced or everything just progress very slowly because of the
> small LRU.
> --
> Michal Hocko
> SUSE Labs

Michal,

I think this provides the data you are looking for:

It seems that the situation was invoking memory-consuming user program
in pallarel expecting that the system will kick oom-killer at the end.

The node 0-3 are small containing system data and almost all files.
The node 4-7 are large prepared to contain user data only.
The issue described in above was observed on node 4-7, where
had very few memory for files.

The node 4-7 has more cpu than node 0-3.
Only cpus on node 4-7 are configuerd to be nohz_full.
So we often found unflushed percpu vmstat on cpus of node 4-7.