linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] memcg remove warning at DEBUG_VM=off
@ 2009-04-08  5:20 KAMEZAWA Hiroyuki
  2009-04-08  5:27 ` Balbir Singh
  0 siblings, 1 reply; 7+ messages in thread
From: KAMEZAWA Hiroyuki @ 2009-04-08  5:20 UTC (permalink / raw)
  To: linux-kernel@vger.kernel.org
  Cc: linux-mm@kvack.org, balbir@linux.vnet.ibm.com,
	nishimura@mxp.nes.nec.co.jp, akpm@linux-foundation.org

From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
This is against 2.6.30-rc1. (maybe no problem against mmotm.)

==
Fix warning as

  CC      mm/memcontrol.o
mm/memcontrol.c:318: warning: ‘mem_cgroup_is_obsolete’ defined but not used

This is called only from VM_BUG_ON().

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
Index: linux-2.6.30-rc1/mm/memcontrol.c
===================================================================
--- linux-2.6.30-rc1.orig/mm/memcontrol.c
+++ linux-2.6.30-rc1/mm/memcontrol.c
@@ -314,13 +314,14 @@ static struct mem_cgroup *try_get_mem_cg
 	return mem;
 }
 
+#ifdef CONFIG_DEBUG_VM
 static bool mem_cgroup_is_obsolete(struct mem_cgroup *mem)
 {
 	if (!mem)
 		return true;
 	return css_is_removed(&mem->css);
 }
-
+#endif
 
 /*
  * Call callback function against all cgroup under hierarchy tree.

--
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] 7+ messages in thread

* Re: [PATCH] memcg remove warning at DEBUG_VM=off
  2009-04-08  5:20 [PATCH] memcg remove warning at DEBUG_VM=off KAMEZAWA Hiroyuki
@ 2009-04-08  5:27 ` Balbir Singh
  2009-04-10  5:25   ` Andrew Morton
  0 siblings, 1 reply; 7+ messages in thread
From: Balbir Singh @ 2009-04-08  5:27 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	nishimura@mxp.nes.nec.co.jp, akpm@linux-foundation.org

* KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2009-04-08 14:20:42]:

> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> This is against 2.6.30-rc1. (maybe no problem against mmotm.)
> 
> ==
> Fix warning as
> 
>   CC      mm/memcontrol.o
> mm/memcontrol.c:318: warning: ?$B!Fmem_cgroup_is_obsolete?$B!G defined but not used
> 
> This is called only from VM_BUG_ON().
> 
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> ---
> Index: linux-2.6.30-rc1/mm/memcontrol.c
> ===================================================================
> --- linux-2.6.30-rc1.orig/mm/memcontrol.c
> +++ linux-2.6.30-rc1/mm/memcontrol.c
> @@ -314,13 +314,14 @@ static struct mem_cgroup *try_get_mem_cg
>  	return mem;
>  }
> 
> +#ifdef CONFIG_DEBUG_VM
>  static bool mem_cgroup_is_obsolete(struct mem_cgroup *mem)
>  {
>  	if (!mem)
>  		return true;
>  	return css_is_removed(&mem->css);
>  }
> -
> +#endif

Can we change the code to use

        VM_BUG_ON(!mem || css_is_removed(&mem->css));

Reviewed-by: Balbir Singh <balbir@linux.vnet.ibm.com>
 

-- 
	Balbir

--
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] 7+ messages in thread

* Re: [PATCH] memcg remove warning at DEBUG_VM=off
  2009-04-08  5:27 ` Balbir Singh
@ 2009-04-10  5:25   ` Andrew Morton
  2009-04-10  6:33     ` KAMEZAWA Hiroyuki
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Morton @ 2009-04-10  5:25 UTC (permalink / raw)
  To: balbir
  Cc: KAMEZAWA Hiroyuki, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, nishimura@mxp.nes.nec.co.jp

On Wed, 8 Apr 2009 10:57:15 +0530 Balbir Singh <balbir@linux.vnet.ibm.com> wrote:

> * KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2009-04-08 14:20:42]:
> 
> > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > This is against 2.6.30-rc1. (maybe no problem against mmotm.)
> > 
> > ==
> > Fix warning as
> > 
> >   CC      mm/memcontrol.o
> > mm/memcontrol.c:318: warning: ?$B!Fmem_cgroup_is_obsolete?$B!G defined but not used
> > 
> > This is called only from VM_BUG_ON().
> > 
> > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > ---
> > Index: linux-2.6.30-rc1/mm/memcontrol.c
> > ===================================================================
> > --- linux-2.6.30-rc1.orig/mm/memcontrol.c
> > +++ linux-2.6.30-rc1/mm/memcontrol.c
> > @@ -314,13 +314,14 @@ static struct mem_cgroup *try_get_mem_cg
> >  	return mem;
> >  }
> > 
> > +#ifdef CONFIG_DEBUG_VM
> >  static bool mem_cgroup_is_obsolete(struct mem_cgroup *mem)
> >  {
> >  	if (!mem)
> >  		return true;
> >  	return css_is_removed(&mem->css);
> >  }
> > -
> > +#endif
> 
> Can we change the code to use
> 
>         VM_BUG_ON(!mem || css_is_removed(&mem->css));
> 

yup.

--- a/mm/memcontrol.c~memcg-remove-warning-when-config_debug_vm=n-fix
+++ a/mm/memcontrol.c
@@ -314,14 +314,13 @@ static struct mem_cgroup *try_get_mem_cg
 	return mem;
 }
 
-#ifdef CONFIG_DEBUG_VM
 static bool mem_cgroup_is_obsolete(struct mem_cgroup *mem)
 {
 	if (!mem)
 		return true;
 	return css_is_removed(&mem->css);
 }
-#endif
+
 
 /*
  * Call callback function against all cgroup under hierarchy tree.
@@ -933,7 +932,7 @@ static int __mem_cgroup_try_charge(struc
 	if (unlikely(!mem))
 		return 0;
 
-	VM_BUG_ON(mem_cgroup_is_obsolete(mem));
+	VM_BUG_ON(!mem || mem_cgroup_is_obsolete(mem));
 
 	while (1) {
 		int ret;
_

Although it really should be

	VM_BUG_ON(!mem);
	VM_BUG_ON(mem_cgroup_is_obsolete(mem));

because if that BUG triggers, you'll be wondering which case caused it.


--
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] 7+ messages in thread

* Re: [PATCH] memcg remove warning at DEBUG_VM=off
  2009-04-10  5:25   ` Andrew Morton
@ 2009-04-10  6:33     ` KAMEZAWA Hiroyuki
  2009-04-15 10:13       ` Andrea Righi
  0 siblings, 1 reply; 7+ messages in thread
From: KAMEZAWA Hiroyuki @ 2009-04-10  6:33 UTC (permalink / raw)
  To: Andrew Morton
  Cc: balbir, linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	nishimura@mxp.nes.nec.co.jp

On Thu, 9 Apr 2009 22:25:12 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Wed, 8 Apr 2009 10:57:15 +0530 Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> 
> > * KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2009-04-08 14:20:42]:
> > 
> > > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > > This is against 2.6.30-rc1. (maybe no problem against mmotm.)
> > > 
> > > ==
> > > Fix warning as
> > > 
> > >   CC      mm/memcontrol.o
> > > mm/memcontrol.c:318: warning: ?$B!Fmem_cgroup_is_obsolete?$B!G defined but not used
> > > 
> > > This is called only from VM_BUG_ON().
> > > 
> > > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > > ---
> > > Index: linux-2.6.30-rc1/mm/memcontrol.c
> > > ===================================================================
> > > --- linux-2.6.30-rc1.orig/mm/memcontrol.c
> > > +++ linux-2.6.30-rc1/mm/memcontrol.c
> > > @@ -314,13 +314,14 @@ static struct mem_cgroup *try_get_mem_cg
> > >  	return mem;
> > >  }
> > > 
> > > +#ifdef CONFIG_DEBUG_VM
> > >  static bool mem_cgroup_is_obsolete(struct mem_cgroup *mem)
> > >  {
> > >  	if (!mem)
> > >  		return true;
> > >  	return css_is_removed(&mem->css);
> > >  }
> > > -
> > > +#endif
> > 
> > Can we change the code to use
> > 
> >         VM_BUG_ON(!mem || css_is_removed(&mem->css));
> > 
> 
> yup.
> 
> --- a/mm/memcontrol.c~memcg-remove-warning-when-config_debug_vm=n-fix
> +++ a/mm/memcontrol.c
> @@ -314,14 +314,13 @@ static struct mem_cgroup *try_get_mem_cg
>  	return mem;
>  }
>  
> -#ifdef CONFIG_DEBUG_VM
>  static bool mem_cgroup_is_obsolete(struct mem_cgroup *mem)
>  {
>  	if (!mem)
>  		return true;
>  	return css_is_removed(&mem->css);
>  }
> -#endif
> +
>  
>  /*
>   * Call callback function against all cgroup under hierarchy tree.
> @@ -933,7 +932,7 @@ static int __mem_cgroup_try_charge(struc
>  	if (unlikely(!mem))
>  		return 0;
>  
> -	VM_BUG_ON(mem_cgroup_is_obsolete(mem));
> +	VM_BUG_ON(!mem || mem_cgroup_is_obsolete(mem));
>  
>  	while (1) {
>  		int ret;
> _
> 
> Although it really should be
> 
> 	VM_BUG_ON(!mem);
> 	VM_BUG_ON(mem_cgroup_is_obsolete(mem));
> 
> because if that BUG triggers, you'll be wondering which case caused it.
> 
Ah, sorry, I missed the reply.
maybe calling css_is_removed() directly is a choice.
I'll prepare v2.

Regards,
-Kame

--
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] 7+ messages in thread

* Re: [PATCH] memcg remove warning at DEBUG_VM=off
  2009-04-10  6:33     ` KAMEZAWA Hiroyuki
@ 2009-04-15 10:13       ` Andrea Righi
  2009-04-16  0:47         ` KAMEZAWA Hiroyuki
  2009-04-16  3:25         ` Balbir Singh
  0 siblings, 2 replies; 7+ messages in thread
From: Andrea Righi @ 2009-04-15 10:13 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: Andrew Morton, balbir, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, nishimura@mxp.nes.nec.co.jp

On Fri, Apr 10, 2009 at 03:33:35PM +0900, KAMEZAWA Hiroyuki wrote:
> On Thu, 9 Apr 2009 22:25:12 -0700
> Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Wed, 8 Apr 2009 10:57:15 +0530 Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> > 
> > > * KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2009-04-08 14:20:42]:
> > > 
> > > > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > > > This is against 2.6.30-rc1. (maybe no problem against mmotm.)
> > > > 
> > > > ==
> > > > Fix warning as
> > > > 
> > > >   CC      mm/memcontrol.o
> > > > mm/memcontrol.c:318: warning: ?$B!Fmem_cgroup_is_obsolete?$B!G defined but not used
> > > > 
> > > > This is called only from VM_BUG_ON().
> > > > 
> > > > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > > > ---
> > > > Index: linux-2.6.30-rc1/mm/memcontrol.c
> > > > ===================================================================
> > > > --- linux-2.6.30-rc1.orig/mm/memcontrol.c
> > > > +++ linux-2.6.30-rc1/mm/memcontrol.c
> > > > @@ -314,13 +314,14 @@ static struct mem_cgroup *try_get_mem_cg
> > > >  	return mem;
> > > >  }
> > > > 
> > > > +#ifdef CONFIG_DEBUG_VM
> > > >  static bool mem_cgroup_is_obsolete(struct mem_cgroup *mem)
> > > >  {
> > > >  	if (!mem)
> > > >  		return true;
> > > >  	return css_is_removed(&mem->css);
> > > >  }
> > > > -
> > > > +#endif
> > > 
> > > Can we change the code to use
> > > 
> > >         VM_BUG_ON(!mem || css_is_removed(&mem->css));
> > > 
> > 
> > yup.
> > 
> > --- a/mm/memcontrol.c~memcg-remove-warning-when-config_debug_vm=n-fix
> > +++ a/mm/memcontrol.c
> > @@ -314,14 +314,13 @@ static struct mem_cgroup *try_get_mem_cg
> >  	return mem;
> >  }
> >  
> > -#ifdef CONFIG_DEBUG_VM
> >  static bool mem_cgroup_is_obsolete(struct mem_cgroup *mem)
> >  {
> >  	if (!mem)
> >  		return true;
> >  	return css_is_removed(&mem->css);
> >  }
> > -#endif
> > +
> >  
> >  /*
> >   * Call callback function against all cgroup under hierarchy tree.
> > @@ -933,7 +932,7 @@ static int __mem_cgroup_try_charge(struc
> >  	if (unlikely(!mem))
> >  		return 0;
> >  
> > -	VM_BUG_ON(mem_cgroup_is_obsolete(mem));
> > +	VM_BUG_ON(!mem || mem_cgroup_is_obsolete(mem));
> >  
> >  	while (1) {
> >  		int ret;
> > _
> > 
> > Although it really should be
> > 
> > 	VM_BUG_ON(!mem);
> > 	VM_BUG_ON(mem_cgroup_is_obsolete(mem));
> > 
> > because if that BUG triggers, you'll be wondering which case caused it.
> > 
> Ah, sorry, I missed the reply.
> maybe calling css_is_removed() directly is a choice.
> I'll prepare v2.
> 
> Regards,
> -Kame

The warning is still there actually. I've just written a fix and seen
this discussion, maybe I can offload a little bit Kame. ;)

-Andrea
---
memcg: remove warning when CONFIG_DEBUG_VM is not set

Fix the following warning removing mem_cgroup_is_obsolete():

  mm/memcontrol.c:318: warning: a??mem_cgroup_is_obsoletea?? defined but not used

Moreover, split the VM_BUG_ON() checks in two parts to be aware of which
one triggered the bug.

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Andrea Righi <righi.andrea@gmail.com>
---
 mm/memcontrol.c |   11 ++---------
 1 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index e44fb0f..8cd6358 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -314,14 +314,6 @@ static struct mem_cgroup *try_get_mem_cgroup_from_mm(struct mm_struct *mm)
 	return mem;
 }
 
-static bool mem_cgroup_is_obsolete(struct mem_cgroup *mem)
-{
-	if (!mem)
-		return true;
-	return css_is_removed(&mem->css);
-}
-
-
 /*
  * Call callback function against all cgroup under hierarchy tree.
  */
@@ -932,7 +924,8 @@ static int __mem_cgroup_try_charge(struct mm_struct *mm,
 	if (unlikely(!mem))
 		return 0;
 
-	VM_BUG_ON(!mem || mem_cgroup_is_obsolete(mem));
+	VM_BUG_ON(!mem);
+	VM_BUG_ON(css_is_removed(&mem->css));
 
 	while (1) {
 		int ret;

--
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] 7+ messages in thread

* Re: [PATCH] memcg remove warning at DEBUG_VM=off
  2009-04-15 10:13       ` Andrea Righi
@ 2009-04-16  0:47         ` KAMEZAWA Hiroyuki
  2009-04-16  3:25         ` Balbir Singh
  1 sibling, 0 replies; 7+ messages in thread
From: KAMEZAWA Hiroyuki @ 2009-04-16  0:47 UTC (permalink / raw)
  To: Andrea Righi
  Cc: Andrew Morton, balbir, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, nishimura@mxp.nes.nec.co.jp

On Wed, 15 Apr 2009 12:13:17 +0200
Andrea Righi <righi.andrea@gmail.com> wrote:

> The warning is still there actually. I've just written a fix and seen
> this discussion, maybe I can offload a little bit Kame. ;)
> 
> -Andrea

Thank you.
Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

> ---
> memcg: remove warning when CONFIG_DEBUG_VM is not set
> 
> Fix the following warning removing mem_cgroup_is_obsolete():
> 
>   mm/memcontrol.c:318: warning: ‘mem_cgroup_is_obsolete’ defined but not used
> 
> Moreover, split the VM_BUG_ON() checks in two parts to be aware of which
> one triggered the bug.
> 
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Andrea Righi <righi.andrea@gmail.com>
> ---
>  mm/memcontrol.c |   11 ++---------
>  1 files changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index e44fb0f..8cd6358 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -314,14 +314,6 @@ static struct mem_cgroup *try_get_mem_cgroup_from_mm(struct mm_struct *mm)
>  	return mem;
>  }
>  
> -static bool mem_cgroup_is_obsolete(struct mem_cgroup *mem)
> -{
> -	if (!mem)
> -		return true;
> -	return css_is_removed(&mem->css);
> -}
> -
> -
>  /*
>   * Call callback function against all cgroup under hierarchy tree.
>   */
> @@ -932,7 +924,8 @@ static int __mem_cgroup_try_charge(struct mm_struct *mm,
>  	if (unlikely(!mem))
>  		return 0;
>  
> -	VM_BUG_ON(!mem || mem_cgroup_is_obsolete(mem));
> +	VM_BUG_ON(!mem);
> +	VM_BUG_ON(css_is_removed(&mem->css));
>  
>  	while (1) {
>  		int ret;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

--
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] 7+ messages in thread

* Re: [PATCH] memcg remove warning at DEBUG_VM=off
  2009-04-15 10:13       ` Andrea Righi
  2009-04-16  0:47         ` KAMEZAWA Hiroyuki
@ 2009-04-16  3:25         ` Balbir Singh
  1 sibling, 0 replies; 7+ messages in thread
From: Balbir Singh @ 2009-04-16  3:25 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki, Andrew Morton, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, nishimura@mxp.nes.nec.co.jp

* Andrea Righi <righi.andrea@gmail.com> [2009-04-15 12:13:17]:

> On Fri, Apr 10, 2009 at 03:33:35PM +0900, KAMEZAWA Hiroyuki wrote:
> > On Thu, 9 Apr 2009 22:25:12 -0700
> > Andrew Morton <akpm@linux-foundation.org> wrote:
> > 
> > > On Wed, 8 Apr 2009 10:57:15 +0530 Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> > > 
> > > > * KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2009-04-08 14:20:42]:
> > > > 
> > > > > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > > > > This is against 2.6.30-rc1. (maybe no problem against mmotm.)
> > > > > 
> > > > > ==
> > > > > Fix warning as
> > > > > 
> > > > >   CC      mm/memcontrol.o
> > > > > mm/memcontrol.c:318: warning: ?$B!Fmem_cgroup_is_obsolete?$B!G defined but not used
> > > > > 
> > > > > This is called only from VM_BUG_ON().
> > > > > 
> > > > > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > > > > ---
> > > > > Index: linux-2.6.30-rc1/mm/memcontrol.c
> > > > > ===================================================================
> > > > > --- linux-2.6.30-rc1.orig/mm/memcontrol.c
> > > > > +++ linux-2.6.30-rc1/mm/memcontrol.c
> > > > > @@ -314,13 +314,14 @@ static struct mem_cgroup *try_get_mem_cg
> > > > >  	return mem;
> > > > >  }
> > > > > 
> > > > > +#ifdef CONFIG_DEBUG_VM
> > > > >  static bool mem_cgroup_is_obsolete(struct mem_cgroup *mem)
> > > > >  {
> > > > >  	if (!mem)
> > > > >  		return true;
> > > > >  	return css_is_removed(&mem->css);
> > > > >  }
> > > > > -
> > > > > +#endif
> > > > 
> > > > Can we change the code to use
> > > > 
> > > >         VM_BUG_ON(!mem || css_is_removed(&mem->css));
> > > > 
> > > 
> > > yup.
> > > 
> > > --- a/mm/memcontrol.c~memcg-remove-warning-when-config_debug_vm=n-fix
> > > +++ a/mm/memcontrol.c
> > > @@ -314,14 +314,13 @@ static struct mem_cgroup *try_get_mem_cg
> > >  	return mem;
> > >  }
> > >  
> > > -#ifdef CONFIG_DEBUG_VM
> > >  static bool mem_cgroup_is_obsolete(struct mem_cgroup *mem)
> > >  {
> > >  	if (!mem)
> > >  		return true;
> > >  	return css_is_removed(&mem->css);
> > >  }
> > > -#endif
> > > +
> > >  
> > >  /*
> > >   * Call callback function against all cgroup under hierarchy tree.
> > > @@ -933,7 +932,7 @@ static int __mem_cgroup_try_charge(struc
> > >  	if (unlikely(!mem))
> > >  		return 0;
> > >  
> > > -	VM_BUG_ON(mem_cgroup_is_obsolete(mem));
> > > +	VM_BUG_ON(!mem || mem_cgroup_is_obsolete(mem));
> > >  
> > >  	while (1) {
> > >  		int ret;
> > > _
> > > 
> > > Although it really should be
> > > 
> > > 	VM_BUG_ON(!mem);
> > > 	VM_BUG_ON(mem_cgroup_is_obsolete(mem));
> > > 
> > > because if that BUG triggers, you'll be wondering which case caused it.
> > > 
> > Ah, sorry, I missed the reply.
> > maybe calling css_is_removed() directly is a choice.
> > I'll prepare v2.
> > 
> > Regards,
> > -Kame
> 
> The warning is still there actually. I've just written a fix and seen
> this discussion, maybe I can offload a little bit Kame. ;)
> 
> -Andrea
> ---
> memcg: remove warning when CONFIG_DEBUG_VM is not set
> 
> Fix the following warning removing mem_cgroup_is_obsolete():
> 
>   mm/memcontrol.c:318: warning: ???mem_cgroup_is_obsolete??? defined but not used
> 
> Moreover, split the VM_BUG_ON() checks in two parts to be aware of which
> one triggered the bug.
> 
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Andrea Righi <righi.andrea@gmail.com>
> ---
>  mm/memcontrol.c |   11 ++---------
>  1 files changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index e44fb0f..8cd6358 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -314,14 +314,6 @@ static struct mem_cgroup *try_get_mem_cgroup_from_mm(struct mm_struct *mm)
>  	return mem;
>  }
> 
> -static bool mem_cgroup_is_obsolete(struct mem_cgroup *mem)
> -{
> -	if (!mem)
> -		return true;
> -	return css_is_removed(&mem->css);
> -}
> -
> -
>  /*
>   * Call callback function against all cgroup under hierarchy tree.
>   */
> @@ -932,7 +924,8 @@ static int __mem_cgroup_try_charge(struct mm_struct *mm,
>  	if (unlikely(!mem))
>  		return 0;
> 
> -	VM_BUG_ON(!mem || mem_cgroup_is_obsolete(mem));
> +	VM_BUG_ON(!mem);
> +	VM_BUG_ON(css_is_removed(&mem->css));
>

Perfect, this is exactly what I wanted. I was in the middle of writing
this patch.

 
Reviewed-by: Balbir Singh <balbir@linux.vnet.ibm.com>
 

-- 
	Balbir

--
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] 7+ messages in thread

end of thread, other threads:[~2009-04-16  3:26 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-04-08  5:20 [PATCH] memcg remove warning at DEBUG_VM=off KAMEZAWA Hiroyuki
2009-04-08  5:27 ` Balbir Singh
2009-04-10  5:25   ` Andrew Morton
2009-04-10  6:33     ` KAMEZAWA Hiroyuki
2009-04-15 10:13       ` Andrea Righi
2009-04-16  0:47         ` KAMEZAWA Hiroyuki
2009-04-16  3:25         ` Balbir Singh

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