* [PATCH] memcg: stop warning on memcg_propagate_kmem
@ 2013-02-04 4:29 ` Hugh Dickins
0 siblings, 0 replies; 12+ messages in thread
From: Hugh Dickins @ 2013-02-04 4:29 UTC (permalink / raw)
To: Andrew Morton; +Cc: Glauber Costa, Michal Hocko, linux-kernel, linux-mm
Whilst I run the risk of a flogging for disloyalty to the Lord of Sealand,
I do have CONFIG_MEMCG=y CONFIG_MEMCG_KMEM not set, and grow tired of the
"mm/memcontrol.c:4972:12: warning: `memcg_propagate_kmem' defined but not
used [-Wunused-function]" seen in 3.8-rc: move the #ifdef outwards.
Signed-off-by: Hugh Dickins <hughd@google.com>
---
mm/memcontrol.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- 3.8-rc6/mm/memcontrol.c 2012-12-22 09:43:27.628015582 -0800
+++ linux/mm/memcontrol.c 2013-02-02 16:56:06.188325771 -0800
@@ -4969,6 +4969,7 @@ out:
return ret;
}
+#ifdef CONFIG_MEMCG_KMEM
static int memcg_propagate_kmem(struct mem_cgroup *memcg)
{
int ret = 0;
@@ -4977,7 +4978,6 @@ static int memcg_propagate_kmem(struct m
goto out;
memcg->kmem_account_flags = parent->kmem_account_flags;
-#ifdef CONFIG_MEMCG_KMEM
/*
* When that happen, we need to disable the static branch only on those
* memcgs that enabled it. To achieve this, we would be forced to
@@ -5003,10 +5003,10 @@ static int memcg_propagate_kmem(struct m
mutex_lock(&set_limit_mutex);
ret = memcg_update_cache_sizes(memcg);
mutex_unlock(&set_limit_mutex);
-#endif
out:
return ret;
}
+#endif /* CONFIG_MEMCG_KMEM */
/*
* The user of this function is...
--
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] 12+ messages in thread* [PATCH] memcg: stop warning on memcg_propagate_kmem
@ 2013-02-04 4:29 ` Hugh Dickins
0 siblings, 0 replies; 12+ messages in thread
From: Hugh Dickins @ 2013-02-04 4:29 UTC (permalink / raw)
To: Andrew Morton; +Cc: Glauber Costa, Michal Hocko, linux-kernel, linux-mm
Whilst I run the risk of a flogging for disloyalty to the Lord of Sealand,
I do have CONFIG_MEMCG=y CONFIG_MEMCG_KMEM not set, and grow tired of the
"mm/memcontrol.c:4972:12: warning: `memcg_propagate_kmem' defined but not
used [-Wunused-function]" seen in 3.8-rc: move the #ifdef outwards.
Signed-off-by: Hugh Dickins <hughd@google.com>
---
mm/memcontrol.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- 3.8-rc6/mm/memcontrol.c 2012-12-22 09:43:27.628015582 -0800
+++ linux/mm/memcontrol.c 2013-02-02 16:56:06.188325771 -0800
@@ -4969,6 +4969,7 @@ out:
return ret;
}
+#ifdef CONFIG_MEMCG_KMEM
static int memcg_propagate_kmem(struct mem_cgroup *memcg)
{
int ret = 0;
@@ -4977,7 +4978,6 @@ static int memcg_propagate_kmem(struct m
goto out;
memcg->kmem_account_flags = parent->kmem_account_flags;
-#ifdef CONFIG_MEMCG_KMEM
/*
* When that happen, we need to disable the static branch only on those
* memcgs that enabled it. To achieve this, we would be forced to
@@ -5003,10 +5003,10 @@ static int memcg_propagate_kmem(struct m
mutex_lock(&set_limit_mutex);
ret = memcg_update_cache_sizes(memcg);
mutex_unlock(&set_limit_mutex);
-#endif
out:
return ret;
}
+#endif /* CONFIG_MEMCG_KMEM */
/*
* The user of this function is...
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] memcg: stop warning on memcg_propagate_kmem
2013-02-04 4:29 ` Hugh Dickins
@ 2013-02-04 7:36 ` Lord Glauber Costa of Sealand
-1 siblings, 0 replies; 12+ messages in thread
From: Lord Glauber Costa of Sealand @ 2013-02-04 7:36 UTC (permalink / raw)
To: Hugh Dickins; +Cc: Andrew Morton, Michal Hocko, linux-kernel, linux-mm
On 02/04/2013 08:29 AM, Hugh Dickins wrote:
> Whilst I run the risk of a flogging for disloyalty to the Lord of Sealand,
> I do have CONFIG_MEMCG=y CONFIG_MEMCG_KMEM not set, and grow tired of the
> "mm/memcontrol.c:4972:12: warning: `memcg_propagate_kmem' defined but not
> used [-Wunused-function]" seen in 3.8-rc: move the #ifdef outwards.
>
Thanks my dear Hugh,
This is no disloyalty at all, and your braveness is indeed much appreciated.
My bad for letting that one slip
--
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] 12+ messages in thread
* Re: [PATCH] memcg: stop warning on memcg_propagate_kmem
@ 2013-02-04 7:36 ` Lord Glauber Costa of Sealand
0 siblings, 0 replies; 12+ messages in thread
From: Lord Glauber Costa of Sealand @ 2013-02-04 7:36 UTC (permalink / raw)
To: Hugh Dickins; +Cc: Andrew Morton, Michal Hocko, linux-kernel, linux-mm
On 02/04/2013 08:29 AM, Hugh Dickins wrote:
> Whilst I run the risk of a flogging for disloyalty to the Lord of Sealand,
> I do have CONFIG_MEMCG=y CONFIG_MEMCG_KMEM not set, and grow tired of the
> "mm/memcontrol.c:4972:12: warning: `memcg_propagate_kmem' defined but not
> used [-Wunused-function]" seen in 3.8-rc: move the #ifdef outwards.
>
Thanks my dear Hugh,
This is no disloyalty at all, and your braveness is indeed much appreciated.
My bad for letting that one slip
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] memcg: stop warning on memcg_propagate_kmem
2013-02-04 4:29 ` Hugh Dickins
@ 2013-02-04 7:57 ` Michal Hocko
-1 siblings, 0 replies; 12+ messages in thread
From: Michal Hocko @ 2013-02-04 7:57 UTC (permalink / raw)
To: Hugh Dickins; +Cc: Andrew Morton, Glauber Costa, linux-kernel, linux-mm
On Sun 03-02-13 20:29:01, Hugh Dickins wrote:
> Whilst I run the risk of a flogging for disloyalty to the Lord of Sealand,
> I do have CONFIG_MEMCG=y CONFIG_MEMCG_KMEM not set, and grow tired of the
> "mm/memcontrol.c:4972:12: warning: `memcg_propagate_kmem' defined but not
> used [-Wunused-function]" seen in 3.8-rc: move the #ifdef outwards.
>
> Signed-off-by: Hugh Dickins <hughd@google.com>
Acked-by: Michal Hocko <mhocko@suse.cz>
Hmm, if you are not too tired then moving the function downwards to
where it is called (memcg_init_kmem) will reduce the number of ifdefs.
But this can wait for a bigger clean up which is getting due:
git grep "def.*CONFIG_MEMCG_KMEM" mm/memcontrol.c | wc -l
12
Thanks
> ---
>
> mm/memcontrol.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> --- 3.8-rc6/mm/memcontrol.c 2012-12-22 09:43:27.628015582 -0800
> +++ linux/mm/memcontrol.c 2013-02-02 16:56:06.188325771 -0800
> @@ -4969,6 +4969,7 @@ out:
> return ret;
> }
>
> +#ifdef CONFIG_MEMCG_KMEM
> static int memcg_propagate_kmem(struct mem_cgroup *memcg)
> {
> int ret = 0;
> @@ -4977,7 +4978,6 @@ static int memcg_propagate_kmem(struct m
> goto out;
>
> memcg->kmem_account_flags = parent->kmem_account_flags;
> -#ifdef CONFIG_MEMCG_KMEM
> /*
> * When that happen, we need to disable the static branch only on those
> * memcgs that enabled it. To achieve this, we would be forced to
> @@ -5003,10 +5003,10 @@ static int memcg_propagate_kmem(struct m
> mutex_lock(&set_limit_mutex);
> ret = memcg_update_cache_sizes(memcg);
> mutex_unlock(&set_limit_mutex);
> -#endif
> out:
> return ret;
> }
> +#endif /* CONFIG_MEMCG_KMEM */
>
> /*
> * The user of this function is...
--
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] 12+ messages in thread* Re: [PATCH] memcg: stop warning on memcg_propagate_kmem
@ 2013-02-04 7:57 ` Michal Hocko
0 siblings, 0 replies; 12+ messages in thread
From: Michal Hocko @ 2013-02-04 7:57 UTC (permalink / raw)
To: Hugh Dickins; +Cc: Andrew Morton, Glauber Costa, linux-kernel, linux-mm
On Sun 03-02-13 20:29:01, Hugh Dickins wrote:
> Whilst I run the risk of a flogging for disloyalty to the Lord of Sealand,
> I do have CONFIG_MEMCG=y CONFIG_MEMCG_KMEM not set, and grow tired of the
> "mm/memcontrol.c:4972:12: warning: `memcg_propagate_kmem' defined but not
> used [-Wunused-function]" seen in 3.8-rc: move the #ifdef outwards.
>
> Signed-off-by: Hugh Dickins <hughd@google.com>
Acked-by: Michal Hocko <mhocko@suse.cz>
Hmm, if you are not too tired then moving the function downwards to
where it is called (memcg_init_kmem) will reduce the number of ifdefs.
But this can wait for a bigger clean up which is getting due:
git grep "def.*CONFIG_MEMCG_KMEM" mm/memcontrol.c | wc -l
12
Thanks
> ---
>
> mm/memcontrol.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> --- 3.8-rc6/mm/memcontrol.c 2012-12-22 09:43:27.628015582 -0800
> +++ linux/mm/memcontrol.c 2013-02-02 16:56:06.188325771 -0800
> @@ -4969,6 +4969,7 @@ out:
> return ret;
> }
>
> +#ifdef CONFIG_MEMCG_KMEM
> static int memcg_propagate_kmem(struct mem_cgroup *memcg)
> {
> int ret = 0;
> @@ -4977,7 +4978,6 @@ static int memcg_propagate_kmem(struct m
> goto out;
>
> memcg->kmem_account_flags = parent->kmem_account_flags;
> -#ifdef CONFIG_MEMCG_KMEM
> /*
> * When that happen, we need to disable the static branch only on those
> * memcgs that enabled it. To achieve this, we would be forced to
> @@ -5003,10 +5003,10 @@ static int memcg_propagate_kmem(struct m
> mutex_lock(&set_limit_mutex);
> ret = memcg_update_cache_sizes(memcg);
> mutex_unlock(&set_limit_mutex);
> -#endif
> out:
> return ret;
> }
> +#endif /* CONFIG_MEMCG_KMEM */
>
> /*
> * The user of this function is...
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] memcg: stop warning on memcg_propagate_kmem
2013-02-04 7:57 ` Michal Hocko
@ 2013-02-04 8:04 ` Lord Glauber Costa of Sealand
-1 siblings, 0 replies; 12+ messages in thread
From: Lord Glauber Costa of Sealand @ 2013-02-04 8:04 UTC (permalink / raw)
To: Michal Hocko; +Cc: Hugh Dickins, Andrew Morton, linux-kernel, linux-mm
On 02/04/2013 11:57 AM, Michal Hocko wrote:
> On Sun 03-02-13 20:29:01, Hugh Dickins wrote:
>> Whilst I run the risk of a flogging for disloyalty to the Lord of Sealand,
>> I do have CONFIG_MEMCG=y CONFIG_MEMCG_KMEM not set, and grow tired of the
>> "mm/memcontrol.c:4972:12: warning: `memcg_propagate_kmem' defined but not
>> used [-Wunused-function]" seen in 3.8-rc: move the #ifdef outwards.
>>
>> Signed-off-by: Hugh Dickins <hughd@google.com>
>
> Acked-by: Michal Hocko <mhocko@suse.cz>
>
> Hmm, if you are not too tired then moving the function downwards to
> where it is called (memcg_init_kmem) will reduce the number of ifdefs.
> But this can wait for a bigger clean up which is getting due:
> git grep "def.*CONFIG_MEMCG_KMEM" mm/memcontrol.c | wc -l
> 12
>
The problem is that I was usually keeping things in clearly separated
blocks, like this :
#if defined(CONFIG_MEMCG_KMEM) && defined(CONFIG_INET)
struct tcp_memcontrol tcp_mem;
#endif
#if defined(CONFIG_MEMCG_KMEM)
/* analogous to slab_common's slab_caches list. per-memcg */
struct list_head memcg_slab_caches;
/* Not a spinlock, we can take a lot of time walking the list */
struct mutex slab_caches_mutex;
/* Index in the kmem_cache->memcg_params->memcg_caches array */
int kmemcg_id;
#endif
If it would be preferable to everybody, this could be easily rewritten as:
#if defined(CONFIG_MEMCG_KMEM)
#if defined(CONFIG_INET)
struct tcp_memcontrol tcp_mem;
#endif
/* analogous to slab_common's slab_caches list. per-memcg */
struct list_head memcg_slab_caches;
/* Not a spinlock, we can take a lot of time walking the list */
struct mutex slab_caches_mutex;
/* Index in the kmem_cache->memcg_params->memcg_caches array */
int kmemcg_id;
#endif
This would allow us to collapse some blocks a bit down as well.
It doesn't bother me *that* much, though.
> Thanks
>> ---
>>
>> mm/memcontrol.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> --- 3.8-rc6/mm/memcontrol.c 2012-12-22 09:43:27.628015582 -0800
>> +++ linux/mm/memcontrol.c 2013-02-02 16:56:06.188325771 -0800
>> @@ -4969,6 +4969,7 @@ out:
>> return ret;
>> }
>>
>> +#ifdef CONFIG_MEMCG_KMEM
>> static int memcg_propagate_kmem(struct mem_cgroup *memcg)
>> {
>> int ret = 0;
>> @@ -4977,7 +4978,6 @@ static int memcg_propagate_kmem(struct m
>> goto out;
>>
>> memcg->kmem_account_flags = parent->kmem_account_flags;
>> -#ifdef CONFIG_MEMCG_KMEM
>> /*
>> * When that happen, we need to disable the static branch only on those
>> * memcgs that enabled it. To achieve this, we would be forced to
>> @@ -5003,10 +5003,10 @@ static int memcg_propagate_kmem(struct m
>> mutex_lock(&set_limit_mutex);
>> ret = memcg_update_cache_sizes(memcg);
>> mutex_unlock(&set_limit_mutex);
>> -#endif
>> out:
>> return ret;
>> }
>> +#endif /* CONFIG_MEMCG_KMEM */
>>
>> /*
>> * The user of this function is...
>
--
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] 12+ messages in thread* Re: [PATCH] memcg: stop warning on memcg_propagate_kmem
@ 2013-02-04 8:04 ` Lord Glauber Costa of Sealand
0 siblings, 0 replies; 12+ messages in thread
From: Lord Glauber Costa of Sealand @ 2013-02-04 8:04 UTC (permalink / raw)
To: Michal Hocko; +Cc: Hugh Dickins, Andrew Morton, linux-kernel, linux-mm
On 02/04/2013 11:57 AM, Michal Hocko wrote:
> On Sun 03-02-13 20:29:01, Hugh Dickins wrote:
>> Whilst I run the risk of a flogging for disloyalty to the Lord of Sealand,
>> I do have CONFIG_MEMCG=y CONFIG_MEMCG_KMEM not set, and grow tired of the
>> "mm/memcontrol.c:4972:12: warning: `memcg_propagate_kmem' defined but not
>> used [-Wunused-function]" seen in 3.8-rc: move the #ifdef outwards.
>>
>> Signed-off-by: Hugh Dickins <hughd@google.com>
>
> Acked-by: Michal Hocko <mhocko@suse.cz>
>
> Hmm, if you are not too tired then moving the function downwards to
> where it is called (memcg_init_kmem) will reduce the number of ifdefs.
> But this can wait for a bigger clean up which is getting due:
> git grep "def.*CONFIG_MEMCG_KMEM" mm/memcontrol.c | wc -l
> 12
>
The problem is that I was usually keeping things in clearly separated
blocks, like this :
#if defined(CONFIG_MEMCG_KMEM) && defined(CONFIG_INET)
struct tcp_memcontrol tcp_mem;
#endif
#if defined(CONFIG_MEMCG_KMEM)
/* analogous to slab_common's slab_caches list. per-memcg */
struct list_head memcg_slab_caches;
/* Not a spinlock, we can take a lot of time walking the list */
struct mutex slab_caches_mutex;
/* Index in the kmem_cache->memcg_params->memcg_caches array */
int kmemcg_id;
#endif
If it would be preferable to everybody, this could be easily rewritten as:
#if defined(CONFIG_MEMCG_KMEM)
#if defined(CONFIG_INET)
struct tcp_memcontrol tcp_mem;
#endif
/* analogous to slab_common's slab_caches list. per-memcg */
struct list_head memcg_slab_caches;
/* Not a spinlock, we can take a lot of time walking the list */
struct mutex slab_caches_mutex;
/* Index in the kmem_cache->memcg_params->memcg_caches array */
int kmemcg_id;
#endif
This would allow us to collapse some blocks a bit down as well.
It doesn't bother me *that* much, though.
> Thanks
>> ---
>>
>> mm/memcontrol.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> --- 3.8-rc6/mm/memcontrol.c 2012-12-22 09:43:27.628015582 -0800
>> +++ linux/mm/memcontrol.c 2013-02-02 16:56:06.188325771 -0800
>> @@ -4969,6 +4969,7 @@ out:
>> return ret;
>> }
>>
>> +#ifdef CONFIG_MEMCG_KMEM
>> static int memcg_propagate_kmem(struct mem_cgroup *memcg)
>> {
>> int ret = 0;
>> @@ -4977,7 +4978,6 @@ static int memcg_propagate_kmem(struct m
>> goto out;
>>
>> memcg->kmem_account_flags = parent->kmem_account_flags;
>> -#ifdef CONFIG_MEMCG_KMEM
>> /*
>> * When that happen, we need to disable the static branch only on those
>> * memcgs that enabled it. To achieve this, we would be forced to
>> @@ -5003,10 +5003,10 @@ static int memcg_propagate_kmem(struct m
>> mutex_lock(&set_limit_mutex);
>> ret = memcg_update_cache_sizes(memcg);
>> mutex_unlock(&set_limit_mutex);
>> -#endif
>> out:
>> return ret;
>> }
>> +#endif /* CONFIG_MEMCG_KMEM */
>>
>> /*
>> * The user of this function is...
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] memcg: stop warning on memcg_propagate_kmem
2013-02-04 8:04 ` Lord Glauber Costa of Sealand
@ 2013-02-04 8:36 ` Michal Hocko
-1 siblings, 0 replies; 12+ messages in thread
From: Michal Hocko @ 2013-02-04 8:36 UTC (permalink / raw)
To: Lord Glauber Costa of Sealand
Cc: Hugh Dickins, Andrew Morton, linux-kernel, linux-mm
On Mon 04-02-13 12:04:06, Glauber Costa wrote:
> On 02/04/2013 11:57 AM, Michal Hocko wrote:
> > On Sun 03-02-13 20:29:01, Hugh Dickins wrote:
> >> Whilst I run the risk of a flogging for disloyalty to the Lord of Sealand,
> >> I do have CONFIG_MEMCG=y CONFIG_MEMCG_KMEM not set, and grow tired of the
> >> "mm/memcontrol.c:4972:12: warning: `memcg_propagate_kmem' defined but not
> >> used [-Wunused-function]" seen in 3.8-rc: move the #ifdef outwards.
> >>
> >> Signed-off-by: Hugh Dickins <hughd@google.com>
> >
> > Acked-by: Michal Hocko <mhocko@suse.cz>
> >
> > Hmm, if you are not too tired then moving the function downwards to
> > where it is called (memcg_init_kmem) will reduce the number of ifdefs.
> > But this can wait for a bigger clean up which is getting due:
> > git grep "def.*CONFIG_MEMCG_KMEM" mm/memcontrol.c | wc -l
> > 12
> >
>
> The problem is that I was usually keeping things in clearly separated
> blocks, like this :
>
> #if defined(CONFIG_MEMCG_KMEM) && defined(CONFIG_INET)
> struct tcp_memcontrol tcp_mem;
> #endif
> #if defined(CONFIG_MEMCG_KMEM)
> /* analogous to slab_common's slab_caches list. per-memcg */
> struct list_head memcg_slab_caches;
> /* Not a spinlock, we can take a lot of time walking the list */
> struct mutex slab_caches_mutex;
> /* Index in the kmem_cache->memcg_params->memcg_caches array */
> int kmemcg_id;
> #endif
>
> If it would be preferable to everybody, this could be easily rewritten as:
>
> #if defined(CONFIG_MEMCG_KMEM)
> #if defined(CONFIG_INET)
> struct tcp_memcontrol tcp_mem;
> #endif
> /* analogous to slab_common's slab_caches list. per-memcg */
> struct list_head memcg_slab_caches;
> /* Not a spinlock, we can take a lot of time walking the list */
> struct mutex slab_caches_mutex;
> /* Index in the kmem_cache->memcg_params->memcg_caches array */
> int kmemcg_id;
> #endif
I was rather interested in reducing CONFIG_KMEM block, the above example
doesn't bother me that much.
> This would allow us to collapse some blocks a bit down as well.
>
> It doesn't bother me *that* much, though.
Yes and a quick attempt shows that a clean up would bring a lot of
churn.
--
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] 12+ messages in thread
* Re: [PATCH] memcg: stop warning on memcg_propagate_kmem
@ 2013-02-04 8:36 ` Michal Hocko
0 siblings, 0 replies; 12+ messages in thread
From: Michal Hocko @ 2013-02-04 8:36 UTC (permalink / raw)
To: Lord Glauber Costa of Sealand
Cc: Hugh Dickins, Andrew Morton, linux-kernel, linux-mm
On Mon 04-02-13 12:04:06, Glauber Costa wrote:
> On 02/04/2013 11:57 AM, Michal Hocko wrote:
> > On Sun 03-02-13 20:29:01, Hugh Dickins wrote:
> >> Whilst I run the risk of a flogging for disloyalty to the Lord of Sealand,
> >> I do have CONFIG_MEMCG=y CONFIG_MEMCG_KMEM not set, and grow tired of the
> >> "mm/memcontrol.c:4972:12: warning: `memcg_propagate_kmem' defined but not
> >> used [-Wunused-function]" seen in 3.8-rc: move the #ifdef outwards.
> >>
> >> Signed-off-by: Hugh Dickins <hughd@google.com>
> >
> > Acked-by: Michal Hocko <mhocko@suse.cz>
> >
> > Hmm, if you are not too tired then moving the function downwards to
> > where it is called (memcg_init_kmem) will reduce the number of ifdefs.
> > But this can wait for a bigger clean up which is getting due:
> > git grep "def.*CONFIG_MEMCG_KMEM" mm/memcontrol.c | wc -l
> > 12
> >
>
> The problem is that I was usually keeping things in clearly separated
> blocks, like this :
>
> #if defined(CONFIG_MEMCG_KMEM) && defined(CONFIG_INET)
> struct tcp_memcontrol tcp_mem;
> #endif
> #if defined(CONFIG_MEMCG_KMEM)
> /* analogous to slab_common's slab_caches list. per-memcg */
> struct list_head memcg_slab_caches;
> /* Not a spinlock, we can take a lot of time walking the list */
> struct mutex slab_caches_mutex;
> /* Index in the kmem_cache->memcg_params->memcg_caches array */
> int kmemcg_id;
> #endif
>
> If it would be preferable to everybody, this could be easily rewritten as:
>
> #if defined(CONFIG_MEMCG_KMEM)
> #if defined(CONFIG_INET)
> struct tcp_memcontrol tcp_mem;
> #endif
> /* analogous to slab_common's slab_caches list. per-memcg */
> struct list_head memcg_slab_caches;
> /* Not a spinlock, we can take a lot of time walking the list */
> struct mutex slab_caches_mutex;
> /* Index in the kmem_cache->memcg_params->memcg_caches array */
> int kmemcg_id;
> #endif
I was rather interested in reducing CONFIG_KMEM block, the above example
doesn't bother me that much.
> This would allow us to collapse some blocks a bit down as well.
>
> It doesn't bother me *that* much, though.
Yes and a quick attempt shows that a clean up would bring a lot of
churn.
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] memcg: stop warning on memcg_propagate_kmem
2013-02-04 8:36 ` Michal Hocko
@ 2013-02-04 8:40 ` Lord Glauber Costa of Sealand
-1 siblings, 0 replies; 12+ messages in thread
From: Lord Glauber Costa of Sealand @ 2013-02-04 8:40 UTC (permalink / raw)
To: Michal Hocko; +Cc: Hugh Dickins, Andrew Morton, linux-kernel, linux-mm
On 02/04/2013 12:36 PM, Michal Hocko wrote:
> On Mon 04-02-13 12:04:06, Glauber Costa wrote:
>> On 02/04/2013 11:57 AM, Michal Hocko wrote:
>>> On Sun 03-02-13 20:29:01, Hugh Dickins wrote:
>>>> Whilst I run the risk of a flogging for disloyalty to the Lord of Sealand,
>>>> I do have CONFIG_MEMCG=y CONFIG_MEMCG_KMEM not set, and grow tired of the
>>>> "mm/memcontrol.c:4972:12: warning: `memcg_propagate_kmem' defined but not
>>>> used [-Wunused-function]" seen in 3.8-rc: move the #ifdef outwards.
>>>>
>>>> Signed-off-by: Hugh Dickins <hughd@google.com>
>>>
>>> Acked-by: Michal Hocko <mhocko@suse.cz>
>>>
>>> Hmm, if you are not too tired then moving the function downwards to
>>> where it is called (memcg_init_kmem) will reduce the number of ifdefs.
>>> But this can wait for a bigger clean up which is getting due:
>>> git grep "def.*CONFIG_MEMCG_KMEM" mm/memcontrol.c | wc -l
>>> 12
>>>
>>
>> The problem is that I was usually keeping things in clearly separated
>> blocks, like this :
>>
>> #if defined(CONFIG_MEMCG_KMEM) && defined(CONFIG_INET)
>> struct tcp_memcontrol tcp_mem;
>> #endif
>> #if defined(CONFIG_MEMCG_KMEM)
>> /* analogous to slab_common's slab_caches list. per-memcg */
>> struct list_head memcg_slab_caches;
>> /* Not a spinlock, we can take a lot of time walking the list */
>> struct mutex slab_caches_mutex;
>> /* Index in the kmem_cache->memcg_params->memcg_caches array */
>> int kmemcg_id;
>> #endif
>>
>> If it would be preferable to everybody, this could be easily rewritten as:
>>
>> #if defined(CONFIG_MEMCG_KMEM)
>> #if defined(CONFIG_INET)
>> struct tcp_memcontrol tcp_mem;
>> #endif
>> /* analogous to slab_common's slab_caches list. per-memcg */
>> struct list_head memcg_slab_caches;
>> /* Not a spinlock, we can take a lot of time walking the list */
>> struct mutex slab_caches_mutex;
>> /* Index in the kmem_cache->memcg_params->memcg_caches array */
>> int kmemcg_id;
>> #endif
>
> I was rather interested in reducing CONFIG_KMEM block, the above example
> doesn't bother me that much.
>
>> This would allow us to collapse some blocks a bit down as well.
>>
>> It doesn't bother me *that* much, though.
>
> Yes and a quick attempt shows that a clean up would bring a lot of
> churn.
>
And some of it, because there are circular dependencies. So we would
have to start adding forward declarations here and there to make it all
work. That is part of the reason why I kept the blocks separate.
--
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] 12+ messages in thread
* Re: [PATCH] memcg: stop warning on memcg_propagate_kmem
@ 2013-02-04 8:40 ` Lord Glauber Costa of Sealand
0 siblings, 0 replies; 12+ messages in thread
From: Lord Glauber Costa of Sealand @ 2013-02-04 8:40 UTC (permalink / raw)
To: Michal Hocko; +Cc: Hugh Dickins, Andrew Morton, linux-kernel, linux-mm
On 02/04/2013 12:36 PM, Michal Hocko wrote:
> On Mon 04-02-13 12:04:06, Glauber Costa wrote:
>> On 02/04/2013 11:57 AM, Michal Hocko wrote:
>>> On Sun 03-02-13 20:29:01, Hugh Dickins wrote:
>>>> Whilst I run the risk of a flogging for disloyalty to the Lord of Sealand,
>>>> I do have CONFIG_MEMCG=y CONFIG_MEMCG_KMEM not set, and grow tired of the
>>>> "mm/memcontrol.c:4972:12: warning: `memcg_propagate_kmem' defined but not
>>>> used [-Wunused-function]" seen in 3.8-rc: move the #ifdef outwards.
>>>>
>>>> Signed-off-by: Hugh Dickins <hughd@google.com>
>>>
>>> Acked-by: Michal Hocko <mhocko@suse.cz>
>>>
>>> Hmm, if you are not too tired then moving the function downwards to
>>> where it is called (memcg_init_kmem) will reduce the number of ifdefs.
>>> But this can wait for a bigger clean up which is getting due:
>>> git grep "def.*CONFIG_MEMCG_KMEM" mm/memcontrol.c | wc -l
>>> 12
>>>
>>
>> The problem is that I was usually keeping things in clearly separated
>> blocks, like this :
>>
>> #if defined(CONFIG_MEMCG_KMEM) && defined(CONFIG_INET)
>> struct tcp_memcontrol tcp_mem;
>> #endif
>> #if defined(CONFIG_MEMCG_KMEM)
>> /* analogous to slab_common's slab_caches list. per-memcg */
>> struct list_head memcg_slab_caches;
>> /* Not a spinlock, we can take a lot of time walking the list */
>> struct mutex slab_caches_mutex;
>> /* Index in the kmem_cache->memcg_params->memcg_caches array */
>> int kmemcg_id;
>> #endif
>>
>> If it would be preferable to everybody, this could be easily rewritten as:
>>
>> #if defined(CONFIG_MEMCG_KMEM)
>> #if defined(CONFIG_INET)
>> struct tcp_memcontrol tcp_mem;
>> #endif
>> /* analogous to slab_common's slab_caches list. per-memcg */
>> struct list_head memcg_slab_caches;
>> /* Not a spinlock, we can take a lot of time walking the list */
>> struct mutex slab_caches_mutex;
>> /* Index in the kmem_cache->memcg_params->memcg_caches array */
>> int kmemcg_id;
>> #endif
>
> I was rather interested in reducing CONFIG_KMEM block, the above example
> doesn't bother me that much.
>
>> This would allow us to collapse some blocks a bit down as well.
>>
>> It doesn't bother me *that* much, though.
>
> Yes and a quick attempt shows that a clean up would bring a lot of
> churn.
>
And some of it, because there are circular dependencies. So we would
have to start adding forward declarations here and there to make it all
work. That is part of the reason why I kept the blocks separate.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-02-04 8:39 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-04 4:29 [PATCH] memcg: stop warning on memcg_propagate_kmem Hugh Dickins
2013-02-04 4:29 ` Hugh Dickins
2013-02-04 7:36 ` Lord Glauber Costa of Sealand
2013-02-04 7:36 ` Lord Glauber Costa of Sealand
2013-02-04 7:57 ` Michal Hocko
2013-02-04 7:57 ` Michal Hocko
2013-02-04 8:04 ` Lord Glauber Costa of Sealand
2013-02-04 8:04 ` Lord Glauber Costa of Sealand
2013-02-04 8:36 ` Michal Hocko
2013-02-04 8:36 ` Michal Hocko
2013-02-04 8:40 ` Lord Glauber Costa of Sealand
2013-02-04 8:40 ` Lord Glauber Costa of Sealand
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.