- Do not take memcg_limit_mutex for resetting limits - the cgroup
cannot be altered from userspace anymore, so no need to protect them.
- Use plain page_counter_limit() for resetting ->memory and ->memsw
limits instead of mem_cgrouop_resize_* helpers - we enlarge the
limits, so no need in special handling.
- Reset ->swap and ->tcpmem limits as well.
Signed-off-by: Vladimir Davydov <[email protected]>
---
mm/memcontrol.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index ae8b81c55685..8615b066b642 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4257,9 +4257,11 @@ static void mem_cgroup_css_reset(struct cgroup_subsys_state *css)
{
struct mem_cgroup *memcg = mem_cgroup_from_css(css);
- mem_cgroup_resize_limit(memcg, PAGE_COUNTER_MAX);
- mem_cgroup_resize_memsw_limit(memcg, PAGE_COUNTER_MAX);
- memcg_update_kmem_limit(memcg, PAGE_COUNTER_MAX);
+ page_counter_limit(&memcg->memory, PAGE_COUNTER_MAX);
+ page_counter_limit(&memcg->swap, PAGE_COUNTER_MAX);
+ page_counter_limit(&memcg->memsw, PAGE_COUNTER_MAX);
+ page_counter_limit(&memcg->kmem, PAGE_COUNTER_MAX);
+ page_counter_limit(&memcg->tcpmem, PAGE_COUNTER_MAX);
memcg->low = 0;
memcg->high = PAGE_COUNTER_MAX;
memcg->soft_limit = PAGE_COUNTER_MAX;
--
2.1.4
An associated css can be around for quite a while after a cgroup
directory has been removed. In general, it makes sense to reset it to
defaults so as not to worry about any remnants. For instance, memory
cgroup needs to reset memory.low, otherwise pages charged to a dead
cgroup might never get reclaimed. There's ->css_reset callback, which
would fit perfectly for the purpose. Currently, it's only called when a
subsystem is disabled in the unified hierarchy and there are other
subsystems dependant on it. Let's call it on css destruction as well.
Suggested-by: Johannes Weiner <[email protected]>
Signed-off-by: Vladimir Davydov <[email protected]>
---
kernel/cgroup.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index cc40463e7b69..2ef78912c996 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -5138,6 +5138,8 @@ static void kill_css(struct cgroup_subsys_state *css)
* See seq_css() for details.
*/
css_clear_dir(css, NULL);
+ if (css->ss->css_reset)
+ css->ss->css_reset(css);
/*
* Killing would put the base ref, but we need to keep it alive
--
2.1.4
On Tue 01-03-16 14:13:12, Vladimir Davydov wrote:
> - Do not take memcg_limit_mutex for resetting limits - the cgroup
> cannot be altered from userspace anymore, so no need to protect them.
>
> - Use plain page_counter_limit() for resetting ->memory and ->memsw
> limits instead of mem_cgrouop_resize_* helpers - we enlarge the
> limits, so no need in special handling.
>
> - Reset ->swap and ->tcpmem limits as well.
>
> Signed-off-by: Vladimir Davydov <[email protected]>
Acked-by: Michal Hocko <[email protected]>
> ---
> mm/memcontrol.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index ae8b81c55685..8615b066b642 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -4257,9 +4257,11 @@ static void mem_cgroup_css_reset(struct cgroup_subsys_state *css)
> {
> struct mem_cgroup *memcg = mem_cgroup_from_css(css);
>
> - mem_cgroup_resize_limit(memcg, PAGE_COUNTER_MAX);
> - mem_cgroup_resize_memsw_limit(memcg, PAGE_COUNTER_MAX);
> - memcg_update_kmem_limit(memcg, PAGE_COUNTER_MAX);
> + page_counter_limit(&memcg->memory, PAGE_COUNTER_MAX);
> + page_counter_limit(&memcg->swap, PAGE_COUNTER_MAX);
> + page_counter_limit(&memcg->memsw, PAGE_COUNTER_MAX);
> + page_counter_limit(&memcg->kmem, PAGE_COUNTER_MAX);
> + page_counter_limit(&memcg->tcpmem, PAGE_COUNTER_MAX);
> memcg->low = 0;
> memcg->high = PAGE_COUNTER_MAX;
> memcg->soft_limit = PAGE_COUNTER_MAX;
> --
> 2.1.4
--
Michal Hocko
SUSE Labs
On Tue, Mar 01, 2016 at 02:13:13PM +0300, Vladimir Davydov wrote:
> @@ -5138,6 +5138,8 @@ static void kill_css(struct cgroup_subsys_state *css)
> * See seq_css() for details.
> */
> css_clear_dir(css, NULL);
> + if (css->ss->css_reset)
> + css->ss->css_reset(css);
I think the better spot for this is in offline_css() right before
->css_offline() is called.
Thanks.
--
tejun
On Tue, Mar 01, 2016 at 11:30:18AM -0500, Tejun Heo wrote:
> On Tue, Mar 01, 2016 at 02:13:13PM +0300, Vladimir Davydov wrote:
> > @@ -5138,6 +5138,8 @@ static void kill_css(struct cgroup_subsys_state *css)
> > * See seq_css() for details.
> > */
> > css_clear_dir(css, NULL);
> > + if (css->ss->css_reset)
> > + css->ss->css_reset(css);
>
> I think the better spot for this is in offline_css() right before
> ->css_offline() is called.
Okay, here it goes.
---
From: Vladimir Davydov <[email protected]>
Subject: [PATCH] cgroup: reset css on destruction
An associated css can be around for quite a while after a cgroup
directory has been removed. In general, it makes sense to reset it to
defaults so as not to worry about any remnants. For instance, memory
cgroup needs to reset memory.low, otherwise pages charged to a dead
cgroup might never get reclaimed. There's ->css_reset callback, which
would fit perfectly for the purpose. Currently, it's only called when a
subsystem is disabled in the unified hierarchy and there are other
subsystems dependant on it. Let's call it on css destruction as well.
Suggested-by: Johannes Weiner <[email protected]>
Signed-off-by: Vladimir Davydov <[email protected]>
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index cc40463e7b69..bb1900b70b01 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -4876,6 +4876,9 @@ static void offline_css(struct cgroup_subsys_state *css)
if (!(css->flags & CSS_ONLINE))
return;
+ if (ss->css_reset)
+ ss->css_reset(css);
+
if (ss->css_offline)
ss->css_offline(css);
On Tue, Mar 01, 2016 at 07:56:30PM +0300, Vladimir Davydov wrote:
> From: Vladimir Davydov <[email protected]>
> Subject: [PATCH] cgroup: reset css on destruction
>
> An associated css can be around for quite a while after a cgroup
> directory has been removed. In general, it makes sense to reset it to
> defaults so as not to worry about any remnants. For instance, memory
> cgroup needs to reset memory.low, otherwise pages charged to a dead
> cgroup might never get reclaimed. There's ->css_reset callback, which
> would fit perfectly for the purpose. Currently, it's only called when a
> subsystem is disabled in the unified hierarchy and there are other
> subsystems dependant on it. Let's call it on css destruction as well.
>
> Suggested-by: Johannes Weiner <[email protected]>
> Signed-off-by: Vladimir Davydov <[email protected]>
Applied to cgroup/for-4.6. Thanks.
--
tejun
On Tue, Mar 01, 2016 at 02:13:12PM +0300, Vladimir Davydov wrote:
> - Do not take memcg_limit_mutex for resetting limits - the cgroup
> cannot be altered from userspace anymore, so no need to protect them.
>
> - Use plain page_counter_limit() for resetting ->memory and ->memsw
> limits instead of mem_cgrouop_resize_* helpers - we enlarge the
> limits, so no need in special handling.
>
> - Reset ->swap and ->tcpmem limits as well.
>
> Signed-off-by: Vladimir Davydov <[email protected]>
Acked-by: Johannes Weiner <[email protected]>
On Tue, Mar 01, 2016 at 07:56:30PM +0300, Vladimir Davydov wrote:
> From: Vladimir Davydov <[email protected]>
> Subject: [PATCH] cgroup: reset css on destruction
>
> An associated css can be around for quite a while after a cgroup
> directory has been removed. In general, it makes sense to reset it to
> defaults so as not to worry about any remnants. For instance, memory
> cgroup needs to reset memory.low, otherwise pages charged to a dead
> cgroup might never get reclaimed. There's ->css_reset callback, which
> would fit perfectly for the purpose. Currently, it's only called when a
> subsystem is disabled in the unified hierarchy and there are other
> subsystems dependant on it. Let's call it on css destruction as well.
>
> Suggested-by: Johannes Weiner <[email protected]>
> Signed-off-by: Vladimir Davydov <[email protected]>
It's already in a git tree, but FWIW
Acked-by: Johannes Weiner <[email protected]>