Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1543122 > unrolled thread
| Started by | Nils Holland <nholland@tisys.org> |
|---|---|
| First post | 2016-12-16 00:10 +0100 |
| Last post | 2016-12-17 00:40 +0100 |
| Articles | 16 on this page of 56 — 8 participants |
Back to article view | Back to linux.kernel
OOM: Better, but still there on 4.9 Nils Holland <nholland@tisys.org> - 2016-12-16 00:10 +0100
Re: OOM: Better, but still there on 4.9 Michal Hocko <mhocko@kernel.org> - 2016-12-16 08:50 +0100
Re: OOM: Better, but still there on Michal Hocko <mhocko@kernel.org> - 2016-12-16 17:00 +0100
[PATCH 1/2] mm: consolidate GFP_NOFAIL checks in the allocator slowpath Michal Hocko <mhocko@kernel.org> - 2016-12-16 17:00 +0100
[PATCH 2/2] mm, oom: do not enfore OOM killer for __GFP_NOFAIL automatically Michal Hocko <mhocko@kernel.org> - 2016-12-16 17:00 +0100
Re: [PATCH 2/2] mm, oom: do not enfore OOM killer for __GFP_NOFAIL automatically Johannes Weiner <hannes@cmpxchg.org> - 2016-12-16 18:40 +0100
Re: [PATCH 2/2] mm, oom: do not enfore OOM killer for __GFP_NOFAIL automatically Michal Hocko <mhocko@kernel.org> - 2016-12-16 23:20 +0100
Re: [PATCH 2/2] mm, oom: do not enfore OOM killer for __GFP_NOFAIL automatically Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-12-17 12:20 +0100
Re: [PATCH 2/2] mm, oom: do not enfore OOM killer for __GFP_NOFAIL automatically Michal Hocko <mhocko@kernel.org> - 2016-12-18 17:40 +0100
Re: OOM: Better, but still there on Michal Hocko <mhocko@kernel.org> - 2016-12-17 01:10 +0100
Re: OOM: Better, but still there on Nils Holland <nholland@tisys.org> - 2016-12-17 14:10 +0100
Re: OOM: Better, but still there on Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-12-17 15:50 +0100
Re: OOM: Better, but still there on Nils Holland <nholland@tisys.org> - 2016-12-17 18:20 +0100
Re: OOM: Better, but still there on Nils Holland <nholland@tisys.org> - 2016-12-17 22:10 +0100
Re: OOM: Better, but still there on Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-12-18 06:20 +0100
Re: OOM: Better, but still there on Michal Hocko <mhocko@kernel.org> - 2016-12-19 14:50 +0100
Re: OOM: Better, but still there on Nils Holland <nholland@tisys.org> - 2016-12-20 03:10 +0100
Re: OOM: Better, but still there on Michal Hocko <mhocko@kernel.org> - 2016-12-21 08:40 +0100
Re: OOM: Better, but still there on Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-12-21 12:10 +0100
Re: OOM: Better, but still there on Michal Hocko <mhocko@kernel.org> - 2016-12-21 12:20 +0100
Re: OOM: Better, but still there on Nils Holland <nholland@tisys.org> - 2016-12-22 11:20 +0100
Re: OOM: Better, but still there on Michal Hocko <mhocko@kernel.org> - 2016-12-22 11:30 +0100
Re: OOM: Better, but still there on Nils Holland <nholland@tisys.org> - 2016-12-22 11:40 +0100
Re: OOM: Better, but still there on Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-12-22 11:50 +0100
Re: OOM: Better, but still there on Michal Hocko <mhocko@kernel.org> - 2016-12-22 20:20 +0100
Re: OOM: Better, but still there on Nils Holland <nholland@tisys.org> - 2016-12-22 22:50 +0100
Re: OOM: Better, but still there on Michal Hocko <mhocko@kernel.org> - 2016-12-23 12:00 +0100
Re: OOM: Better, but still there on Nils Holland <nholland@tisys.org> - 2016-12-23 13:20 +0100
Re: OOM: Better, but still there on Michal Hocko <mhocko@kernel.org> - 2016-12-23 14:00 +0100
[RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) Michal Hocko <mhocko@kernel.org> - 2016-12-23 15:50 +0100
Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) Nils Holland <nholland@tisys.org> - 2016-12-23 23:30 +0100
Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) Michal Hocko <mhocko@kernel.org> - 2016-12-26 13:50 +0100
Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) Nils Holland <nholland@tisys.org> - 2016-12-26 20:00 +0100
Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) Michal Hocko <mhocko@kernel.org> - 2016-12-27 09:10 +0100
Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) Michal Hocko <mhocko@kernel.org> - 2016-12-27 12:30 +0100
Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) Nils Holland <nholland@tisys.org> - 2016-12-27 12:30 +0100
Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) Michal Hocko <mhocko@kernel.org> - 2016-12-27 17:00 +0100
Re: [PATCH] mm, vmscan: consider eligible zones in get_scan_count kbuild test robot <lkp@intel.com> - 2016-12-27 17:30 +0100
Re: [PATCH] mm, vmscan: consider eligible zones in get_scan_count Michal Hocko <mhocko@kernel.org> - 2016-12-28 10:00 +0100
Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) Nils Holland <nholland@tisys.org> - 2016-12-27 20:40 +0100
Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) Michal Hocko <mhocko@kernel.org> - 2016-12-28 10:00 +0100
Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) Minchan Kim <minchan@kernel.org> - 2016-12-29 02:30 +0100
Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) Michal Hocko <mhocko@kernel.org> - 2016-12-29 10:10 +0100
Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) Minchan Kim <minchan@kernel.org> - 2016-12-30 03:10 +0100
Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) Michal Hocko <mhocko@kernel.org> - 2016-12-30 11:50 +0100
Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) Minchan Kim <minchan@kernel.org> - 2016-12-29 01:40 +0100
Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) Minchan Kim <minchan@kernel.org> - 2016-12-29 02:10 +0100
Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) Michal Hocko <mhocko@kernel.org> - 2016-12-29 10:00 +0100
Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) Mel Gorman <mgorman@suse.de> - 2016-12-30 11:20 +0100
Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) Michal Hocko <mhocko@kernel.org> - 2016-12-30 12:10 +0100
Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) Mel Gorman <mgorman@suse.de> - 2016-12-30 13:50 +0100
Re: [lkp-developer] [mm, memcg] d18e2b2aca: WARNING:at_mm/memcontrol.c:#mem_cgroup_update_lru_size Michal Hocko <mhocko@kernel.org> - 2016-12-26 13:30 +0100
Re: [lkp-developer] [mm, memcg] d18e2b2aca: WARNING:at_mm/memcontrol.c:#mem_cgroup_update_lru_size Michal Hocko <mhocko@kernel.org> - 2016-12-26 14:00 +0100
Re: OOM: Better, but still there on "Xin Zhou" <xin.zhou@gmx.com> - 2016-12-18 01:40 +0100
Re: OOM: Better, but still there on 4.9 Michal Hocko <mhocko@kernel.org> - 2016-12-16 23:20 +0100
Re: OOM: Better, but still there on 4.9 Michal Hocko <mhocko@kernel.org> - 2016-12-17 00:40 +0100
Page 3 of 3 — ← Prev page 1 2 [3]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-12-28 10:00 +0100 |
| Subject | Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) |
| Message-ID | <sTifv-5On-1@gated-at.bofh.it> |
| In reply to | #1547729 |
On Tue 27-12-16 20:33:09, Nils Holland wrote:
> On Tue, Dec 27, 2016 at 04:55:33PM +0100, Michal Hocko wrote:
> > Hi,
> > could you try to run with the following patch on top of the previous
> > one? I do not think it will make a large change in your workload but
> > I think we need something like that so some testing under which is known
> > to make a high lowmem pressure would be really appreciated. If you have
> > more time to play with it then running with and without the patch with
> > mm_vmscan_direct_reclaim_{start,end} tracepoints enabled could tell us
> > whether it make any difference at all.
>
> Of course, no problem!
>
> First, about the events to trace: mm_vmscan_direct_reclaim_start
> doesn't seem to exist, but mm_vmscan_direct_reclaim_begin does. I'm
> sure that's what you meant and so I took that one instead.
yes, sorry about the confusion
> Then I have to admit in both cases (once without the latest patch,
> once with) very little trace data was actually produced. In the case
> without the patch, the reclaim was started more often and reclaimed a
> smaller number of pages each time, in the case with the patch it was
> invoked less often, and with the last time it was invoked it reclaimed
> a rather big number of pages. I have no clue, however, if that
> happened "by chance" or if it was actually causes by the patch and
> thus an expected change.
yes that seems to be a variation of the workload I would say because if
anything the patch should reduce the number of scanned pages.
> In both cases, my test case was: Reboot, setup logging, do "emerge
> firefox" (which unpacks and builds the firefox sources), then, when
> the emerge had come so far that the unpacking was done and the
> building had started, switch to another console and untar the latest
> kernel, libreoffice and (once more) firefox sources there. After that
> had completed, I aborted the emerge build process and stopped tracing.
>
> Here's the trace data captured without the latest patch applied:
>
> khugepaged-22 [000] .... 566.123383: mm_vmscan_direct_reclaim_begin: order=9 may_writepage=1 gfp_flags=GFP_TRANSHUGE classzone_idx=3
> khugepaged-22 [000] .N.. 566.165520: mm_vmscan_direct_reclaim_end: nr_reclaimed=1100
> khugepaged-22 [001] .... 587.515424: mm_vmscan_direct_reclaim_begin: order=9 may_writepage=1 gfp_flags=GFP_TRANSHUGE classzone_idx=3
> khugepaged-22 [000] .... 587.596035: mm_vmscan_direct_reclaim_end: nr_reclaimed=1029
> khugepaged-22 [001] .... 599.879536: mm_vmscan_direct_reclaim_begin: order=9 may_writepage=1 gfp_flags=GFP_TRANSHUGE classzone_idx=3
> khugepaged-22 [000] .... 601.000812: mm_vmscan_direct_reclaim_end: nr_reclaimed=1100
> khugepaged-22 [001] .... 601.228137: mm_vmscan_direct_reclaim_begin: order=9 may_writepage=1 gfp_flags=GFP_TRANSHUGE classzone_idx=3
> khugepaged-22 [001] .... 601.309952: mm_vmscan_direct_reclaim_end: nr_reclaimed=1081
> khugepaged-22 [001] .... 694.935267: mm_vmscan_direct_reclaim_begin: order=9 may_writepage=1 gfp_flags=GFP_TRANSHUGE classzone_idx=3
> khugepaged-22 [001] .N.. 695.081943: mm_vmscan_direct_reclaim_end: nr_reclaimed=1071
> khugepaged-22 [001] .... 701.370707: mm_vmscan_direct_reclaim_begin: order=9 may_writepage=1 gfp_flags=GFP_TRANSHUGE classzone_idx=3
> khugepaged-22 [001] .... 701.372798: mm_vmscan_direct_reclaim_end: nr_reclaimed=1089
> khugepaged-22 [001] .... 764.752036: mm_vmscan_direct_reclaim_begin: order=9 may_writepage=1 gfp_flags=GFP_TRANSHUGE classzone_idx=3
> khugepaged-22 [000] .... 771.047905: mm_vmscan_direct_reclaim_end: nr_reclaimed=1039
> khugepaged-22 [000] .... 781.760515: mm_vmscan_direct_reclaim_begin: order=9 may_writepage=1 gfp_flags=GFP_TRANSHUGE classzone_idx=3
> khugepaged-22 [001] .... 781.826543: mm_vmscan_direct_reclaim_end: nr_reclaimed=1040
> khugepaged-22 [001] .... 782.595575: mm_vmscan_direct_reclaim_begin: order=9 may_writepage=1 gfp_flags=GFP_TRANSHUGE classzone_idx=3
> khugepaged-22 [000] .... 782.638591: mm_vmscan_direct_reclaim_end: nr_reclaimed=1040
> khugepaged-22 [001] .... 782.930455: mm_vmscan_direct_reclaim_begin: order=9 may_writepage=1 gfp_flags=GFP_TRANSHUGE classzone_idx=3
> khugepaged-22 [001] .... 782.993608: mm_vmscan_direct_reclaim_end: nr_reclaimed=1040
> khugepaged-22 [001] .... 783.330378: mm_vmscan_direct_reclaim_begin: order=9 may_writepage=1 gfp_flags=GFP_TRANSHUGE classzone_idx=3
> khugepaged-22 [001] .... 783.369653: mm_vmscan_direct_reclaim_end: nr_reclaimed=1040
>
> And this is the same with the patch applied:
>
> khugepaged-22 [001] .... 523.599997: mm_vmscan_direct_reclaim_begin: order=9 may_writepage=1 gfp_flags=GFP_TRANSHUGE classzone_idx=3
> khugepaged-22 [001] .... 523.683110: mm_vmscan_direct_reclaim_end: nr_reclaimed=1092
> khugepaged-22 [001] .... 535.345477: mm_vmscan_direct_reclaim_begin: order=9 may_writepage=1 gfp_flags=GFP_TRANSHUGE classzone_idx=3
> khugepaged-22 [001] .... 535.401189: mm_vmscan_direct_reclaim_end: nr_reclaimed=1078
> khugepaged-22 [000] .... 692.876716: mm_vmscan_direct_reclaim_begin: order=9 may_writepage=1 gfp_flags=GFP_TRANSHUGE classzone_idx=3
> khugepaged-22 [001] .... 703.312399: mm_vmscan_direct_reclaim_end: nr_reclaimed=197759
In these cases there is no real difference because this is not the
lowmem pressure because those requests can go to the highmem zone.
> If my test case and thus the results don't sound good, I could of
> course try some other test cases ... like capturing for a longer
> period of time or trying to produce more memory pressure by running
> more processes at the same time, or something like that.
yes, a stronger memory pressure would be needed. I suspect that your
original issues was more about active list aging than a really strong
memory pressure. So it might be possible that your workload will not
notice. If you can collect those two tracepoints over a longer time it
can still tell us something but I do not want you to burn a lot of time
on this. The main issue seems to be fixed and the follow up fix can wait
for a throughout review after both Mel and Johannes are back from
holiday.
> Besides that I can say that the patch hasn't produced any warnings or
> other issues so far, so at first glance, it doesn't seem to hurt
> anything.
Thanks!
--
Michal Hocko
SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Minchan Kim <minchan@kernel.org> |
|---|---|
| Date | 2016-12-29 02:30 +0100 |
| Subject | Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) |
| Message-ID | <sTxHz-8fG-5@gated-at.bofh.it> |
| In reply to | #1547621 |
On Tue, Dec 27, 2016 at 04:55:33PM +0100, Michal Hocko wrote:
> Hi,
> could you try to run with the following patch on top of the previous
> one? I do not think it will make a large change in your workload but
> I think we need something like that so some testing under which is known
> to make a high lowmem pressure would be really appreciated. If you have
> more time to play with it then running with and without the patch with
> mm_vmscan_direct_reclaim_{start,end} tracepoints enabled could tell us
> whether it make any difference at all.
>
> I would also appreciate if Mel and Johannes had a look at it. I am not
> yet sure whether we need the same thing for anon/file balancing in
> get_scan_count. I suspect we need but need to think more about that.
>
> Thanks a lot again!
> ---
> From b51f50340fe9e40b68be198b012f8ab9869c1850 Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@suse.com>
> Date: Tue, 27 Dec 2016 16:28:44 +0100
> Subject: [PATCH] mm, vmscan: consider eligible zones in get_scan_count
>
> get_scan_count considers the whole node LRU size when
> - doing SCAN_FILE due to many page cache inactive pages
> - calculating the number of pages to scan
>
> in both cases this might lead to unexpected behavior especially on 32b
> systems where we can expect lowmem memory pressure very often.
>
> A large highmem zone can easily distort SCAN_FILE heuristic because
> there might be only few file pages from the eligible zones on the node
> lru and we would still enforce file lru scanning which can lead to
> trashing while we could still scan anonymous pages.
Nit:
It doesn't make thrashing because isolate_lru_pages filter out them
but I agree it makes pointless CPU burning to find eligible pages.
>
> The later use of lruvec_lru_size can be problematic as well. Especially
> when there are not many pages from the eligible zones. We would have to
> skip over many pages to find anything to reclaim but shrink_node_memcg
> would only reduce the remaining number to scan by SWAP_CLUSTER_MAX
> at maximum. Therefore we can end up going over a large LRU many times
> without actually having chance to reclaim much if anything at all. The
> closer we are out of memory on lowmem zone the worse the problem will
> be.
>
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
> mm/vmscan.c | 30 ++++++++++++++++++++++++++++--
> 1 file changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index c98b1a585992..785b4d7fb8a0 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -252,6 +252,32 @@ unsigned long lruvec_zone_lru_size(struct lruvec *lruvec, enum lru_list lru, int
> }
>
> /*
> + * Return the number of pages on the given lru which are eligibne for the
eligible
> + * given zone_idx
> + */
> +static unsigned long lruvec_lru_size_zone_idx(struct lruvec *lruvec,
> + enum lru_list lru, int zone_idx)
Nit:
Although there is a comment, function name is rather confusing when I compared
it with lruvec_zone_lru_size.
lruvec_eligible_zones_lru_size is better?
> +{
> + struct pglist_data *pgdat = lruvec_pgdat(lruvec);
> + unsigned long lru_size;
> + int zid;
> +
> + lru_size = lruvec_lru_size(lruvec, lru);
> + for (zid = zone_idx + 1; zid < MAX_NR_ZONES; zid++) {
> + struct zone *zone = &pgdat->node_zones[zid];
> + unsigned long size;
> +
> + if (!managed_zone(zone))
> + continue;
> +
> + size = lruvec_zone_lru_size(lruvec, lru, zid);
> + lru_size -= min(size, lru_size);
> + }
> +
> + return lru_size;
> +}
> +
> +/*
> * Add a shrinker callback to be called from the vm.
> */
> int register_shrinker(struct shrinker *shrinker)
> @@ -2207,7 +2233,7 @@ static void get_scan_count(struct lruvec *lruvec, struct mem_cgroup *memcg,
> * system is under heavy pressure.
> */
> if (!inactive_list_is_low(lruvec, true, sc) &&
> - lruvec_lru_size(lruvec, LRU_INACTIVE_FILE) >> sc->priority) {
> + lruvec_lru_size_zone_idx(lruvec, LRU_INACTIVE_FILE, sc->reclaim_idx) >> sc->priority) {
> scan_balance = SCAN_FILE;
> goto out;
> }
> @@ -2274,7 +2300,7 @@ static void get_scan_count(struct lruvec *lruvec, struct mem_cgroup *memcg,
> unsigned long size;
> unsigned long scan;
>
> - size = lruvec_lru_size(lruvec, lru);
> + size = lruvec_lru_size_zone_idx(lruvec, lru, sc->reclaim_idx);
> scan = size >> sc->priority;
>
> if (!scan && pass && force_scan)
> --
> 2.10.2
Nit:
With this patch, inactive_list_is_low can use lruvec_lru_size_zone_idx rather than
own custom calculation to filter out non-eligible pages.
Anyway, I think this patch does right things so I suppose this.
Acked-by: Minchan Kim <minchan@kernel.org>
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-12-29 10:10 +0100 |
| Subject | Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) |
| Message-ID | <sTESJ-4D2-7@gated-at.bofh.it> |
| In reply to | #1548166 |
On Thu 29-12-16 10:20:26, Minchan Kim wrote:
> On Tue, Dec 27, 2016 at 04:55:33PM +0100, Michal Hocko wrote:
> > Hi,
> > could you try to run with the following patch on top of the previous
> > one? I do not think it will make a large change in your workload but
> > I think we need something like that so some testing under which is known
> > to make a high lowmem pressure would be really appreciated. If you have
> > more time to play with it then running with and without the patch with
> > mm_vmscan_direct_reclaim_{start,end} tracepoints enabled could tell us
> > whether it make any difference at all.
> >
> > I would also appreciate if Mel and Johannes had a look at it. I am not
> > yet sure whether we need the same thing for anon/file balancing in
> > get_scan_count. I suspect we need but need to think more about that.
> >
> > Thanks a lot again!
> > ---
> > From b51f50340fe9e40b68be198b012f8ab9869c1850 Mon Sep 17 00:00:00 2001
> > From: Michal Hocko <mhocko@suse.com>
> > Date: Tue, 27 Dec 2016 16:28:44 +0100
> > Subject: [PATCH] mm, vmscan: consider eligible zones in get_scan_count
> >
> > get_scan_count considers the whole node LRU size when
> > - doing SCAN_FILE due to many page cache inactive pages
> > - calculating the number of pages to scan
> >
> > in both cases this might lead to unexpected behavior especially on 32b
> > systems where we can expect lowmem memory pressure very often.
> >
> > A large highmem zone can easily distort SCAN_FILE heuristic because
> > there might be only few file pages from the eligible zones on the node
> > lru and we would still enforce file lru scanning which can lead to
> > trashing while we could still scan anonymous pages.
>
> Nit:
> It doesn't make thrashing because isolate_lru_pages filter out them
> but I agree it makes pointless CPU burning to find eligible pages.
This is not about isolate_lru_pages. The trashing could happen if we had
lowmem pagecache user which would constantly reclaim recently faulted
in pages while there is anonymous memory in the lowmem which could be
reclaimed instead.
[...]
> > /*
> > + * Return the number of pages on the given lru which are eligibne for the
> eligible
fixed
> > + * given zone_idx
> > + */
> > +static unsigned long lruvec_lru_size_zone_idx(struct lruvec *lruvec,
> > + enum lru_list lru, int zone_idx)
>
> Nit:
>
> Although there is a comment, function name is rather confusing when I compared
> it with lruvec_zone_lru_size.
I am all for a better name.
> lruvec_eligible_zones_lru_size is better?
this would be too easy to confuse with lruvec_eligible_zone_lru_size.
What about lruvec_lru_size_eligible_zones?
> Nit:
>
> With this patch, inactive_list_is_low can use lruvec_lru_size_zone_idx rather than
> own custom calculation to filter out non-eligible pages.
Yes, that would be possible and I was considering that. But then I found
useful to see total and reduced numbers in the tracepoint
http://lkml.kernel.org/r/20161228153032.10821-8-mhocko@kernel.org
and didn't want to call lruvec_lru_size 2 times. But if you insist then
I can just do that.
> Anyway, I think this patch does right things so I suppose this.
>
> Acked-by: Minchan Kim <minchan@kernel.org>
Thanks for the review!
--
Michal Hocko
SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Minchan Kim <minchan@kernel.org> |
|---|---|
| Date | 2016-12-30 03:10 +0100 |
| Subject | Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) |
| Message-ID | <sTUNP-6vU-5@gated-at.bofh.it> |
| In reply to | #1548252 |
On Thu, Dec 29, 2016 at 10:04:32AM +0100, Michal Hocko wrote:
> On Thu 29-12-16 10:20:26, Minchan Kim wrote:
> > On Tue, Dec 27, 2016 at 04:55:33PM +0100, Michal Hocko wrote:
> > > Hi,
> > > could you try to run with the following patch on top of the previous
> > > one? I do not think it will make a large change in your workload but
> > > I think we need something like that so some testing under which is known
> > > to make a high lowmem pressure would be really appreciated. If you have
> > > more time to play with it then running with and without the patch with
> > > mm_vmscan_direct_reclaim_{start,end} tracepoints enabled could tell us
> > > whether it make any difference at all.
> > >
> > > I would also appreciate if Mel and Johannes had a look at it. I am not
> > > yet sure whether we need the same thing for anon/file balancing in
> > > get_scan_count. I suspect we need but need to think more about that.
> > >
> > > Thanks a lot again!
> > > ---
> > > From b51f50340fe9e40b68be198b012f8ab9869c1850 Mon Sep 17 00:00:00 2001
> > > From: Michal Hocko <mhocko@suse.com>
> > > Date: Tue, 27 Dec 2016 16:28:44 +0100
> > > Subject: [PATCH] mm, vmscan: consider eligible zones in get_scan_count
> > >
> > > get_scan_count considers the whole node LRU size when
> > > - doing SCAN_FILE due to many page cache inactive pages
> > > - calculating the number of pages to scan
> > >
> > > in both cases this might lead to unexpected behavior especially on 32b
> > > systems where we can expect lowmem memory pressure very often.
> > >
> > > A large highmem zone can easily distort SCAN_FILE heuristic because
> > > there might be only few file pages from the eligible zones on the node
> > > lru and we would still enforce file lru scanning which can lead to
> > > trashing while we could still scan anonymous pages.
> >
> > Nit:
> > It doesn't make thrashing because isolate_lru_pages filter out them
> > but I agree it makes pointless CPU burning to find eligible pages.
>
> This is not about isolate_lru_pages. The trashing could happen if we had
> lowmem pagecache user which would constantly reclaim recently faulted
> in pages while there is anonymous memory in the lowmem which could be
> reclaimed instead.
>
> [...]
> > > /*
> > > + * Return the number of pages on the given lru which are eligibne for the
> > eligible
>
> fixed
>
> > > + * given zone_idx
> > > + */
> > > +static unsigned long lruvec_lru_size_zone_idx(struct lruvec *lruvec,
> > > + enum lru_list lru, int zone_idx)
> >
> > Nit:
> >
> > Although there is a comment, function name is rather confusing when I compared
> > it with lruvec_zone_lru_size.
>
> I am all for a better name.
>
> > lruvec_eligible_zones_lru_size is better?
>
> this would be too easy to confuse with lruvec_eligible_zone_lru_size.
> What about lruvec_lru_size_eligible_zones?
Don't mind.
>
> > Nit:
> >
> > With this patch, inactive_list_is_low can use lruvec_lru_size_zone_idx rather than
> > own custom calculation to filter out non-eligible pages.
>
> Yes, that would be possible and I was considering that. But then I found
> useful to see total and reduced numbers in the tracepoint
> http://lkml.kernel.org/r/20161228153032.10821-8-mhocko@kernel.org
> and didn't want to call lruvec_lru_size 2 times. But if you insist then
> I can just do that.
I don't mind either but I think we need to describe the reason if you want to
go with your open-coded version. Otherwise, someone will try to fix it.
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-12-30 11:50 +0100 |
| Subject | Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) |
| Message-ID | <sU2V3-3cQ-9@gated-at.bofh.it> |
| In reply to | #1548430 |
On Fri 30-12-16 11:05:22, Minchan Kim wrote:
> On Thu, Dec 29, 2016 at 10:04:32AM +0100, Michal Hocko wrote:
> > On Thu 29-12-16 10:20:26, Minchan Kim wrote:
> > > On Tue, Dec 27, 2016 at 04:55:33PM +0100, Michal Hocko wrote:
[...]
> > > > + * given zone_idx
> > > > + */
> > > > +static unsigned long lruvec_lru_size_zone_idx(struct lruvec *lruvec,
> > > > + enum lru_list lru, int zone_idx)
> > >
> > > Nit:
> > >
> > > Although there is a comment, function name is rather confusing when I compared
> > > it with lruvec_zone_lru_size.
> >
> > I am all for a better name.
> >
> > > lruvec_eligible_zones_lru_size is better?
> >
> > this would be too easy to confuse with lruvec_eligible_zone_lru_size.
> > What about lruvec_lru_size_eligible_zones?
>
> Don't mind.
I will go with lruvec_lru_size_eligible_zones then.
> > > Nit:
> > >
> > > With this patch, inactive_list_is_low can use lruvec_lru_size_zone_idx rather than
> > > own custom calculation to filter out non-eligible pages.
> >
> > Yes, that would be possible and I was considering that. But then I found
> > useful to see total and reduced numbers in the tracepoint
> > http://lkml.kernel.org/r/20161228153032.10821-8-mhocko@kernel.org
> > and didn't want to call lruvec_lru_size 2 times. But if you insist then
> > I can just do that.
>
> I don't mind either but I think we need to describe the reason if you want to
> go with your open-coded version. Otherwise, someone will try to fix it.
OK, I will go with the follow up patch on top of the tracepoints series.
I was hoping that the way how tracing is full of macros would allow us
to evaluate arguments only when the tracepoint is enabled but this
doesn't seem to be the case. Let's CC Steven. Would it be possible to
define a tracepoint in such a way that all given arguments are evaluated
only when the tracepoint is enabled?
---
From 9a561d652f91f3557db22161600f10ca2462c74f Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.com>
Date: Fri, 30 Dec 2016 11:28:20 +0100
Subject: [PATCH] mm, vmscan: cleanup up inactive_list_is_low
inactive_list_is_low is effectively duplicating logic implemented by
lruvec_lru_size_eligibe_zones. Let's use the dedicated function to
get the number of eligible pages on the lru list and ask use
lruvec_lru_size to get the total LRU lize only when the tracing is
really requested. We are still iterating over all LRUs two times in that
case but a) inactive_list_is_low is not a hot path and b) this can be
addressed at the tracing layer and only evaluate arguments only when the
tracing is enabled in future if that ever matters.
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
mm/vmscan.c | 38 ++++++++++----------------------------
1 file changed, 10 insertions(+), 28 deletions(-)
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 137bc85067d3..a9c881f06c0e 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2054,11 +2054,10 @@ static bool inactive_list_is_low(struct lruvec *lruvec, bool file,
struct scan_control *sc, bool trace)
{
unsigned long inactive_ratio;
- unsigned long total_inactive, inactive;
- unsigned long total_active, active;
+ unsigned long inactive, active;
+ enum lru_list inactive_lru = file * LRU_FILE;
+ enum lru_list active_lru = file * LRU_FILE + LRU_ACTIVE;
unsigned long gb;
- struct pglist_data *pgdat = lruvec_pgdat(lruvec);
- int zid;
/*
* If we don't have swap space, anonymous page deactivation
@@ -2067,27 +2066,8 @@ static bool inactive_list_is_low(struct lruvec *lruvec, bool file,
if (!file && !total_swap_pages)
return false;
- total_inactive = inactive = lruvec_lru_size(lruvec, file * LRU_FILE);
- total_active = active = lruvec_lru_size(lruvec, file * LRU_FILE + LRU_ACTIVE);
-
- /*
- * For zone-constrained allocations, it is necessary to check if
- * deactivations are required for lowmem to be reclaimed. This
- * calculates the inactive/active pages available in eligible zones.
- */
- for (zid = sc->reclaim_idx + 1; zid < MAX_NR_ZONES; zid++) {
- struct zone *zone = &pgdat->node_zones[zid];
- unsigned long inactive_zone, active_zone;
-
- if (!managed_zone(zone))
- continue;
-
- inactive_zone = lruvec_zone_lru_size(lruvec, file * LRU_FILE, zid);
- active_zone = lruvec_zone_lru_size(lruvec, (file * LRU_FILE) + LRU_ACTIVE, zid);
-
- inactive -= min(inactive, inactive_zone);
- active -= min(active, active_zone);
- }
+ inactive = lruvec_lru_size_eligibe_zones(lruvec, inactive_lru, sc->reclaim_idx);
+ active = lruvec_lru_size_eligibe_zones(lruvec, active_lru, sc->reclaim_idx);
gb = (inactive + active) >> (30 - PAGE_SHIFT);
if (gb)
@@ -2096,10 +2076,12 @@ static bool inactive_list_is_low(struct lruvec *lruvec, bool file,
inactive_ratio = 1;
if (trace)
- trace_mm_vmscan_inactive_list_is_low(pgdat->node_id,
+ trace_mm_vmscan_inactive_list_is_low(lruvec_pgdat(lruvec)->node_id,
sc->reclaim_idx,
- total_inactive, inactive,
- total_active, active, inactive_ratio, file);
+ lruvec_lru_size(lruvec, inactive_lru), inactive,
+ lruvec_lru_size(lruvec, active_lru), active,
+ inactive_ratio, file);
+
return inactive * inactive_ratio < active;
}
--
2.10.2
--
Michal Hocko
SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Minchan Kim <minchan@kernel.org> |
|---|---|
| Date | 2016-12-29 01:40 +0100 |
| Subject | Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) |
| Message-ID | <sTwVc-7Gw-15@gated-at.bofh.it> |
| In reply to | #1547313 |
On Mon, Dec 26, 2016 at 01:48:40PM +0100, Michal Hocko wrote:
> On Fri 23-12-16 23:26:00, Nils Holland wrote:
> > On Fri, Dec 23, 2016 at 03:47:39PM +0100, Michal Hocko wrote:
> > >
> > > Nils, even though this is still highly experimental, could you give it a
> > > try please?
> >
> > Yes, no problem! So I kept the very first patch you sent but had to
> > revert the latest version of the debugging patch (the one in
> > which you added the "mm_vmscan_inactive_list_is_low" event) because
> > otherwise the patch you just sent wouldn't apply. Then I rebooted with
> > memory cgroups enabled again, and the first thing that strikes the eye
> > is that I get this during boot:
> >
> > [ 1.568174] ------------[ cut here ]------------
> > [ 1.568327] WARNING: CPU: 0 PID: 1 at mm/memcontrol.c:1032 mem_cgroup_update_lru_size+0x118/0x130
> > [ 1.568543] mem_cgroup_update_lru_size(f4406400, 2, 1): lru_size 0 but not empty
>
> Ohh, I can see what is wrong! a) there is a bug in the accounting in
> my patch (I double account) and b) the detection for the empty list
> cannot work after my change because per node zone will not match per
> zone statistics. The updated patch is below. So I hope my brain already
> works after it's been mostly off last few days...
> ---
> From 397adf46917b2d9493180354a7b0182aee280a8b Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@suse.com>
> Date: Fri, 23 Dec 2016 15:11:54 +0100
> Subject: [PATCH] mm, memcg: fix the active list aging for lowmem requests when
> memcg is enabled
>
> Nils Holland has reported unexpected OOM killer invocations with 32b
> kernel starting with 4.8 kernels
>
> kworker/u4:5 invoked oom-killer: gfp_mask=0x2400840(GFP_NOFS|__GFP_NOFAIL), nodemask=0, order=0, oom_score_adj=0
> kworker/u4:5 cpuset=/ mems_allowed=0
> CPU: 1 PID: 2603 Comm: kworker/u4:5 Not tainted 4.9.0-gentoo #2
> [...]
> Mem-Info:
> active_anon:58685 inactive_anon:90 isolated_anon:0
> active_file:274324 inactive_file:281962 isolated_file:0
> unevictable:0 dirty:649 writeback:0 unstable:0
> slab_reclaimable:40662 slab_unreclaimable:17754
> mapped:7382 shmem:202 pagetables:351 bounce:0
> free:206736 free_pcp:332 free_cma:0
> Node 0 active_anon:234740kB inactive_anon:360kB active_file:1097296kB inactive_file:1127848kB unevictable:0kB isolated(anon):0kB isolated(file):0kB mapped:29528kB dirty:2596kB writeback:0kB shmem:0kB shmem_thp: 0kB shmem_pmdmapped: 184320kB anon_thp: 808kB writeback_tmp:0kB unstable:0kB pages_scanned:0 all_unreclaimable? no
> DMA free:3952kB min:788kB low:984kB high:1180kB active_anon:0kB inactive_anon:0kB active_file:7316kB inactive_file:0kB unevictable:0kB writepending:96kB present:15992kB managed:15916kB mlocked:0kB slab_reclaimable:3200kB slab_unreclaimable:1408kB kernel_stack:0kB pagetables:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB
> lowmem_reserve[]: 0 813 3474 3474
> Normal free:41332kB min:41368kB low:51708kB high:62048kB active_anon:0kB inactive_anon:0kB active_file:532748kB inactive_file:44kB unevictable:0kB writepending:24kB present:897016kB managed:836248kB mlocked:0kB slab_reclaimable:159448kB slab_unreclaimable:69608kB kernel_stack:1112kB pagetables:1404kB bounce:0kB free_pcp:528kB local_pcp:340kB free_cma:0kB
> lowmem_reserve[]: 0 0 21292 21292
> HighMem free:781660kB min:512kB low:34356kB high:68200kB active_anon:234740kB inactive_anon:360kB active_file:557232kB inactive_file:1127804kB unevictable:0kB writepending:2592kB present:2725384kB managed:2725384kB mlocked:0kB slab_reclaimable:0kB slab_unreclaimable:0kB kernel_stack:0kB pagetables:0kB bounce:0kB free_pcp:800kB local_pcp:608kB free_cma:0kB
>
> the oom killer is clearly pre-mature because there there is still a
> lot of page cache in the zone Normal which should satisfy this lowmem
> request. Further debugging has shown that the reclaim cannot make any
> forward progress because the page cache is hidden in the active list
> which doesn't get rotated because inactive_list_is_low is not memcg
> aware.
> It simply subtracts per-zone highmem counters from the respective
> memcg's lru sizes which doesn't make any sense. We can simply end up
> always seeing the resulting active and inactive counts 0 and return
> false. This issue is not limited to 32b kernels but in practice the
> effect on systems without CONFIG_HIGHMEM would be much harder to notice
> because we do not invoke the OOM killer for allocations requests
> targeting < ZONE_NORMAL.
>
> Fix the issue by tracking per zone lru page counts in mem_cgroup_per_node
> and subtract per-memcg highmem counts when memcg is enabled. Introduce
> helper lruvec_zone_lru_size which redirects to either zone counters or
> mem_cgroup_get_zone_lru_size when appropriate.
>
> We are loosing empty LRU but non-zero lru size detection introduced by
> ca707239e8a7 ("mm: update_lru_size warn and reset bad lru_size") because
> of the inherent zone vs. node discrepancy.
>
> Fixes: f8d1a31163fc ("mm: consider whether to decivate based on eligible zones inactive ratio")
> Cc: stable # 4.8+
> Reported-by: Nils Holland <nholland@tisys.org>
> Signed-off-by: Michal Hocko <mhocko@suse.com>
Acked-by: Minchan Kim <minchan@kernel.org>
[toc] | [prev] | [next] | [standalone]
| From | Minchan Kim <minchan@kernel.org> |
|---|---|
| Date | 2016-12-29 02:10 +0100 |
| Subject | Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) |
| Message-ID | <sTxod-87x-11@gated-at.bofh.it> |
| In reply to | #1548137 |
On Thu, Dec 29, 2016 at 09:31:54AM +0900, Minchan Kim wrote:
> On Mon, Dec 26, 2016 at 01:48:40PM +0100, Michal Hocko wrote:
> > On Fri 23-12-16 23:26:00, Nils Holland wrote:
> > > On Fri, Dec 23, 2016 at 03:47:39PM +0100, Michal Hocko wrote:
> > > >
> > > > Nils, even though this is still highly experimental, could you give it a
> > > > try please?
> > >
> > > Yes, no problem! So I kept the very first patch you sent but had to
> > > revert the latest version of the debugging patch (the one in
> > > which you added the "mm_vmscan_inactive_list_is_low" event) because
> > > otherwise the patch you just sent wouldn't apply. Then I rebooted with
> > > memory cgroups enabled again, and the first thing that strikes the eye
> > > is that I get this during boot:
> > >
> > > [ 1.568174] ------------[ cut here ]------------
> > > [ 1.568327] WARNING: CPU: 0 PID: 1 at mm/memcontrol.c:1032 mem_cgroup_update_lru_size+0x118/0x130
> > > [ 1.568543] mem_cgroup_update_lru_size(f4406400, 2, 1): lru_size 0 but not empty
> >
> > Ohh, I can see what is wrong! a) there is a bug in the accounting in
> > my patch (I double account) and b) the detection for the empty list
> > cannot work after my change because per node zone will not match per
> > zone statistics. The updated patch is below. So I hope my brain already
> > works after it's been mostly off last few days...
> > ---
> > From 397adf46917b2d9493180354a7b0182aee280a8b Mon Sep 17 00:00:00 2001
> > From: Michal Hocko <mhocko@suse.com>
> > Date: Fri, 23 Dec 2016 15:11:54 +0100
> > Subject: [PATCH] mm, memcg: fix the active list aging for lowmem requests when
> > memcg is enabled
> >
> > Nils Holland has reported unexpected OOM killer invocations with 32b
> > kernel starting with 4.8 kernels
> >
> > kworker/u4:5 invoked oom-killer: gfp_mask=0x2400840(GFP_NOFS|__GFP_NOFAIL), nodemask=0, order=0, oom_score_adj=0
> > kworker/u4:5 cpuset=/ mems_allowed=0
> > CPU: 1 PID: 2603 Comm: kworker/u4:5 Not tainted 4.9.0-gentoo #2
> > [...]
> > Mem-Info:
> > active_anon:58685 inactive_anon:90 isolated_anon:0
> > active_file:274324 inactive_file:281962 isolated_file:0
> > unevictable:0 dirty:649 writeback:0 unstable:0
> > slab_reclaimable:40662 slab_unreclaimable:17754
> > mapped:7382 shmem:202 pagetables:351 bounce:0
> > free:206736 free_pcp:332 free_cma:0
> > Node 0 active_anon:234740kB inactive_anon:360kB active_file:1097296kB inactive_file:1127848kB unevictable:0kB isolated(anon):0kB isolated(file):0kB mapped:29528kB dirty:2596kB writeback:0kB shmem:0kB shmem_thp: 0kB shmem_pmdmapped: 184320kB anon_thp: 808kB writeback_tmp:0kB unstable:0kB pages_scanned:0 all_unreclaimable? no
> > DMA free:3952kB min:788kB low:984kB high:1180kB active_anon:0kB inactive_anon:0kB active_file:7316kB inactive_file:0kB unevictable:0kB writepending:96kB present:15992kB managed:15916kB mlocked:0kB slab_reclaimable:3200kB slab_unreclaimable:1408kB kernel_stack:0kB pagetables:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB
> > lowmem_reserve[]: 0 813 3474 3474
> > Normal free:41332kB min:41368kB low:51708kB high:62048kB active_anon:0kB inactive_anon:0kB active_file:532748kB inactive_file:44kB unevictable:0kB writepending:24kB present:897016kB managed:836248kB mlocked:0kB slab_reclaimable:159448kB slab_unreclaimable:69608kB kernel_stack:1112kB pagetables:1404kB bounce:0kB free_pcp:528kB local_pcp:340kB free_cma:0kB
> > lowmem_reserve[]: 0 0 21292 21292
> > HighMem free:781660kB min:512kB low:34356kB high:68200kB active_anon:234740kB inactive_anon:360kB active_file:557232kB inactive_file:1127804kB unevictable:0kB writepending:2592kB present:2725384kB managed:2725384kB mlocked:0kB slab_reclaimable:0kB slab_unreclaimable:0kB kernel_stack:0kB pagetables:0kB bounce:0kB free_pcp:800kB local_pcp:608kB free_cma:0kB
> >
> > the oom killer is clearly pre-mature because there there is still a
> > lot of page cache in the zone Normal which should satisfy this lowmem
> > request. Further debugging has shown that the reclaim cannot make any
> > forward progress because the page cache is hidden in the active list
> > which doesn't get rotated because inactive_list_is_low is not memcg
> > aware.
> > It simply subtracts per-zone highmem counters from the respective
> > memcg's lru sizes which doesn't make any sense. We can simply end up
> > always seeing the resulting active and inactive counts 0 and return
> > false. This issue is not limited to 32b kernels but in practice the
> > effect on systems without CONFIG_HIGHMEM would be much harder to notice
> > because we do not invoke the OOM killer for allocations requests
> > targeting < ZONE_NORMAL.
> >
> > Fix the issue by tracking per zone lru page counts in mem_cgroup_per_node
> > and subtract per-memcg highmem counts when memcg is enabled. Introduce
> > helper lruvec_zone_lru_size which redirects to either zone counters or
> > mem_cgroup_get_zone_lru_size when appropriate.
> >
> > We are loosing empty LRU but non-zero lru size detection introduced by
> > ca707239e8a7 ("mm: update_lru_size warn and reset bad lru_size") because
> > of the inherent zone vs. node discrepancy.
> >
> > Fixes: f8d1a31163fc ("mm: consider whether to decivate based on eligible zones inactive ratio")
> > Cc: stable # 4.8+
> > Reported-by: Nils Holland <nholland@tisys.org>
> > Signed-off-by: Michal Hocko <mhocko@suse.com>
> Acked-by: Minchan Kim <minchan@kernel.org>
Nit:
WARNING: line over 80 characters
#53: FILE: include/linux/memcontrol.h:689:
+unsigned long mem_cgroup_get_zone_lru_size(struct lruvec *lruvec, enum lru_list lru,
WARNING: line over 80 characters
#147: FILE: mm/vmscan.c:248:
+unsigned long lruvec_zone_lru_size(struct lruvec *lruvec, enum lru_list lru, int zone_idx)
WARNING: line over 80 characters
#177: FILE: mm/vmscan.c:1446:
+ mem_cgroup_update_lru_size(lruvec, lru, zid, -nr_zone_taken[zid]);
WARNING: line over 80 characters
#201: FILE: mm/vmscan.c:2099:
+ inactive_zone = lruvec_zone_lru_size(lruvec, file * LRU_FILE, zid);
WARNING: line over 80 characters
#202: FILE: mm/vmscan.c:2100:
+ active_zone = lruvec_zone_lru_size(lruvec, (file * LRU_FILE) + LRU_ACTIVE, zid);
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-12-29 10:00 +0100 |
| Subject | Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) |
| Message-ID | <sTEJ4-4ke-1@gated-at.bofh.it> |
| In reply to | #1548159 |
On Thu 29-12-16 09:48:24, Minchan Kim wrote: > On Thu, Dec 29, 2016 at 09:31:54AM +0900, Minchan Kim wrote: [...] > > Acked-by: Minchan Kim <minchan@kernel.org> Thanks! > Nit: > > WARNING: line over 80 characters > #53: FILE: include/linux/memcontrol.h:689: > +unsigned long mem_cgroup_get_zone_lru_size(struct lruvec *lruvec, enum lru_list lru, > > WARNING: line over 80 characters > #147: FILE: mm/vmscan.c:248: > +unsigned long lruvec_zone_lru_size(struct lruvec *lruvec, enum lru_list lru, int zone_idx) > > WARNING: line over 80 characters > #177: FILE: mm/vmscan.c:1446: > + mem_cgroup_update_lru_size(lruvec, lru, zid, -nr_zone_taken[zid]); fixed > WARNING: line over 80 characters > #201: FILE: mm/vmscan.c:2099: > + inactive_zone = lruvec_zone_lru_size(lruvec, file * LRU_FILE, zid); > > WARNING: line over 80 characters > #202: FILE: mm/vmscan.c:2100: > + active_zone = lruvec_zone_lru_size(lruvec, (file * LRU_FILE) + LRU_ACTIVE, zid); I would prefer to have those on the same line though. It will make them easier to follow. -- Michal Hocko SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Mel Gorman <mgorman@suse.de> |
|---|---|
| Date | 2016-12-30 11:20 +0100 |
| Subject | Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) |
| Message-ID | <sU2s1-32E-5@gated-at.bofh.it> |
| In reply to | #1547313 |
On Mon, Dec 26, 2016 at 01:48:40PM +0100, Michal Hocko wrote: > On Fri 23-12-16 23:26:00, Nils Holland wrote: > > On Fri, Dec 23, 2016 at 03:47:39PM +0100, Michal Hocko wrote: > > > > > > Nils, even though this is still highly experimental, could you give it a > > > try please? > > > > Yes, no problem! So I kept the very first patch you sent but had to > > revert the latest version of the debugging patch (the one in > > which you added the "mm_vmscan_inactive_list_is_low" event) because > > otherwise the patch you just sent wouldn't apply. Then I rebooted with > > memory cgroups enabled again, and the first thing that strikes the eye > > is that I get this during boot: > > > > [ 1.568174] ------------[ cut here ]------------ > > [ 1.568327] WARNING: CPU: 0 PID: 1 at mm/memcontrol.c:1032 mem_cgroup_update_lru_size+0x118/0x130 > > [ 1.568543] mem_cgroup_update_lru_size(f4406400, 2, 1): lru_size 0 but not empty > > Ohh, I can see what is wrong! a) there is a bug in the accounting in > my patch (I double account) and b) the detection for the empty list > cannot work after my change because per node zone will not match per > zone statistics. The updated patch is below. So I hope my brain already > works after it's been mostly off last few days... > --- > From 397adf46917b2d9493180354a7b0182aee280a8b Mon Sep 17 00:00:00 2001 > From: Michal Hocko <mhocko@suse.com> > Date: Fri, 23 Dec 2016 15:11:54 +0100 > Subject: [PATCH] mm, memcg: fix the active list aging for lowmem requests when > memcg is enabled > > Nils Holland has reported unexpected OOM killer invocations with 32b > kernel starting with 4.8 kernels > I think it's unfortunate that per-zone stats are reintroduced to the memcg structure. I can't help but think that it would have also worked to always rotate a small number of pages if !inactive_list_is_low and reclaiming for memcg even if it distorted page aging. However, given that such an approach would be less robust and this has been heavily tested; Acked-by: Mel Gorman <mgorman@suse.de> -- Mel Gorman SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-12-30 12:10 +0100 |
| Subject | Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) |
| Message-ID | <sU3ep-3yT-25@gated-at.bofh.it> |
| In reply to | #1548523 |
On Fri 30-12-16 10:19:26, Mel Gorman wrote:
> On Mon, Dec 26, 2016 at 01:48:40PM +0100, Michal Hocko wrote:
> > On Fri 23-12-16 23:26:00, Nils Holland wrote:
> > > On Fri, Dec 23, 2016 at 03:47:39PM +0100, Michal Hocko wrote:
> > > >
> > > > Nils, even though this is still highly experimental, could you give it a
> > > > try please?
> > >
> > > Yes, no problem! So I kept the very first patch you sent but had to
> > > revert the latest version of the debugging patch (the one in
> > > which you added the "mm_vmscan_inactive_list_is_low" event) because
> > > otherwise the patch you just sent wouldn't apply. Then I rebooted with
> > > memory cgroups enabled again, and the first thing that strikes the eye
> > > is that I get this during boot:
> > >
> > > [ 1.568174] ------------[ cut here ]------------
> > > [ 1.568327] WARNING: CPU: 0 PID: 1 at mm/memcontrol.c:1032 mem_cgroup_update_lru_size+0x118/0x130
> > > [ 1.568543] mem_cgroup_update_lru_size(f4406400, 2, 1): lru_size 0 but not empty
> >
> > Ohh, I can see what is wrong! a) there is a bug in the accounting in
> > my patch (I double account) and b) the detection for the empty list
> > cannot work after my change because per node zone will not match per
> > zone statistics. The updated patch is below. So I hope my brain already
> > works after it's been mostly off last few days...
> > ---
> > From 397adf46917b2d9493180354a7b0182aee280a8b Mon Sep 17 00:00:00 2001
> > From: Michal Hocko <mhocko@suse.com>
> > Date: Fri, 23 Dec 2016 15:11:54 +0100
> > Subject: [PATCH] mm, memcg: fix the active list aging for lowmem requests when
> > memcg is enabled
> >
> > Nils Holland has reported unexpected OOM killer invocations with 32b
> > kernel starting with 4.8 kernels
> >
>
> I think it's unfortunate that per-zone stats are reintroduced to the
> memcg structure.
the original patch I had didn't add per zone stats but rather did a
nr_highmem counter to mem_cgroup_per_node (inside ifdeff CONFIG_HIGMEM).
This would help for this particular case but it wouldn't work for other
lowmem requests (e.g. GFP_DMA32) and with the kmem accounting this might
be a problem in future. So I've decided to go with a more generic
approach which requires per-zone tracking. I cannot say I would be
overly happy about this at all.
> I can't help but think that it would have also worked
> to always rotate a small number of pages if !inactive_list_is_low and
> reclaiming for memcg even if it distorted page aging.
I am not really sure how that would work. Do you mean something like the
following?
diff --git a/mm/vmscan.c b/mm/vmscan.c
index fa30010a5277..563ada3c02ac 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2044,6 +2044,9 @@ static bool inactive_list_is_low(struct lruvec *lruvec, bool file,
inactive = lruvec_lru_size(lruvec, file * LRU_FILE);
active = lruvec_lru_size(lruvec, file * LRU_FILE + LRU_ACTIVE);
+ if (!mem_cgroup_disabled())
+ goto out;
+
/*
* For zone-constrained allocations, it is necessary to check if
* deactivations are required for lowmem to be reclaimed. This
@@ -2063,6 +2066,7 @@ static bool inactive_list_is_low(struct lruvec *lruvec, bool file,
active -= min(active, active_zone);
}
+out:
gb = (inactive + active) >> (30 - PAGE_SHIFT);
if (gb)
inactive_ratio = int_sqrt(10 * gb);
The problem I see with such an approach is that chances are that this
would reintroduce what f8d1a31163fc ("mm: consider whether to decivate
based on eligible zones inactive ratio") tried to fix. But maybe I have
missed your point.
> However, given that such an approach would be less robust and this has
> been heavily tested;
>
> Acked-by: Mel Gorman <mgorman@suse.de>
Thanks!
--
Michal Hocko
SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Mel Gorman <mgorman@suse.de> |
|---|---|
| Date | 2016-12-30 13:50 +0100 |
| Subject | Re: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) |
| Message-ID | <sU4Nb-4oV-1@gated-at.bofh.it> |
| In reply to | #1548540 |
On Fri, Dec 30, 2016 at 12:05:45PM +0100, Michal Hocko wrote:
> On Fri 30-12-16 10:19:26, Mel Gorman wrote:
> > On Mon, Dec 26, 2016 at 01:48:40PM +0100, Michal Hocko wrote:
> > > On Fri 23-12-16 23:26:00, Nils Holland wrote:
> > > > On Fri, Dec 23, 2016 at 03:47:39PM +0100, Michal Hocko wrote:
> > > > >
> > > > > Nils, even though this is still highly experimental, could you give it a
> > > > > try please?
> > > >
> > > > Yes, no problem! So I kept the very first patch you sent but had to
> > > > revert the latest version of the debugging patch (the one in
> > > > which you added the "mm_vmscan_inactive_list_is_low" event) because
> > > > otherwise the patch you just sent wouldn't apply. Then I rebooted with
> > > > memory cgroups enabled again, and the first thing that strikes the eye
> > > > is that I get this during boot:
> > > >
> > > > [ 1.568174] ------------[ cut here ]------------
> > > > [ 1.568327] WARNING: CPU: 0 PID: 1 at mm/memcontrol.c:1032 mem_cgroup_update_lru_size+0x118/0x130
> > > > [ 1.568543] mem_cgroup_update_lru_size(f4406400, 2, 1): lru_size 0 but not empty
> > >
> > > Ohh, I can see what is wrong! a) there is a bug in the accounting in
> > > my patch (I double account) and b) the detection for the empty list
> > > cannot work after my change because per node zone will not match per
> > > zone statistics. The updated patch is below. So I hope my brain already
> > > works after it's been mostly off last few days...
> > > ---
> > > From 397adf46917b2d9493180354a7b0182aee280a8b Mon Sep 17 00:00:00 2001
> > > From: Michal Hocko <mhocko@suse.com>
> > > Date: Fri, 23 Dec 2016 15:11:54 +0100
> > > Subject: [PATCH] mm, memcg: fix the active list aging for lowmem requests when
> > > memcg is enabled
> > >
> > > Nils Holland has reported unexpected OOM killer invocations with 32b
> > > kernel starting with 4.8 kernels
> > >
> >
> > I think it's unfortunate that per-zone stats are reintroduced to the
> > memcg structure.
>
> the original patch I had didn't add per zone stats but rather did a
> nr_highmem counter to mem_cgroup_per_node (inside ifdeff CONFIG_HIGMEM).
> This would help for this particular case but it wouldn't work for other
> lowmem requests (e.g. GFP_DMA32) and with the kmem accounting this might
> be a problem in future.
That did occur to me.
> So I've decided to go with a more generic
> approach which requires per-zone tracking. I cannot say I would be
> overly happy about this at all.
>
> > I can't help but think that it would have also worked
> > to always rotate a small number of pages if !inactive_list_is_low and
> > reclaiming for memcg even if it distorted page aging.
>
> I am not really sure how that would work. Do you mean something like the
> following?
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index fa30010a5277..563ada3c02ac 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2044,6 +2044,9 @@ static bool inactive_list_is_low(struct lruvec *lruvec, bool file,
> inactive = lruvec_lru_size(lruvec, file * LRU_FILE);
> active = lruvec_lru_size(lruvec, file * LRU_FILE + LRU_ACTIVE);
>
> + if (!mem_cgroup_disabled())
> + goto out;
> +
> /*
> * For zone-constrained allocations, it is necessary to check if
> * deactivations are required for lowmem to be reclaimed. This
> @@ -2063,6 +2066,7 @@ static bool inactive_list_is_low(struct lruvec *lruvec, bool file,
> active -= min(active, active_zone);
> }
>
> +out:
> gb = (inactive + active) >> (30 - PAGE_SHIFT);
> if (gb)
> inactive_ratio = int_sqrt(10 * gb);
>
> The problem I see with such an approach is that chances are that this
> would reintroduce what f8d1a31163fc ("mm: consider whether to decivate
> based on eligible zones inactive ratio") tried to fix. But maybe I have
> missed your point.
>
No, you didn't miss the point. It was something like that I had in mind
but as I thought about it, I could see some cases where it might not work
and still cause a premature OOM. The per-zone accounting is unfortunate
but it's robust hence the Ack.
--
Mel Gorman
SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-12-26 13:30 +0100 |
| Subject | Re: [lkp-developer] [mm, memcg] d18e2b2aca: WARNING:at_mm/memcontrol.c:#mem_cgroup_update_lru_size |
| Message-ID | <sSCzD-4Kg-1@gated-at.bofh.it> |
| In reply to | #1546883 |
On Mon 26-12-16 06:25:56, kernel test robot wrote:
>
> FYI, we noticed the following commit:
>
> commit: d18e2b2aca0396849f588241e134787a829c707d ("mm, memcg: fix (Re: OOM: Better, but still there on)")
> url: https://github.com/0day-ci/linux/commits/Michal-Hocko/mm-memcg-fix-Re-OOM-Better-but-still-there-on/20161223-225057
> base: git://git.cmpxchg.org/linux-mmotm.git master
>
> in testcase: boot
>
> on test machine: qemu-system-i386 -enable-kvm -m 360M
>
> caused below changes:
>
>
> +--------------------------------------------------------+------------+------------+
> | | c7d85b880b | d18e2b2aca |
> +--------------------------------------------------------+------------+------------+
> | boot_successes | 8 | 0 |
> | boot_failures | 0 | 2 |
> | WARNING:at_mm/memcontrol.c:#mem_cgroup_update_lru_size | 0 | 2 |
> | kernel_BUG_at_mm/memcontrol.c | 0 | 2 |
> | invalid_opcode:#[##]DEBUG_PAGEALLOC | 0 | 2 |
> | Kernel_panic-not_syncing:Fatal_exception | 0 | 2 |
> +--------------------------------------------------------+------------+------------+
>
>
>
> [ 95.226364] init: tty6 main process (990) killed by TERM signal
> [ 95.314020] init: plymouth-upstart-bridge main process (1039) terminated with status 1
> [ 97.588568] ------------[ cut here ]------------
> [ 97.594364] WARNING: CPU: 0 PID: 1055 at mm/memcontrol.c:1032 mem_cgroup_update_lru_size+0xdd/0x12b
> [ 97.606654] mem_cgroup_update_lru_size(40297f00, 0, -1): lru_size 1 but empty
> [ 97.615140] Modules linked in:
> [ 97.618834] CPU: 0 PID: 1055 Comm: killall5 Not tainted 4.9.0-mm1-00095-gd18e2b2 #82
> [ 97.628008] Call Trace:
> [ 97.631025] dump_stack+0x16/0x18
> [ 97.635107] __warn+0xaf/0xc6
> [ 97.638729] ? mem_cgroup_update_lru_size+0xdd/0x12b
Do you have the full backtrace?
--
Michal Hocko
SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-12-26 14:00 +0100 |
| Subject | Re: [lkp-developer] [mm, memcg] d18e2b2aca: WARNING:at_mm/memcontrol.c:#mem_cgroup_update_lru_size |
| Message-ID | <sSD2G-4Tz-3@gated-at.bofh.it> |
| In reply to | #1547311 |
On Mon 26-12-16 13:26:51, Michal Hocko wrote: > On Mon 26-12-16 06:25:56, kernel test robot wrote: [...] > > [ 95.226364] init: tty6 main process (990) killed by TERM signal > > [ 95.314020] init: plymouth-upstart-bridge main process (1039) terminated with status 1 > > [ 97.588568] ------------[ cut here ]------------ > > [ 97.594364] WARNING: CPU: 0 PID: 1055 at mm/memcontrol.c:1032 mem_cgroup_update_lru_size+0xdd/0x12b > > [ 97.606654] mem_cgroup_update_lru_size(40297f00, 0, -1): lru_size 1 but empty > > [ 97.615140] Modules linked in: > > [ 97.618834] CPU: 0 PID: 1055 Comm: killall5 Not tainted 4.9.0-mm1-00095-gd18e2b2 #82 > > [ 97.628008] Call Trace: > > [ 97.631025] dump_stack+0x16/0x18 > > [ 97.635107] __warn+0xaf/0xc6 > > [ 97.638729] ? mem_cgroup_update_lru_size+0xdd/0x12b > > Do you have the full backtrace? It's not needed. I found the bug in my patch and it should be fixed by the updated patch http://lkml.kernel.org/r/20161226124839.GB20715@dhcp22.suse.cz -- Michal Hocko SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | "Xin Zhou" <xin.zhou@gmx.com> |
|---|---|
| Date | 2016-12-18 01:40 +0100 |
| Subject | Re: OOM: Better, but still there on |
| Message-ID | <sPxG9-7a-1@gated-at.bofh.it> |
| In reply to | #1543981 |
Hi, The system supposes to have special memory reservation for coredump and other debug info when encountering panic, the size seems configurable. Thanks, Xin Sent: Saturday, December 17, 2016 at 6:44 AM From: "Tetsuo Handa" <penguin-kernel@I-love.SAKURA.ne.jp> To: "Nils Holland" <nholland@tisys.org>, "Michal Hocko" <mhocko@kernel.org> Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, "Chris Mason" <clm@fb.com>, "David Sterba" <dsterba@suse.cz>, linux-btrfs@vger.kernel.org Subject: Re: OOM: Better, but still there on On 2016/12/17 21:59, Nils Holland wrote: > On Sat, Dec 17, 2016 at 01:02:03AM +0100, Michal Hocko wrote: >> mount -t tracefs none /debug/trace >> echo 1 > /debug/trace/events/vmscan/enable >> cat /debug/trace/trace_pipe > trace.log >> >> should help >> [...] > > No problem! I enabled writing the trace data to a file and then tried > to trigger another OOM situation. That worked, this time without a > complete kernel panic, but with only my processes being killed and the > system becoming unresponsive. When that happened, I let it run for > another minute or two so that in case it was still logging something > to the trace file, it could continue to do so some time longer. Then I > rebooted with the only thing that still worked, i.e. by means of magic > SysRequest. Under OOM situation, writing to a file on disk unlikely works. Maybe logging via network ( "cat /debug/trace/trace_pipe > /dev/udp/$ip/$port" if your are using bash) works better. (I wish we can do it from kernel so that /bin/cat is not disturbed by delays due to page fault.) If you can configure netconsole for logging OOM killer messages and UDP socket for logging trace_pipe messages, udplogger at https://osdn.net/projects/akari/scm/svn/tree/head/branches/udplogger/ might fit for logging both output with timestamp into a single file. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html[http://vger.kernel.org/majordomo-info.html]
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-12-16 23:20 +0100 |
| Message-ID | <sP918-11E-5@gated-at.bofh.it> |
| In reply to | #1543230 |
On Fri 16-12-16 13:15:18, Chris Mason wrote: > On 12/16/2016 02:39 AM, Michal Hocko wrote: [...] > > I believe the right way to go around this is to pursue what I've started > > in [1]. I will try to prepare something for testing today for you. Stay > > tuned. But I would be really happy if somebody from the btrfs camp could > > check the NOFS aspect of this allocation. We have already seen > > allocation stalls from this path quite recently > > Just double checking, are you asking why we're using GFP_NOFS to avoid going > into btrfs from the btrfs writepages call, or are you asking why we aren't > allowing highmem? I am more interested in the NOFS part. Why cannot this be a full GFP_KERNEL context? What kind of locks we would lock up when recursing to the fs via slab shrinkers? -- Michal Hocko SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-12-17 00:40 +0100 |
| Message-ID | <sPagx-1J3-1@gated-at.bofh.it> |
| In reply to | #1543786 |
On Fri 16-12-16 17:47:25, Chris Mason wrote: > On 12/16/2016 05:14 PM, Michal Hocko wrote: > > On Fri 16-12-16 13:15:18, Chris Mason wrote: > > > On 12/16/2016 02:39 AM, Michal Hocko wrote: > > [...] > > > > I believe the right way to go around this is to pursue what I've started > > > > in [1]. I will try to prepare something for testing today for you. Stay > > > > tuned. But I would be really happy if somebody from the btrfs camp could > > > > check the NOFS aspect of this allocation. We have already seen > > > > allocation stalls from this path quite recently > > > > > > Just double checking, are you asking why we're using GFP_NOFS to avoid going > > > into btrfs from the btrfs writepages call, or are you asking why we aren't > > > allowing highmem? > > > > I am more interested in the NOFS part. Why cannot this be a full > > GFP_KERNEL context? What kind of locks we would lock up when recursing > > to the fs via slab shrinkers? > > > > Since this is our writepages call, any jump into direct reclaim would go to > writepage, which would end up calling the same set of code to read metadata > blocks, which would do a GFP_KERNEL allocation and end up back in writepage > again. But we are not doing pageout on the page cache from the direct reclaim for a long time. So basically the only way to recurse back to the fs code is via slab ([di]cache) shrinkers. Are those a problem as well? -- 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