linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] memcg: fix kmemcg registration for late caches
@ 2013-02-11 16:39 Glauber Costa
  2013-02-12 14:56 ` Michal Hocko
  2013-02-13 10:00 ` Kamezawa Hiroyuki
  0 siblings, 2 replies; 3+ messages in thread
From: Glauber Costa @ 2013-02-11 16:39 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Michal Hocko, Johannes Weiner, kamezawa.hiroyu, linux-mm, cgroups,
	Glauber Costa

The designed workflow for the caches in kmemcg is: register it with
memcg_register_cache() if kmemcg is already available or later on when a
new kmemcg appears at memcg_update_cache_sizes() which will handle all
caches in the system. The caches created at boot time will be handled by
the later, and the memcg-caches as well as any system caches that are
registered later on by the former.

There is a bug, however, in memcg_register_cache: we correctly set up
the array size, but do not mark the cache as a root cache. This means
that allocations for any cache appearing late in the game will see
memcg->memcg_params->is_root_cache == false, and in particular, trigger
VM_BUG_ON(!cachep->memcg_params->is_root_cache) in
__memcg_kmem_cache_get.

The obvious fix is to include the missing assignment.

Signed-off-by: Glauber Costa <glommer@parallels.com>
---
 mm/memcontrol.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 03ebf68..d4e83d0 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3147,7 +3147,9 @@ int memcg_register_cache(struct mem_cgroup *memcg, struct kmem_cache *s,
 	if (memcg) {
 		s->memcg_params->memcg = memcg;
 		s->memcg_params->root_cache = root_cache;
-	}
+	} else
+		s->memcg_params->is_root_cache = true;
+
 	return 0;
 }
 
-- 
1.8.1.2

--
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>

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] memcg: fix kmemcg registration for late caches
  2013-02-11 16:39 [PATCH] memcg: fix kmemcg registration for late caches Glauber Costa
@ 2013-02-12 14:56 ` Michal Hocko
  2013-02-13 10:00 ` Kamezawa Hiroyuki
  1 sibling, 0 replies; 3+ messages in thread
From: Michal Hocko @ 2013-02-12 14:56 UTC (permalink / raw)
  To: Glauber Costa
  Cc: Andrew Morton, Johannes Weiner, kamezawa.hiroyu, linux-mm,
	cgroups

On Mon 11-02-13 20:39:57, Glauber Costa wrote:
> The designed workflow for the caches in kmemcg is: register it with
> memcg_register_cache() if kmemcg is already available or later on when a
> new kmemcg appears at memcg_update_cache_sizes() which will handle all
> caches in the system. The caches created at boot time will be handled by
> the later, and the memcg-caches as well as any system caches that are
> registered later on by the former.
> 
> There is a bug, however, in memcg_register_cache: we correctly set up
> the array size, but do not mark the cache as a root cache. This means
> that allocations for any cache appearing late in the game will see
> memcg->memcg_params->is_root_cache == false, and in particular, trigger
> VM_BUG_ON(!cachep->memcg_params->is_root_cache) in
> __memcg_kmem_cache_get.
> 
> The obvious fix is to include the missing assignment.
> 
> Signed-off-by: Glauber Costa <glommer@parallels.com>

Reviewed-by: Michal Hocko <mhocko@suse.cz>

> ---
>  mm/memcontrol.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 03ebf68..d4e83d0 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -3147,7 +3147,9 @@ int memcg_register_cache(struct mem_cgroup *memcg, struct kmem_cache *s,
>  	if (memcg) {
>  		s->memcg_params->memcg = memcg;
>  		s->memcg_params->root_cache = root_cache;
> -	}
> +	} else
> +		s->memcg_params->is_root_cache = true;
> +
>  	return 0;
>  }
>  
> -- 
> 1.8.1.2
> 
> --
> 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>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] memcg: fix kmemcg registration for late caches
  2013-02-11 16:39 [PATCH] memcg: fix kmemcg registration for late caches Glauber Costa
  2013-02-12 14:56 ` Michal Hocko
@ 2013-02-13 10:00 ` Kamezawa Hiroyuki
  1 sibling, 0 replies; 3+ messages in thread
From: Kamezawa Hiroyuki @ 2013-02-13 10:00 UTC (permalink / raw)
  To: Glauber Costa
  Cc: Andrew Morton, Michal Hocko, Johannes Weiner, linux-mm, cgroups

(2013/02/12 1:39), Glauber Costa wrote:
> The designed workflow for the caches in kmemcg is: register it with
> memcg_register_cache() if kmemcg is already available or later on when a
> new kmemcg appears at memcg_update_cache_sizes() which will handle all
> caches in the system. The caches created at boot time will be handled by
> the later, and the memcg-caches as well as any system caches that are
> registered later on by the former.
> 
> There is a bug, however, in memcg_register_cache: we correctly set up
> the array size, but do not mark the cache as a root cache. This means
> that allocations for any cache appearing late in the game will see
> memcg->memcg_params->is_root_cache == false, and in particular, trigger
> VM_BUG_ON(!cachep->memcg_params->is_root_cache) in
> __memcg_kmem_cache_get.
> 
> The obvious fix is to include the missing assignment.
> 
> Signed-off-by: Glauber Costa <glommer@parallels.com>

Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

> ---
>   mm/memcontrol.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 03ebf68..d4e83d0 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -3147,7 +3147,9 @@ int memcg_register_cache(struct mem_cgroup *memcg, struct kmem_cache *s,
>   	if (memcg) {
>   		s->memcg_params->memcg = memcg;
>   		s->memcg_params->root_cache = root_cache;
> -	}
> +	} else
> +		s->memcg_params->is_root_cache = true;
> +
>   	return 0;
>   }
>   
> 


--
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>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-02-13 10:00 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-11 16:39 [PATCH] memcg: fix kmemcg registration for late caches Glauber Costa
2013-02-12 14:56 ` Michal Hocko
2013-02-13 10:00 ` Kamezawa Hiroyuki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).