From: Michal Hocko <mhocko@kernel.org> To: Mel Gorman <mgorman@suse.de> Cc: Nils Holland <nholland@tisys.org>, Johannes Weiner <hannes@cmpxchg.org>, Vladimir Davydov <vdavydov.dev@gmail.com>, Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>, 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: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) Date: Fri, 30 Dec 2016 12:05:45 +0100 [thread overview] Message-ID: <20161230110545.GF13301@dhcp22.suse.cz> (raw) In-Reply-To: <20161230101926.jjjw76negqcvyaim@suse.de> 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
WARNING: multiple messages have this Message-ID (diff)
From: Michal Hocko <mhocko@kernel.org> To: Mel Gorman <mgorman@suse.de> Cc: Nils Holland <nholland@tisys.org>, Johannes Weiner <hannes@cmpxchg.org>, Vladimir Davydov <vdavydov.dev@gmail.com>, Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>, 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: [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) Date: Fri, 30 Dec 2016 12:05:45 +0100 [thread overview] Message-ID: <20161230110545.GF13301@dhcp22.suse.cz> (raw) In-Reply-To: <20161230101926.jjjw76negqcvyaim@suse.de> 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 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2016-12-30 11:05 UTC|newest] Thread overview: 123+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-12-15 22:57 OOM: Better, but still there on 4.9 Nils Holland 2016-12-16 7:39 ` Michal Hocko 2016-12-16 7:39 ` Michal Hocko 2016-12-16 15:58 ` OOM: Better, but still there on Michal Hocko 2016-12-16 15:58 ` Michal Hocko 2016-12-16 15:58 ` [PATCH 1/2] mm: consolidate GFP_NOFAIL checks in the allocator slowpath Michal Hocko 2016-12-16 15:58 ` Michal Hocko 2016-12-16 15:58 ` [PATCH 2/2] mm, oom: do not enfore OOM killer for __GFP_NOFAIL automatically Michal Hocko 2016-12-16 15:58 ` Michal Hocko 2016-12-16 17:31 ` Johannes Weiner 2016-12-16 17:31 ` Johannes Weiner 2016-12-16 22:12 ` Michal Hocko 2016-12-16 22:12 ` Michal Hocko 2016-12-17 11:17 ` Tetsuo Handa 2016-12-17 11:17 ` Tetsuo Handa 2016-12-18 16:37 ` Michal Hocko 2016-12-18 16:37 ` Michal Hocko 2016-12-16 18:47 ` OOM: Better, but still there on Nils Holland 2016-12-16 18:47 ` Nils Holland 2016-12-17 0:02 ` Michal Hocko 2016-12-17 0:02 ` Michal Hocko 2016-12-17 12:59 ` Nils Holland 2016-12-17 12:59 ` Nils Holland 2016-12-17 14:44 ` Tetsuo Handa 2016-12-17 14:44 ` Tetsuo Handa 2016-12-17 17:11 ` Nils Holland 2016-12-17 17:11 ` Nils Holland 2016-12-17 21:06 ` Nils Holland 2016-12-17 21:06 ` Nils Holland 2016-12-18 5:14 ` Tetsuo Handa 2016-12-18 5:14 ` Tetsuo Handa 2016-12-19 13:45 ` Michal Hocko 2016-12-19 13:45 ` Michal Hocko 2016-12-20 2:08 ` Nils Holland 2016-12-20 2:08 ` Nils Holland 2016-12-21 7:36 ` Michal Hocko 2016-12-21 7:36 ` Michal Hocko 2016-12-21 11:00 ` Tetsuo Handa 2016-12-21 11:00 ` Tetsuo Handa 2016-12-21 11:16 ` Michal Hocko 2016-12-21 11:16 ` Michal Hocko 2016-12-21 14:04 ` Chris Mason 2016-12-21 14:04 ` Chris Mason 2016-12-22 10:10 ` Nils Holland 2016-12-22 10:10 ` Nils Holland 2016-12-22 10:27 ` Michal Hocko 2016-12-22 10:27 ` Michal Hocko 2016-12-22 10:35 ` Nils Holland 2016-12-22 10:35 ` Nils Holland 2016-12-22 10:46 ` Tetsuo Handa 2016-12-22 10:46 ` Tetsuo Handa 2016-12-22 19:17 ` Michal Hocko 2016-12-22 19:17 ` Michal Hocko 2016-12-22 21:46 ` Nils Holland 2016-12-22 21:46 ` Nils Holland 2016-12-23 10:51 ` Michal Hocko 2016-12-23 10:51 ` Michal Hocko 2016-12-23 12:18 ` Nils Holland 2016-12-23 12:18 ` Nils Holland 2016-12-23 12:57 ` Michal Hocko 2016-12-23 12:57 ` Michal Hocko 2016-12-23 14:47 ` [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) Michal Hocko 2016-12-23 14:47 ` Michal Hocko 2016-12-23 22:26 ` Nils Holland 2016-12-23 22:26 ` Nils Holland 2016-12-26 12:48 ` Michal Hocko 2016-12-26 12:48 ` Michal Hocko 2016-12-26 18:57 ` Nils Holland 2016-12-26 18:57 ` Nils Holland 2016-12-27 8:08 ` Michal Hocko 2016-12-27 8:08 ` Michal Hocko 2016-12-27 11:23 ` Nils Holland 2016-12-27 11:23 ` Nils Holland 2016-12-27 11:27 ` Michal Hocko 2016-12-27 11:27 ` Michal Hocko 2016-12-27 15:55 ` Michal Hocko 2016-12-27 15:55 ` Michal Hocko 2016-12-27 16:28 ` [PATCH] mm, vmscan: consider eligible zones in get_scan_count kbuild test robot 2016-12-28 8:51 ` Michal Hocko 2016-12-28 8:51 ` Michal Hocko 2016-12-27 19:33 ` [RFC PATCH] mm, memcg: fix (Re: OOM: Better, but still there on) Nils Holland 2016-12-27 19:33 ` Nils Holland 2016-12-28 8:57 ` Michal Hocko 2016-12-28 8:57 ` Michal Hocko 2016-12-29 1:20 ` Minchan Kim 2016-12-29 1:20 ` Minchan Kim 2016-12-29 9:04 ` Michal Hocko 2016-12-29 9:04 ` Michal Hocko 2016-12-30 2:05 ` Minchan Kim 2016-12-30 2:05 ` Minchan Kim 2016-12-30 10:40 ` Michal Hocko 2016-12-30 10:40 ` Michal Hocko 2016-12-29 0:31 ` Minchan Kim 2016-12-29 0:31 ` Minchan Kim 2016-12-29 0:48 ` Minchan Kim 2016-12-29 0:48 ` Minchan Kim 2016-12-29 8:52 ` Michal Hocko 2016-12-29 8:52 ` Michal Hocko 2016-12-30 10:19 ` Mel Gorman 2016-12-30 10:19 ` Mel Gorman 2016-12-30 11:05 ` Michal Hocko [this message] 2016-12-30 11:05 ` Michal Hocko 2016-12-30 12:43 ` Mel Gorman 2016-12-30 12:43 ` Mel Gorman 2016-12-25 22:25 ` [lkp-developer] [mm, memcg] d18e2b2aca: WARNING:at_mm/memcontrol.c:#mem_cgroup_update_lru_size kernel test robot 2016-12-25 22:25 ` kernel test robot 2016-12-26 12:26 ` Michal Hocko 2016-12-26 12:26 ` Michal Hocko 2016-12-26 12:26 ` Michal Hocko 2016-12-26 12:50 ` Michal Hocko 2016-12-26 12:50 ` Michal Hocko 2016-12-26 12:50 ` Michal Hocko 2016-12-18 0:28 ` OOM: Better, but still there on Xin Zhou 2016-12-16 18:15 ` OOM: Better, but still there on 4.9 Chris Mason 2016-12-16 18:15 ` Chris Mason 2016-12-16 22:14 ` Michal Hocko 2016-12-16 22:14 ` Michal Hocko 2016-12-16 22:47 ` Chris Mason 2016-12-16 22:47 ` Chris Mason 2016-12-16 23:31 ` Michal Hocko 2016-12-16 23:31 ` Michal Hocko 2016-12-16 19:50 ` Chris Mason 2016-12-16 19:50 ` Chris Mason
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20161230110545.GF13301@dhcp22.suse.cz \ --to=mhocko@kernel.org \ --cc=clm@fb.com \ --cc=dsterba@suse.cz \ --cc=hannes@cmpxchg.org \ --cc=linux-btrfs@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mgorman@suse.de \ --cc=nholland@tisys.org \ --cc=penguin-kernel@I-love.SAKURA.ne.jp \ --cc=vdavydov.dev@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.