From: Michal Hocko <mhocko@kernel.org> To: Andrey Ryabinin <aryabinin@virtuozzo.com> Cc: Andrew Morton <akpm@linux-foundation.org>, Johannes Weiner <hannes@cmpxchg.org>, Vladimir Davydov <vdavydov.dev@gmail.com>, cgroups@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 1/2] mm/memcg: try harder to decrease [memory,memsw].limit_in_bytes Date: Wed, 20 Dec 2017 14:53:29 +0100 [thread overview] Message-ID: <20171220135329.GS4831@dhcp22.suse.cz> (raw) In-Reply-To: <20171220132114.6883-1-aryabinin@virtuozzo.com> On Wed 20-12-17 16:21:13, Andrey Ryabinin wrote: > mem_cgroup_resize_[memsw]_limit() tries to free only 32 (SWAP_CLUSTER_MAX) > pages on each iteration. This makes practically impossible to decrease > limit of memory cgroup. Tasks could easily allocate back 32 pages, > so we can't reduce memory usage, and once retry_count reaches zero we return > -EBUSY. > > Easy to reproduce the problem by running the following commands: > > mkdir /sys/fs/cgroup/memory/test > echo $$ >> /sys/fs/cgroup/memory/test/tasks > cat big_file > /dev/null & > sleep 1 && echo $((100*1024*1024)) > /sys/fs/cgroup/memory/test/memory.limit_in_bytes > -bash: echo: write error: Device or resource busy > > Instead of relying on retry_count, keep trying to free required amount of pages > until reclaimer makes any progress. The wording of the changelog has some room for improvements. The last sentence should read something like "Instead of relying on retry_count, keep retrying the reclaim until the desired limit is reached or fail if the reclaim doesn't make any progress or a signal is pending." I am bussy as hell today so I will look closer tomorrow or on Friday. But from a very quick glance the patch seems reasonable. > Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com> > --- > mm/memcontrol.c | 70 +++++++++++++-------------------------------------------- > 1 file changed, 16 insertions(+), 54 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index f40b5ad3f959..0d26db9a665d 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -1176,20 +1176,6 @@ void mem_cgroup_print_oom_info(struct mem_cgroup *memcg, struct task_struct *p) > } > > /* > - * This function returns the number of memcg under hierarchy tree. Returns > - * 1(self count) if no children. > - */ > -static int mem_cgroup_count_children(struct mem_cgroup *memcg) > -{ > - int num = 0; > - struct mem_cgroup *iter; > - > - for_each_mem_cgroup_tree(iter, memcg) > - num++; > - return num; > -} > - > -/* > * Return the memory (and swap, if configured) limit for a memcg. > */ > unsigned long mem_cgroup_get_limit(struct mem_cgroup *memcg) > @@ -2462,22 +2448,10 @@ static DEFINE_MUTEX(memcg_limit_mutex); > static int mem_cgroup_resize_limit(struct mem_cgroup *memcg, > unsigned long limit) > { > - unsigned long curusage; > - unsigned long oldusage; > + unsigned long usage; > bool enlarge = false; > - int retry_count; > int ret; > > - /* > - * For keeping hierarchical_reclaim simple, how long we should retry > - * is depends on callers. We set our retry-count to be function > - * of # of children which we should visit in this loop. > - */ > - retry_count = MEM_CGROUP_RECLAIM_RETRIES * > - mem_cgroup_count_children(memcg); > - > - oldusage = page_counter_read(&memcg->memory); > - > do { > if (signal_pending(current)) { > ret = -EINTR; > @@ -2498,15 +2472,13 @@ static int mem_cgroup_resize_limit(struct mem_cgroup *memcg, > if (!ret) > break; > > - try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, true); > - > - curusage = page_counter_read(&memcg->memory); > - /* Usage is reduced ? */ > - if (curusage >= oldusage) > - retry_count--; > - else > - oldusage = curusage; > - } while (retry_count); > + usage = page_counter_read(&memcg->memory); > + if (!try_to_free_mem_cgroup_pages(memcg, usage - limit, > + GFP_KERNEL, true)) { > + ret = -EBUSY; > + break; > + } > + } while (true); > > if (!ret && enlarge) > memcg_oom_recover(memcg); > @@ -2517,18 +2489,10 @@ static int mem_cgroup_resize_limit(struct mem_cgroup *memcg, > static int mem_cgroup_resize_memsw_limit(struct mem_cgroup *memcg, > unsigned long limit) > { > - unsigned long curusage; > - unsigned long oldusage; > + unsigned long usage; > bool enlarge = false; > - int retry_count; > int ret; > > - /* see mem_cgroup_resize_res_limit */ > - retry_count = MEM_CGROUP_RECLAIM_RETRIES * > - mem_cgroup_count_children(memcg); > - > - oldusage = page_counter_read(&memcg->memsw); > - > do { > if (signal_pending(current)) { > ret = -EINTR; > @@ -2549,15 +2513,13 @@ static int mem_cgroup_resize_memsw_limit(struct mem_cgroup *memcg, > if (!ret) > break; > > - try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, false); > - > - curusage = page_counter_read(&memcg->memsw); > - /* Usage is reduced ? */ > - if (curusage >= oldusage) > - retry_count--; > - else > - oldusage = curusage; > - } while (retry_count); > + usage = page_counter_read(&memcg->memsw); > + if (!try_to_free_mem_cgroup_pages(memcg, usage - limit, > + GFP_KERNEL, false)) { > + ret = -EBUSY; > + break; > + } > + } while (true); > > if (!ret && enlarge) > memcg_oom_recover(memcg); > -- > 2.13.6 > > -- > 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> -- Michal Hocko SUSE Labs
WARNING: multiple messages have this Message-ID (diff)
From: Michal Hocko <mhocko@kernel.org> To: Andrey Ryabinin <aryabinin@virtuozzo.com> Cc: Andrew Morton <akpm@linux-foundation.org>, Johannes Weiner <hannes@cmpxchg.org>, Vladimir Davydov <vdavydov.dev@gmail.com>, cgroups@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 1/2] mm/memcg: try harder to decrease [memory,memsw].limit_in_bytes Date: Wed, 20 Dec 2017 14:53:29 +0100 [thread overview] Message-ID: <20171220135329.GS4831@dhcp22.suse.cz> (raw) In-Reply-To: <20171220132114.6883-1-aryabinin@virtuozzo.com> On Wed 20-12-17 16:21:13, Andrey Ryabinin wrote: > mem_cgroup_resize_[memsw]_limit() tries to free only 32 (SWAP_CLUSTER_MAX) > pages on each iteration. This makes practically impossible to decrease > limit of memory cgroup. Tasks could easily allocate back 32 pages, > so we can't reduce memory usage, and once retry_count reaches zero we return > -EBUSY. > > Easy to reproduce the problem by running the following commands: > > mkdir /sys/fs/cgroup/memory/test > echo $$ >> /sys/fs/cgroup/memory/test/tasks > cat big_file > /dev/null & > sleep 1 && echo $((100*1024*1024)) > /sys/fs/cgroup/memory/test/memory.limit_in_bytes > -bash: echo: write error: Device or resource busy > > Instead of relying on retry_count, keep trying to free required amount of pages > until reclaimer makes any progress. The wording of the changelog has some room for improvements. The last sentence should read something like "Instead of relying on retry_count, keep retrying the reclaim until the desired limit is reached or fail if the reclaim doesn't make any progress or a signal is pending." I am bussy as hell today so I will look closer tomorrow or on Friday. But from a very quick glance the patch seems reasonable. > Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com> > --- > mm/memcontrol.c | 70 +++++++++++++-------------------------------------------- > 1 file changed, 16 insertions(+), 54 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index f40b5ad3f959..0d26db9a665d 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -1176,20 +1176,6 @@ void mem_cgroup_print_oom_info(struct mem_cgroup *memcg, struct task_struct *p) > } > > /* > - * This function returns the number of memcg under hierarchy tree. Returns > - * 1(self count) if no children. > - */ > -static int mem_cgroup_count_children(struct mem_cgroup *memcg) > -{ > - int num = 0; > - struct mem_cgroup *iter; > - > - for_each_mem_cgroup_tree(iter, memcg) > - num++; > - return num; > -} > - > -/* > * Return the memory (and swap, if configured) limit for a memcg. > */ > unsigned long mem_cgroup_get_limit(struct mem_cgroup *memcg) > @@ -2462,22 +2448,10 @@ static DEFINE_MUTEX(memcg_limit_mutex); > static int mem_cgroup_resize_limit(struct mem_cgroup *memcg, > unsigned long limit) > { > - unsigned long curusage; > - unsigned long oldusage; > + unsigned long usage; > bool enlarge = false; > - int retry_count; > int ret; > > - /* > - * For keeping hierarchical_reclaim simple, how long we should retry > - * is depends on callers. We set our retry-count to be function > - * of # of children which we should visit in this loop. > - */ > - retry_count = MEM_CGROUP_RECLAIM_RETRIES * > - mem_cgroup_count_children(memcg); > - > - oldusage = page_counter_read(&memcg->memory); > - > do { > if (signal_pending(current)) { > ret = -EINTR; > @@ -2498,15 +2472,13 @@ static int mem_cgroup_resize_limit(struct mem_cgroup *memcg, > if (!ret) > break; > > - try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, true); > - > - curusage = page_counter_read(&memcg->memory); > - /* Usage is reduced ? */ > - if (curusage >= oldusage) > - retry_count--; > - else > - oldusage = curusage; > - } while (retry_count); > + usage = page_counter_read(&memcg->memory); > + if (!try_to_free_mem_cgroup_pages(memcg, usage - limit, > + GFP_KERNEL, true)) { > + ret = -EBUSY; > + break; > + } > + } while (true); > > if (!ret && enlarge) > memcg_oom_recover(memcg); > @@ -2517,18 +2489,10 @@ static int mem_cgroup_resize_limit(struct mem_cgroup *memcg, > static int mem_cgroup_resize_memsw_limit(struct mem_cgroup *memcg, > unsigned long limit) > { > - unsigned long curusage; > - unsigned long oldusage; > + unsigned long usage; > bool enlarge = false; > - int retry_count; > int ret; > > - /* see mem_cgroup_resize_res_limit */ > - retry_count = MEM_CGROUP_RECLAIM_RETRIES * > - mem_cgroup_count_children(memcg); > - > - oldusage = page_counter_read(&memcg->memsw); > - > do { > if (signal_pending(current)) { > ret = -EINTR; > @@ -2549,15 +2513,13 @@ static int mem_cgroup_resize_memsw_limit(struct mem_cgroup *memcg, > if (!ret) > break; > > - try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, false); > - > - curusage = page_counter_read(&memcg->memsw); > - /* Usage is reduced ? */ > - if (curusage >= oldusage) > - retry_count--; > - else > - oldusage = curusage; > - } while (retry_count); > + usage = page_counter_read(&memcg->memsw); > + if (!try_to_free_mem_cgroup_pages(memcg, usage - limit, > + GFP_KERNEL, false)) { > + ret = -EBUSY; > + break; > + } > + } while (true); > > if (!ret && enlarge) > memcg_oom_recover(memcg); > -- > 2.13.6 > > -- > 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> -- 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:[~2017-12-20 13:53 UTC|newest] Thread overview: 125+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-12-20 10:24 [PATCH 1/2] mm/memcg: try harder to decrease [memory,memsw].limit_in_bytes Andrey Ryabinin 2017-12-20 10:24 ` Andrey Ryabinin 2017-12-20 10:24 ` [PATCH 2/2] mm/memcg: Consolidate mem_cgroup_resize_[memsw]_limit() functions Andrey Ryabinin 2017-12-20 10:24 ` Andrey Ryabinin 2017-12-20 10:33 ` [PATCH 1/2] mm/memcg: try harder to decrease [memory,memsw].limit_in_bytes Michal Hocko 2017-12-20 10:33 ` Michal Hocko 2017-12-20 11:32 ` Andrey Ryabinin 2017-12-20 11:32 ` Andrey Ryabinin 2017-12-20 11:34 ` Michal Hocko 2017-12-20 11:34 ` Michal Hocko 2017-12-20 18:15 ` Shakeel Butt 2017-12-20 18:15 ` Shakeel Butt 2017-12-21 10:00 ` Andrey Ryabinin 2017-12-21 10:00 ` Andrey Ryabinin 2017-12-21 10:00 ` Andrey Ryabinin 2017-12-20 13:21 ` [PATCH v2 " Andrey Ryabinin 2017-12-20 13:21 ` Andrey Ryabinin 2017-12-20 13:21 ` [PATCH v2 2/2] mm/memcg: Consolidate mem_cgroup_resize_[memsw]_limit() functions Andrey Ryabinin 2017-12-20 13:21 ` Andrey Ryabinin 2017-12-20 13:53 ` Michal Hocko [this message] 2017-12-20 13:53 ` [PATCH v2 1/2] mm/memcg: try harder to decrease [memory,memsw].limit_in_bytes Michal Hocko 2018-01-09 16:58 ` [PATCH v3 " Andrey Ryabinin 2018-01-09 16:58 ` Andrey Ryabinin 2018-01-09 16:58 ` [PATCH v3 2/2] mm/memcg: Consolidate mem_cgroup_resize_[memsw]_limit() functions Andrey Ryabinin 2018-01-09 16:58 ` Andrey Ryabinin 2018-01-09 17:10 ` Shakeel Butt 2018-01-09 17:10 ` Shakeel Butt 2018-01-09 17:10 ` Shakeel Butt 2018-01-09 17:26 ` Andrey Ryabinin 2018-01-09 17:26 ` Andrey Ryabinin 2018-01-09 23:26 ` Andrew Morton 2018-01-09 23:26 ` Andrew Morton 2018-01-10 12:43 ` [PATCH v4] mm/memcg: try harder to decrease [memory,memsw].limit_in_bytes Andrey Ryabinin 2018-01-10 12:43 ` Andrey Ryabinin 2018-01-10 12:43 ` Andrey Ryabinin 2018-01-10 22:31 ` Andrew Morton 2018-01-10 22:31 ` Andrew Morton 2018-01-11 11:59 ` Andrey Ryabinin 2018-01-11 11:59 ` Andrey Ryabinin 2018-01-12 0:21 ` Andrew Morton 2018-01-12 0:21 ` Andrew Morton 2018-01-12 0:21 ` Andrew Morton 2018-01-12 9:08 ` Andrey Ryabinin 2018-01-12 9:08 ` Andrey Ryabinin 2018-01-11 10:42 ` Michal Hocko 2018-01-11 10:42 ` Michal Hocko 2018-01-11 10:42 ` Michal Hocko 2018-01-11 12:21 ` Andrey Ryabinin 2018-01-11 12:21 ` Andrey Ryabinin 2018-01-11 12:21 ` Andrey Ryabinin 2018-01-11 12:46 ` Michal Hocko 2018-01-11 12:46 ` Michal Hocko 2018-01-11 15:23 ` Andrey Ryabinin 2018-01-11 15:23 ` Andrey Ryabinin 2018-01-11 15:23 ` Andrey Ryabinin 2018-01-11 16:29 ` Michal Hocko 2018-01-11 16:29 ` Michal Hocko 2018-01-11 16:29 ` Michal Hocko 2018-01-11 21:59 ` Andrey Ryabinin 2018-01-11 21:59 ` Andrey Ryabinin 2018-01-11 21:59 ` Andrey Ryabinin 2018-01-12 12:24 ` Michal Hocko 2018-01-12 12:24 ` Michal Hocko 2018-01-12 22:57 ` Shakeel Butt 2018-01-12 22:57 ` Shakeel Butt 2018-01-12 22:57 ` Shakeel Butt 2018-01-15 12:29 ` Andrey Ryabinin 2018-01-15 12:29 ` Andrey Ryabinin 2018-01-15 17:04 ` Shakeel Butt 2018-01-15 17:04 ` Shakeel Butt 2018-01-15 17:04 ` Shakeel Butt 2018-01-15 12:30 ` Andrey Ryabinin 2018-01-15 12:30 ` Andrey Ryabinin 2018-01-15 12:46 ` Michal Hocko 2018-01-15 12:46 ` Michal Hocko 2018-01-15 12:53 ` Andrey Ryabinin 2018-01-15 12:53 ` Andrey Ryabinin 2018-01-15 12:58 ` Michal Hocko 2018-01-15 12:58 ` Michal Hocko 2018-01-09 17:08 ` [PATCH v3 1/2] " Andrey Ryabinin 2018-01-09 17:08 ` Andrey Ryabinin 2018-01-09 17:08 ` Andrey Ryabinin 2018-01-09 17:22 ` Shakeel Butt 2018-01-09 17:22 ` Shakeel Butt 2018-01-19 13:25 ` [PATCH v5 1/2] mm/memcontrol.c: " Andrey Ryabinin 2018-01-19 13:25 ` Andrey Ryabinin 2018-01-19 13:25 ` Andrey Ryabinin 2018-01-19 13:25 ` [PATCH v5 2/2] mm/memcontrol.c: Reduce reclaim retries in mem_cgroup_resize_limit() Andrey Ryabinin 2018-01-19 13:25 ` Andrey Ryabinin 2018-01-19 13:35 ` Michal Hocko 2018-01-19 13:35 ` Michal Hocko 2018-01-19 14:49 ` Shakeel Butt 2018-01-19 14:49 ` Shakeel Butt 2018-01-19 14:49 ` Shakeel Butt 2018-01-19 15:11 ` Michal Hocko 2018-01-19 15:11 ` Michal Hocko 2018-01-19 15:11 ` Michal Hocko 2018-01-19 15:24 ` Shakeel Butt 2018-01-19 15:24 ` Shakeel Butt 2018-01-19 15:31 ` Michal Hocko 2018-01-19 15:31 ` Michal Hocko 2018-01-19 15:31 ` Michal Hocko 2018-02-21 20:17 ` Andrew Morton 2018-02-21 20:17 ` Andrew Morton 2018-02-22 13:50 ` Andrey Ryabinin 2018-02-22 13:50 ` Andrey Ryabinin 2018-02-22 14:09 ` Michal Hocko 2018-02-22 14:09 ` Michal Hocko 2018-02-22 15:13 ` Andrey Ryabinin 2018-02-22 15:13 ` Andrey Ryabinin 2018-02-22 15:33 ` Michal Hocko 2018-02-22 15:33 ` Michal Hocko 2018-02-22 15:38 ` Andrey Ryabinin 2018-02-22 15:38 ` Andrey Ryabinin 2018-02-22 15:44 ` Michal Hocko 2018-02-22 15:44 ` Michal Hocko 2018-02-22 16:01 ` Andrey Ryabinin 2018-02-22 16:01 ` Andrey Ryabinin 2018-02-22 16:30 ` Michal Hocko 2018-02-22 16:30 ` Michal Hocko 2018-01-19 13:32 ` [PATCH v5 1/2] mm/memcontrol.c: try harder to decrease [memory,memsw].limit_in_bytes Michal Hocko 2018-01-19 13:32 ` Michal Hocko 2018-01-19 13:32 ` Michal Hocko 2018-01-25 19:44 ` Andrey Ryabinin 2018-01-25 19:44 ` Andrey Ryabinin
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=20171220135329.GS4831@dhcp22.suse.cz \ --to=mhocko@kernel.org \ --cc=akpm@linux-foundation.org \ --cc=aryabinin@virtuozzo.com \ --cc=cgroups@vger.kernel.org \ --cc=hannes@cmpxchg.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --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.