From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755411AbeASNci (ORCPT ); Fri, 19 Jan 2018 08:32:38 -0500 Received: from mx2.suse.de ([195.135.220.15]:34602 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754759AbeASNca (ORCPT ); Fri, 19 Jan 2018 08:32:30 -0500 Date: Fri, 19 Jan 2018 14:32:27 +0100 From: Michal Hocko To: Andrey Ryabinin Cc: Andrew Morton , cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Shakeel Butt , Johannes Weiner , Vladimir Davydov Subject: Re: [PATCH v5 1/2] mm/memcontrol.c: try harder to decrease [memory,memsw].limit_in_bytes Message-ID: <20180119133227.GC6584@dhcp22.suse.cz> References: <20171220102429.31601-1-aryabinin@virtuozzo.com> <20180119132544.19569-1-aryabinin@virtuozzo.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180119132544.19569-1-aryabinin@virtuozzo.com> User-Agent: Mutt/1.9.2 (2017-12-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri 19-01-18 16:25:43, Andrey Ryabinin wrote: > mem_cgroup_resize_[memsw]_limit() tries to free only 32 (SWAP_CLUSTER_MAX) > pages on each iteration. This makes it 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 retrying the reclaim until the > desired limit is reached or fail if the reclaim doesn't make any progress > or a signal is pending. Thanks for splitting the original patch. I am OK with this part. > Signed-off-by: Andrey Ryabinin > Cc: Shakeel Butt > Cc: Michal Hocko > Cc: Johannes Weiner > Cc: Vladimir Davydov Acked-by: Michal Hocko > --- > mm/memcontrol.c | 42 ++++++------------------------------------ > 1 file changed, 6 insertions(+), 36 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 13aeccf32c2e..9d987f3e79dc 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,24 +2448,11 @@ static DEFINE_MUTEX(memcg_limit_mutex); > static int mem_cgroup_resize_limit(struct mem_cgroup *memcg, > unsigned long limit, bool memsw) > { > - unsigned long curusage; > - unsigned long oldusage; > bool enlarge = false; > - int retry_count; > int ret; > bool limits_invariant; > struct page_counter *counter = memsw ? &memcg->memsw : &memcg->memory; > > - /* > - * 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(counter); > - > do { > if (signal_pending(current)) { > ret = -EINTR; > @@ -2506,15 +2479,12 @@ static int mem_cgroup_resize_limit(struct mem_cgroup *memcg, > if (!ret) > break; > > - try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, !memsw); > - > - curusage = page_counter_read(counter); > - /* Usage is reduced ? */ > - if (curusage >= oldusage) > - retry_count--; > - else > - oldusage = curusage; > - } while (retry_count); > + if (!try_to_free_mem_cgroup_pages(memcg, 1, > + GFP_KERNEL, !memsw)) { > + ret = -EBUSY; > + break; > + } > + } while (true); > > if (!ret && enlarge) > memcg_oom_recover(memcg); > -- > 2.13.6 > > -- > To unsubscribe from this list: send the line "unsubscribe cgroups" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Michal Hocko SUSE Labs From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f198.google.com (mail-wr0-f198.google.com [209.85.128.198]) by kanga.kvack.org (Postfix) with ESMTP id 1CEB66B0038 for ; Fri, 19 Jan 2018 08:32:30 -0500 (EST) Received: by mail-wr0-f198.google.com with SMTP id g13so1241591wrh.19 for ; Fri, 19 Jan 2018 05:32:30 -0800 (PST) Received: from mx2.suse.de (mx2.suse.de. [195.135.220.15]) by mx.google.com with ESMTPS id p6si7730338wrd.350.2018.01.19.05.32.28 for (version=TLS1 cipher=AES128-SHA bits=128/128); Fri, 19 Jan 2018 05:32:29 -0800 (PST) Date: Fri, 19 Jan 2018 14:32:27 +0100 From: Michal Hocko Subject: Re: [PATCH v5 1/2] mm/memcontrol.c: try harder to decrease [memory,memsw].limit_in_bytes Message-ID: <20180119133227.GC6584@dhcp22.suse.cz> References: <20171220102429.31601-1-aryabinin@virtuozzo.com> <20180119132544.19569-1-aryabinin@virtuozzo.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180119132544.19569-1-aryabinin@virtuozzo.com> Sender: owner-linux-mm@kvack.org List-ID: To: Andrey Ryabinin Cc: Andrew Morton , cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Shakeel Butt , Johannes Weiner , Vladimir Davydov On Fri 19-01-18 16:25:43, Andrey Ryabinin wrote: > mem_cgroup_resize_[memsw]_limit() tries to free only 32 (SWAP_CLUSTER_MAX) > pages on each iteration. This makes it 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 retrying the reclaim until the > desired limit is reached or fail if the reclaim doesn't make any progress > or a signal is pending. Thanks for splitting the original patch. I am OK with this part. > Signed-off-by: Andrey Ryabinin > Cc: Shakeel Butt > Cc: Michal Hocko > Cc: Johannes Weiner > Cc: Vladimir Davydov Acked-by: Michal Hocko > --- > mm/memcontrol.c | 42 ++++++------------------------------------ > 1 file changed, 6 insertions(+), 36 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 13aeccf32c2e..9d987f3e79dc 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,24 +2448,11 @@ static DEFINE_MUTEX(memcg_limit_mutex); > static int mem_cgroup_resize_limit(struct mem_cgroup *memcg, > unsigned long limit, bool memsw) > { > - unsigned long curusage; > - unsigned long oldusage; > bool enlarge = false; > - int retry_count; > int ret; > bool limits_invariant; > struct page_counter *counter = memsw ? &memcg->memsw : &memcg->memory; > > - /* > - * 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(counter); > - > do { > if (signal_pending(current)) { > ret = -EINTR; > @@ -2506,15 +2479,12 @@ static int mem_cgroup_resize_limit(struct mem_cgroup *memcg, > if (!ret) > break; > > - try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, !memsw); > - > - curusage = page_counter_read(counter); > - /* Usage is reduced ? */ > - if (curusage >= oldusage) > - retry_count--; > - else > - oldusage = curusage; > - } while (retry_count); > + if (!try_to_free_mem_cgroup_pages(memcg, 1, > + GFP_KERNEL, !memsw)) { > + ret = -EBUSY; > + break; > + } > + } while (true); > > if (!ret && enlarge) > memcg_oom_recover(memcg); > -- > 2.13.6 > > -- > To unsubscribe from this list: send the line "unsubscribe cgroups" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michal Hocko Subject: Re: [PATCH v5 1/2] mm/memcontrol.c: try harder to decrease [memory,memsw].limit_in_bytes Date: Fri, 19 Jan 2018 14:32:27 +0100 Message-ID: <20180119133227.GC6584@dhcp22.suse.cz> References: <20171220102429.31601-1-aryabinin@virtuozzo.com> <20180119132544.19569-1-aryabinin@virtuozzo.com> Mime-Version: 1.0 Return-path: Content-Disposition: inline In-Reply-To: <20180119132544.19569-1-aryabinin-5HdwGun5lf+gSpxsJD1C4w@public.gmane.org> Sender: cgroups-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-ID: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Andrey Ryabinin Cc: Andrew Morton , cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, Shakeel Butt , Johannes Weiner , Vladimir Davydov On Fri 19-01-18 16:25:43, Andrey Ryabinin wrote: > mem_cgroup_resize_[memsw]_limit() tries to free only 32 (SWAP_CLUSTER_MAX) > pages on each iteration. This makes it 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 retrying the reclaim until the > desired limit is reached or fail if the reclaim doesn't make any progress > or a signal is pending. Thanks for splitting the original patch. I am OK with this part. > Signed-off-by: Andrey Ryabinin > Cc: Shakeel Butt > Cc: Michal Hocko > Cc: Johannes Weiner > Cc: Vladimir Davydov Acked-by: Michal Hocko > --- > mm/memcontrol.c | 42 ++++++------------------------------------ > 1 file changed, 6 insertions(+), 36 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 13aeccf32c2e..9d987f3e79dc 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,24 +2448,11 @@ static DEFINE_MUTEX(memcg_limit_mutex); > static int mem_cgroup_resize_limit(struct mem_cgroup *memcg, > unsigned long limit, bool memsw) > { > - unsigned long curusage; > - unsigned long oldusage; > bool enlarge = false; > - int retry_count; > int ret; > bool limits_invariant; > struct page_counter *counter = memsw ? &memcg->memsw : &memcg->memory; > > - /* > - * 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(counter); > - > do { > if (signal_pending(current)) { > ret = -EINTR; > @@ -2506,15 +2479,12 @@ static int mem_cgroup_resize_limit(struct mem_cgroup *memcg, > if (!ret) > break; > > - try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, !memsw); > - > - curusage = page_counter_read(counter); > - /* Usage is reduced ? */ > - if (curusage >= oldusage) > - retry_count--; > - else > - oldusage = curusage; > - } while (retry_count); > + if (!try_to_free_mem_cgroup_pages(memcg, 1, > + GFP_KERNEL, !memsw)) { > + ret = -EBUSY; > + break; > + } > + } while (true); > > if (!ret && enlarge) > memcg_oom_recover(memcg); > -- > 2.13.6 > > -- > To unsubscribe from this list: send the line "unsubscribe cgroups" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Michal Hocko SUSE Labs