Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1405135 > unrolled thread

Re: zone_reclaimable() leads to livelock in __alloc_pages_slowpath()

Started byMichal Hocko <mhocko@kernel.org>
First post2016-05-23 09:30 +0200
Last post2016-05-29 23:30 +0200
Articles 5 — 2 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.

Contents

  Re: zone_reclaimable() leads to livelock in __alloc_pages_slowpath() Michal Hocko <mhocko@kernel.org> - 2016-05-23 09:30 +0200
    Re: zone_reclaimable() leads to livelock in __alloc_pages_slowpath() Oleg Nesterov <oleg@redhat.com> - 2016-05-23 17:20 +0200
      Re: zone_reclaimable() leads to livelock in __alloc_pages_slowpath() Michal Hocko <mhocko@kernel.org> - 2016-05-24 09:20 +0200
        Re: zone_reclaimable() leads to livelock in __alloc_pages_slowpath() Oleg Nesterov <oleg@redhat.com> - 2016-05-25 00:50 +0200
          Re: zone_reclaimable() leads to livelock in __alloc_pages_slowpath() Oleg Nesterov <oleg@redhat.com> - 2016-05-29 23:30 +0200

#1405135 — Re: zone_reclaimable() leads to livelock in __alloc_pages_slowpath()

FromMichal Hocko <mhocko@kernel.org>
Date2016-05-23 09:30 +0200
SubjectRe: zone_reclaimable() leads to livelock in __alloc_pages_slowpath()
Message-ID<rBStj-2UV-15@gated-at.bofh.it>
Hi,
Tetsuo has already pointed you at my oom detection rework which removes
the zone_reclaimable ugliness (btw. one of the top reasons to rework
this area) and it is likely to fix your problem. I would still like to
understand what happens with your test case because we might want to
prepare a stable patch for older kernels.

On Fri 20-05-16 22:28:17, Oleg Nesterov wrote:
> I don't understand vmscan.c, and in fact I don't even understand NR_PAGES_SCANNED
[...]
> counter... why it has to be atomic/per-cpu? It is always updated under ->lru_lock
> except free_pcppages_bulk/free_one_page try to reset this counter. But note that
> they both do

It doesn't really have to be atomic/per-cpu because it is really updated
under the lock. It just uses the generic vmstat infrastructure...

> 	nr_scanned = zone_page_state(zone, NR_PAGES_SCANNED);
> 	if (nr_scanned)
> 		__mod_zone_page_state(zone, NR_PAGES_SCANNED, -nr_scanned);
> 
> and this doesn't look exactly right: zone_page_state() ignores the per-cpu
> ->vm_stat_diff[] counters (and we probably do not want for_each_online_cpu()
> loop here). And I do not know if this is really bad or not, but note that if
> I change calculate_normal_threshold() to return 0, the problem goes away too.

You are absolutely right that this is racy. In the worst case we would
end up missing nr_cpus*threshold scanned pages which would stay behind.
But

bool zone_reclaimable(struct zone *zone)
{
	return zone_page_state_snapshot(zone, NR_PAGES_SCANNED) <
		zone_reclaimable_pages(zone) * 6;
}

So the left over shouldn't cause it to return true all the time. In
fact it could prematurely say false, right? (note that _snapshot variant
considers per-cpu diffs [1]).

That being said I am not really sure why would the 0 threshold help for
your test case. Could you add some tracing and see what are the numbers
above? Is it possible that zone_reclaimable_pages is some small number
which actuall prevents us to scan anything? Aka a bug is get_scan_count
or somewhere else?

[1] I am not really sure which kernel version have you tested - your
config says 4.6.0-rc7 but this is true since 0db2cb8da89d ("mm, vmscan:
make zone_reclaimable_pages more precise") which is 4.6-rc1.
-- 
Michal Hocko
SUSE Labs

[toc] | [next] | [standalone]

#1405438

FromOleg Nesterov <oleg@redhat.com>
Date2016-05-23 17:20 +0200
Message-ID<rBZOa-7tz-25@gated-at.bofh.it>
In reply to#1405135
On 05/23, Michal Hocko wrote:
>
> > 	nr_scanned = zone_page_state(zone, NR_PAGES_SCANNED);
> > 	if (nr_scanned)
> > 		__mod_zone_page_state(zone, NR_PAGES_SCANNED, -nr_scanned);
> >
> > and this doesn't look exactly right: zone_page_state() ignores the per-cpu
> > ->vm_stat_diff[] counters (and we probably do not want for_each_online_cpu()
> > loop here). And I do not know if this is really bad or not, but note that if
> > I change calculate_normal_threshold() to return 0, the problem goes away too.
>
> You are absolutely right that this is racy. In the worst case we would
> end up missing nr_cpus*threshold scanned pages which would stay behind.

and the sum of ->vm_diff[] can be negative, so...

> But
>
> bool zone_reclaimable(struct zone *zone)
> {
> 	return zone_page_state_snapshot(zone, NR_PAGES_SCANNED) <
> 		zone_reclaimable_pages(zone) * 6;
> }
>
> So the left over shouldn't cause it to return true all the time.

well if NR_PAGES_SCANNED doesn't grow enough it can even stay negative,
but zone_page_state_snapshot() returns zero in this case. In any case
we can underestimate zone_page_state_snapshot(NR_PAGES_SCANNED).

> In
> fact it could prematurely say false, right? (note that _snapshot variant
> considers per-cpu diffs [1]).

exactly because _snapshot() doesn't ignore the per-cpu counters.

> That being said I am not really sure why would the 0 threshold help for
> your test case.

Neither me. Except, of course, threshold==0 means the the code above will
work correctly. But I do not think this was the root of the problem.

> Could you add some tracing and see what are the numbers
> above?

with the patch below I can press Ctrl-C when it hangs, this breaks the
endless loop and the output looks like

	vmscan: ZONE=ffffffff8189f180 0 scanned=0 pages=6
	vmscan: ZONE=ffffffff8189eb00 0 scanned=1 pages=0
	...
	vmscan: ZONE=ffffffff8189eb00 0 scanned=2 pages=1
	vmscan: ZONE=ffffffff8189f180 0 scanned=4 pages=6
	...
	vmscan: ZONE=ffffffff8189f180 0 scanned=4 pages=6
	vmscan: ZONE=ffffffff8189f180 0 scanned=4 pages=6

the numbers are always small.

> [1] I am not really sure which kernel version have you tested - your
> config says 4.6.0-rc7 but this is true since 0db2cb8da89d ("mm, vmscan:
> make zone_reclaimable_pages more precise") which is 4.6-rc1.

Yes, I am on c5114626f33b62fa7595e57d87f33d9d1f8298a2, it has this change.

Oleg.

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 142cb61..6d221f9 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2614,6 +2614,12 @@ static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
 		if (shrink_zone(zone, sc, zone_idx(zone) == classzone_idx))
 			reclaimable = true;
 
+if (fatal_signal_pending(current))
+	pr_crit("ZONE=%p %d scanned=%ld pages=%ld\n",
+		zone, reclaimable,
+		zone_page_state_snapshot(zone, NR_PAGES_SCANNED),
+		zone_reclaimable_pages(zone));
+else
 		if (global_reclaim(sc) &&
 		    !reclaimable && zone_reclaimable(zone))
 			reclaimable = true;

[toc] | [prev] | [next] | [standalone]

#1405904

FromMichal Hocko <mhocko@kernel.org>
Date2016-05-24 09:20 +0200
Message-ID<rCeNc-dQ-25@gated-at.bofh.it>
In reply to#1405438
On Mon 23-05-16 17:14:19, Oleg Nesterov wrote:
> On 05/23, Michal Hocko wrote:
[...]
> > Could you add some tracing and see what are the numbers
> > above?
> 
> with the patch below I can press Ctrl-C when it hangs, this breaks the
> endless loop and the output looks like
> 
> 	vmscan: ZONE=ffffffff8189f180 0 scanned=0 pages=6
> 	vmscan: ZONE=ffffffff8189eb00 0 scanned=1 pages=0
> 	...
> 	vmscan: ZONE=ffffffff8189eb00 0 scanned=2 pages=1
> 	vmscan: ZONE=ffffffff8189f180 0 scanned=4 pages=6
> 	...
> 	vmscan: ZONE=ffffffff8189f180 0 scanned=4 pages=6
> 	vmscan: ZONE=ffffffff8189f180 0 scanned=4 pages=6
> 
> the numbers are always small.

Small but scanned is not 0 and constant which means it either gets reset
repeatedly (something gets freed) or we have stopped scanning. Which
pattern can you see? I assume that the swap space is full at the time
(could you add get_nr_swap_pages() to the output). Also zone->name would
be better than the pointer.

I am trying to reproduce but your test case always hits the oom killer:

This is in a qemu x86_64 virtual machine:
# free
             total       used       free     shared    buffers     cached
Mem:        490212      96788     393424          0       3196       9976
-/+ buffers/cache:      83616     406596
Swap:       138236      57740      80496

I have tried with much larger swap space but no change except for the
run time of the test which is expected.

# grep "^processor" /proc/cpuinfo | wc -l
1

[... Skipped several previous attempts ...]
[  695.215235] vmscan: XXX: zone:DMA32 nr_pages_scanned:0 reclaimable:20
[  695.215245] vmscan: XXX: zone:DMA32 nr_pages_scanned:0 reclaimable:20
[  695.215255] vmscan: XXX: zone:DMA32 nr_pages_scanned:0 reclaimable:20
[  695.215282] vmscan: XXX: zone:DMA32 nr_pages_scanned:1 reclaimable:27
[  695.215303] vmscan: XXX: zone:DMA32 nr_pages_scanned:5 reclaimable:27
[  695.215327] vmscan: XXX: zone:DMA32 nr_pages_scanned:18 reclaimable:27
[  695.215351] vmscan: XXX: zone:DMA32 nr_pages_scanned:45 reclaimable:27
[  695.215362] vmscan: XXX: zone:DMA32 nr_pages_scanned:45 reclaimable:27
[  695.215373] vmscan: XXX: zone:DMA32 nr_pages_scanned:45 reclaimable:27
[  695.215382] vmscan: XXX: zone:DMA32 nr_pages_scanned:45 reclaimable:27
[  695.215392] vmscan: XXX: zone:DMA32 nr_pages_scanned:45 reclaimable:27
[  695.215402] vmscan: XXX: zone:DMA32 nr_pages_scanned:45 reclaimable:27
[  695.215412] vmscan: XXX: zone:DMA32 nr_pages_scanned:45 reclaimable:27
[  695.215422] vmscan: XXX: zone:DMA32 nr_pages_scanned:45 reclaimable:27
[  695.215431] vmscan: XXX: zone:DMA32 nr_pages_scanned:45 reclaimable:27
[  695.215442] vmscan: XXX: zone:DMA32 nr_pages_scanned:46 reclaimable:27
[  695.215462] vmscan: XXX: zone:DMA32 nr_pages_scanned:48 reclaimable:27
[  695.215482] vmscan: XXX: zone:DMA32 nr_pages_scanned:53 reclaimable:27
[  695.215504] vmscan: XXX: zone:DMA32 nr_pages_scanned:63 reclaimable:27
[  695.215528] vmscan: XXX: zone:DMA32 nr_pages_scanned:90 reclaimable:27
[...]
[  695.215620] vmscan: XXX: zone:DMA32 nr_pages_scanned:91 reclaimable:27
[  695.215640] vmscan: XXX: zone:DMA32 nr_pages_scanned:94 reclaimable:27
[  695.215659] vmscan: XXX: zone:DMA32 nr_pages_scanned:100 reclaimable:27
[  695.215683] vmscan: XXX: zone:DMA32 nr_pages_scanned:113 reclaimable:27
[...]
[  695.215786] vmscan: XXX: zone:DMA32 nr_pages_scanned:140 reclaimable:27
[  695.215797] vmscan: XXX: zone:DMA32 nr_pages_scanned:141 reclaimable:27
[  695.215816] vmscan: XXX: zone:DMA32 nr_pages_scanned:144 reclaimable:27
[  695.215836] vmscan: XXX: zone:DMA32 nr_pages_scanned:150 reclaimable:27
[  695.215906] test-oleg invoked oom-killer: gfp_mask=0x24201ca(GFP_HIGHUSER_MOVABLE|__GFP_COLD), order=0, oom_score_adj=0
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1406501

FromOleg Nesterov <oleg@redhat.com>
Date2016-05-25 00:50 +0200
Message-ID<rCtjc-Ur-19@gated-at.bofh.it>
In reply to#1405904
On 05/24, Michal Hocko wrote:
>
> On Mon 23-05-16 17:14:19, Oleg Nesterov wrote:
> > On 05/23, Michal Hocko wrote:
> [...]
> > > Could you add some tracing and see what are the numbers
> > > above?
> >
> > with the patch below I can press Ctrl-C when it hangs, this breaks the
> > endless loop and the output looks like
> >
> > 	vmscan: ZONE=ffffffff8189f180 0 scanned=0 pages=6
> > 	vmscan: ZONE=ffffffff8189eb00 0 scanned=1 pages=0
> > 	...
> > 	vmscan: ZONE=ffffffff8189eb00 0 scanned=2 pages=1
> > 	vmscan: ZONE=ffffffff8189f180 0 scanned=4 pages=6
> > 	...
> > 	vmscan: ZONE=ffffffff8189f180 0 scanned=4 pages=6
> > 	vmscan: ZONE=ffffffff8189f180 0 scanned=4 pages=6
> >
> > the numbers are always small.
>
> Small but scanned is not 0 and constant which means it either gets reset
> repeatedly (something gets freed) or we have stopped scanning. Which
> pattern can you see? I assume that the swap space is full at the time
> (could you add get_nr_swap_pages() to the output).

no, I tested this without SWAP,

> Also zone->name would
> be better than the pointer.

Yes, forgot to mention, this is DMA32. To remind, only 512m of RAM so
this is natural.

> I am trying to reproduce but your test case always hits the oom killer:

Did you try to run it in a loop? Usually it takes a while before the system
hangs.

> Swap:       138236      57740      80496

perhaps this makes a difference? See above, I have no SWAP.


So. I spent almost the whole day trying to understand whats going on, and
of course I failed.

But. It _seems to me_ that the kernel "leaks" some pages in LRU_INACTIVE_FILE
list because inactive_file_is_low() returns the wrong value. And do not even
ask me why I think so, unlikely I will be able to explain ;) to remind, I never
tried to read vmscan.c before.

But. if I change lruvec_lru_size()

	-       return zone_page_state(lruvec_zone(lruvec), NR_LRU_BASE + lru);
	+       return zone_page_state_snapshot(lruvec_zone(lruvec), NR_LRU_BASE + lru);

the problem goes away too.

To remind, it also goes away if I change calculate_normal_threshold() to return
zero, and it was not clear why. Now we can probably conclude that that this is
because the change obviouslt affects lruvec_lru_size().

Oleg.

[toc] | [prev] | [next] | [standalone]

#1408666

FromOleg Nesterov <oleg@redhat.com>
Date2016-05-29 23:30 +0200
Message-ID<rEgrv-39r-1@gated-at.bofh.it>
In reply to#1406501
sorry for delay,

On 05/25, Michal Hocko wrote:
>
> On Wed 25-05-16 00:43:41, Oleg Nesterov wrote:
> >
> > But. It _seems to me_ that the kernel "leaks" some pages in LRU_INACTIVE_FILE
> > list because inactive_file_is_low() returns the wrong value. And do not even
> > ask me why I think so, unlikely I will be able to explain ;) to remind, I never
> > tried to read vmscan.c before.

No, this is not because of inactive_file_is_low(), but

> >
> > But. if I change lruvec_lru_size()
> >
> > 	-       return zone_page_state(lruvec_zone(lruvec), NR_LRU_BASE + lru);
> > 	+       return zone_page_state_snapshot(lruvec_zone(lruvec), NR_LRU_BASE + lru);
> >
> > the problem goes away too.

Yes,

> This is a bit surprising but my testing shows that the result shouldn't
> make much difference. I can see some discrepancies between lru_vec size
> and zone_reclaimable_pages but they are too small to actually matter.

Yes, the difference is small but it does matter.

I do not pretend I understand this all, but finally it seems I understand
whats going on on my system when it hangs. At least, why the change in
lruvec_lru_size() or calculate_normal_threshold() makes a difference.

This single change in get_scan_count() under for_each_evictable_lru() loop

	-	size = lruvec_lru_size(lruvec, lru);
	+	size = zone_page_state_snapshot(lruvec_zone(lruvec), NR_LRU_BASE + lru);

fixes the problem too.

Without this change shrink*() continues to scan the LRU_ACTIVE_FILE list
while it is empty. LRU_INACTIVE_FILE is not empty (just a few pages) but
we do not even try to scan it, lruvec_lru_size() returns zero.

Then later we recheck zone_reclaimable() and it notices the INACTIVE_FILE
counter because it uses the _snapshot variant, this leads to livelock.

I guess this doesn't really matter, but in my particular case these
ACTIVE/INACTIVE counters were screwed by the recent putback_inactive_pages()
logic. The pages we "leak" in INACTIVE list were recently moved from ACTIVE
to INACTIVE list, and this updated only the per-cpu ->vm_stat_diff[] counters,
so the "non snapshot" lruvec_lru_size() in get_scan_count() sees the "old"
numbers.

I even added more printk's, and yes when the system hangs I have something
like, say,

	->vm_stat[ACTIVE] 	 = NR;		// small number
	->vm_stat_diff[ACTIVE]	 = -NR;		// so it is actually zero but
						// get_scan_count() sees NR

	->vm_stat[INACTIVE]	 = 0;		// this is what get_scan_count() sees
	->vm_stat_diff[INACTIVE] = NR;		// and this is what zone_reclaimable()

Oleg.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web