[PATCH] make sparse happy with gfp.h

[PATCH] make sparse happy with gfp.h

[Dave Hansen]

Running sparse on page_alloc.c today, it errors out:
        include/linux/gfp.h:254:17: error: bad constant expression
        include/linux/gfp.h:254:17: error: cannot size expression

which is a line in gfp_zone():

        BUILD_BUG_ON((GFP_ZONE_BAD >> bit) & 1);

That's really unfortunate, because it ends up hiding all of the other
legitimate sparse messages like this:
        mm/page_alloc.c:5315:59: warning: incorrect type in argument 1 (different base types)
        mm/page_alloc.c:5315:59:    expected unsigned long [unsigned] [usertype] size
        mm/page_alloc.c:5315:59:    got restricted gfp_t [usertype] <noident>
...

Having sparse be able to catch these very oopsable bugs is a lot more
important than keeping a BUILD_BUG_ON().  Kill the BUILD_BUG_ON().

Compiles on x86_64 with and without CONFIG_DEBUG_VM=y.  defconfig
boots fine for me.

Signed-off-by: Dave Hansen <dave@linux.vnet.ibm.com>
---

 linux-2.6.git-dave/include/linux/gfp.h |    7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff -puN include/linux/gfp.h~make-sparse-happy-with-gfp_h include/linux/gfp.h
--- linux-2.6.git/include/linux/gfp.h~make-sparse-happy-with-gfp_h	2011-04-14 14:47:02.629275904 -0700
+++ linux-2.6.git-dave/include/linux/gfp.h	2011-04-14 14:47:38.813272674 -0700
@@ -249,14 +249,9 @@ static inline enum zone_type gfp_zone(gf
 
 	z = (GFP_ZONE_TABLE >> (bit * ZONES_SHIFT)) &
 					 ((1 << ZONES_SHIFT) - 1);
-
-	if (__builtin_constant_p(bit))
-		BUILD_BUG_ON((GFP_ZONE_BAD >> bit) & 1);
-	else {
 #ifdef CONFIG_DEBUG_VM
-		BUG_ON((GFP_ZONE_BAD >> bit) & 1);
+	BUG_ON((GFP_ZONE_BAD >> bit) & 1);
 #endif
-	}
 	return z;
 }
 
_

[PATCH] make sparse happy with gfp.h

[Dave Hansen]

Running sparse on page_alloc.c today, it errors out:
        include/linux/gfp.h:254:17: error: bad constant expression
        include/linux/gfp.h:254:17: error: cannot size expression

which is a line in gfp_zone():

        BUILD_BUG_ON((GFP_ZONE_BAD >> bit) & 1);

That's really unfortunate, because it ends up hiding all of the other
legitimate sparse messages like this:
        mm/page_alloc.c:5315:59: warning: incorrect type in argument 1 (different base types)
        mm/page_alloc.c:5315:59:    expected unsigned long [unsigned] [usertype] size
        mm/page_alloc.c:5315:59:    got restricted gfp_t [usertype] <noident>
...

Having sparse be able to catch these very oopsable bugs is a lot more
important than keeping a BUILD_BUG_ON().  Kill the BUILD_BUG_ON().

Compiles on x86_64 with and without CONFIG_DEBUG_VM=y.  defconfig
boots fine for me.

Signed-off-by: Dave Hansen <dave@linux.vnet.ibm.com>
---

 linux-2.6.git-dave/include/linux/gfp.h |    7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff -puN include/linux/gfp.h~make-sparse-happy-with-gfp_h include/linux/gfp.h
--- linux-2.6.git/include/linux/gfp.h~make-sparse-happy-with-gfp_h	2011-04-14 14:47:02.629275904 -0700
+++ linux-2.6.git-dave/include/linux/gfp.h	2011-04-14 14:47:38.813272674 -0700
@@ -249,14 +249,9 @@ static inline enum zone_type gfp_zone(gf
 
 	z = (GFP_ZONE_TABLE >> (bit * ZONES_SHIFT)) &
 					 ((1 << ZONES_SHIFT) - 1);
-
-	if (__builtin_constant_p(bit))
-		BUILD_BUG_ON((GFP_ZONE_BAD >> bit) & 1);
-	else {
 #ifdef CONFIG_DEBUG_VM
-		BUG_ON((GFP_ZONE_BAD >> bit) & 1);
+	BUG_ON((GFP_ZONE_BAD >> bit) & 1);
 #endif
-	}
 	return z;
 }
 
_

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] make sparse happy with gfp.h

[KOSAKI Motohiro]

Hello,

> diff -puN include/linux/gfp.h~make-sparse-happy-with-gfp_h include/linux/gfp.h
> --- linux-2.6.git/include/linux/gfp.h~make-sparse-happy-with-gfp_h	2011-04-14 14:47:02.629275904 -0700
> +++ linux-2.6.git-dave/include/linux/gfp.h	2011-04-14 14:47:38.813272674 -0700
> @@ -249,14 +249,9 @@ static inline enum zone_type gfp_zone(gf
>  
>  	z = (GFP_ZONE_TABLE >> (bit * ZONES_SHIFT)) &
>  					 ((1 << ZONES_SHIFT) - 1);
> -
> -	if (__builtin_constant_p(bit))
> -		BUILD_BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> -	else {
>  #ifdef CONFIG_DEBUG_VM
> -		BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> +	BUG_ON((GFP_ZONE_BAD >> bit) & 1);
>  #endif
> -	}
>  	return z;

Why don't you use VM_BUG_ON?

Re: [PATCH] make sparse happy with gfp.h

[KOSAKI Motohiro]

Hello,

> diff -puN include/linux/gfp.h~make-sparse-happy-with-gfp_h include/linux/gfp.h
> --- linux-2.6.git/include/linux/gfp.h~make-sparse-happy-with-gfp_h	2011-04-14 14:47:02.629275904 -0700
> +++ linux-2.6.git-dave/include/linux/gfp.h	2011-04-14 14:47:38.813272674 -0700
> @@ -249,14 +249,9 @@ static inline enum zone_type gfp_zone(gf
>  
>  	z = (GFP_ZONE_TABLE >> (bit * ZONES_SHIFT)) &
>  					 ((1 << ZONES_SHIFT) - 1);
> -
> -	if (__builtin_constant_p(bit))
> -		BUILD_BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> -	else {
>  #ifdef CONFIG_DEBUG_VM
> -		BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> +	BUG_ON((GFP_ZONE_BAD >> bit) & 1);
>  #endif
> -	}
>  	return z;

Why don't you use VM_BUG_ON?


--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] make sparse happy with gfp.h

[Dave Hansen]

On Fri, 2011-04-15 at 12:14 +0900, KOSAKI Motohiro wrote:
> >  #ifdef CONFIG_DEBUG_VM
> > -             BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> > +     BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> >  #endif
> > -     }
> >       return z;
> 
> Why don't you use VM_BUG_ON?

I was just trying to make a minimal patch that did a single thing.

Feel free to submit another one that does that.  I'm sure there are a
couple more places that could use similar love.

-- Dave

Re: [PATCH] make sparse happy with gfp.h

[Dave Hansen]

On Fri, 2011-04-15 at 12:14 +0900, KOSAKI Motohiro wrote:
> >  #ifdef CONFIG_DEBUG_VM
> > -             BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> > +     BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> >  #endif
> > -     }
> >       return z;
> 
> Why don't you use VM_BUG_ON?

I was just trying to make a minimal patch that did a single thing.

Feel free to submit another one that does that.  I'm sure there are a
couple more places that could use similar love.

-- Dave

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH] define dummy BUILD_BUG_ON definition for sparse

[KOSAKI Motohiro]

> Hello,
> 
> > diff -puN include/linux/gfp.h~make-sparse-happy-with-gfp_h include/linux/gfp.h
> > --- linux-2.6.git/include/linux/gfp.h~make-sparse-happy-with-gfp_h	2011-04-14 14:47:02.629275904 -0700
> > +++ linux-2.6.git-dave/include/linux/gfp.h	2011-04-14 14:47:38.813272674 -0700
> > @@ -249,14 +249,9 @@ static inline enum zone_type gfp_zone(gf
> >  
> >  	z = (GFP_ZONE_TABLE >> (bit * ZONES_SHIFT)) &
> >  					 ((1 << ZONES_SHIFT) - 1);
> > -
> > -	if (__builtin_constant_p(bit))
> > -		BUILD_BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> > -	else {
> >  #ifdef CONFIG_DEBUG_VM
> > -		BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> > +	BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> >  #endif
> > -	}
> >  	return z;
> 
> Why don't you use VM_BUG_ON?

After while thinking, I decided to make another patch. If we take your
approach we will remove all BUILD_BUG_ON eventually. It's no happy result.


>From 2da32b2875a6bd0bb0166993b4663eac0c5d1d6d Mon Sep 17 00:00:00 2001
From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Date: Fri, 15 Apr 2011 13:37:24 +0900
Subject: [PATCH] define dummy BUILD_BUG_ON definition for sparse

BUILD_BUG_ON() makes syntax error to detect coding error. Then it
naturally makes sparse error too. It reduce sparse usefulness.

Then, this patch makes dummy BUILD_BUG_ON() definition for sparse.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
---
 include/linux/kernel.h |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 00cec4d..9ac44b8 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -637,6 +637,14 @@ struct sysinfo {
 	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. */
 };
 
+#ifdef __CHECKER__
+#define BUILD_BUG_ON_NOT_POWER_OF_2(n)
+#define BUILD_BUG_ON_ZERO(e)
+#define BUILD_BUG_ON_NULL(e)
+#define BUILD_BUG_ON(condition)
+#else /* __CHECKER__ */
+
 /* Force a compilation error if a constant expression is not a power of 2 */
 #define BUILD_BUG_ON_NOT_POWER_OF_2(n)			\
 	BUILD_BUG_ON((n) == 0 || (((n) & ((n) - 1)) != 0))
@@ -673,6 +681,7 @@ extern int __build_bug_on_failed;
 		if (condition) __build_bug_on_failed = 1;	\
 	} while(0)
 #endif
+#endif /* __CHECKER__ */
 
 /* Trap pasters of __FUNCTION__ at compile-time */
 #define __FUNCTION__ (__func__)
-- 
1.7.3.1

[PATCH] define dummy BUILD_BUG_ON definition for sparse

[KOSAKI Motohiro]

> Hello,
> 
> > diff -puN include/linux/gfp.h~make-sparse-happy-with-gfp_h include/linux/gfp.h
> > --- linux-2.6.git/include/linux/gfp.h~make-sparse-happy-with-gfp_h	2011-04-14 14:47:02.629275904 -0700
> > +++ linux-2.6.git-dave/include/linux/gfp.h	2011-04-14 14:47:38.813272674 -0700
> > @@ -249,14 +249,9 @@ static inline enum zone_type gfp_zone(gf
> >  
> >  	z = (GFP_ZONE_TABLE >> (bit * ZONES_SHIFT)) &
> >  					 ((1 << ZONES_SHIFT) - 1);
> > -
> > -	if (__builtin_constant_p(bit))
> > -		BUILD_BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> > -	else {
> >  #ifdef CONFIG_DEBUG_VM
> > -		BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> > +	BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> >  #endif
> > -	}
> >  	return z;
> 
> Why don't you use VM_BUG_ON?

After while thinking, I decided to make another patch. If we take your
approach we will remove all BUILD_BUG_ON eventually. It's no happy result.


From 2da32b2875a6bd0bb0166993b4663eac0c5d1d6d Mon Sep 17 00:00:00 2001
From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Date: Fri, 15 Apr 2011 13:37:24 +0900
Subject: [PATCH] define dummy BUILD_BUG_ON definition for sparse

BUILD_BUG_ON() makes syntax error to detect coding error. Then it
naturally makes sparse error too. It reduce sparse usefulness.

Then, this patch makes dummy BUILD_BUG_ON() definition for sparse.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
---
 include/linux/kernel.h |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 00cec4d..9ac44b8 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -637,6 +637,14 @@ struct sysinfo {
 	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. */
 };
 
+#ifdef __CHECKER__
+#define BUILD_BUG_ON_NOT_POWER_OF_2(n)
+#define BUILD_BUG_ON_ZERO(e)
+#define BUILD_BUG_ON_NULL(e)
+#define BUILD_BUG_ON(condition)
+#else /* __CHECKER__ */
+
 /* Force a compilation error if a constant expression is not a power of 2 */
 #define BUILD_BUG_ON_NOT_POWER_OF_2(n)			\
 	BUILD_BUG_ON((n) == 0 || (((n) & ((n) - 1)) != 0))
@@ -673,6 +681,7 @@ extern int __build_bug_on_failed;
 		if (condition) __build_bug_on_failed = 1;	\
 	} while(0)
 #endif
+#endif /* __CHECKER__ */
 
 /* Trap pasters of __FUNCTION__ at compile-time */
 #define __FUNCTION__ (__func__)
-- 
1.7.3.1





--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH] define __must_be_array() for __CHECKER__

[KOSAKI Motohiro]

This fixes another sparse splat.


>From 711131e2e16925970a67103156af1296993dbc93 Mon Sep 17 00:00:00 2001
From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Date: Fri, 15 Apr 2011 13:28:16 +0900
Subject: [PATCH] define __must_be_array() for __CHECKER__

commit c5e631cf65f (ARRAY_SIZE: check for type) added __must_be_array().
but sparse can't parse this gcc extention.

Then, now make C=2 makes following sparse errors a lot.

  kernel/futex.c:2699:25: error: No right hand side of '+'-expression

Because __must_be_array() is used for ARRAY_SIZE() macro and it is
used very widely.

This patch fixes it.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
---
 include/linux/compiler-gcc.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index cb4c1eb..59e4028 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -34,8 +34,12 @@
     __asm__ ("" : "=r"(__ptr) : "0"(ptr));		\
     (typeof(ptr)) (__ptr + (off)); })
 
+#ifdef __CHECKER__
+#define __must_be_array(arr) 0
+#else
 /* &a[0] degrades to a pointer: a different type from an array */
 #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
+#endif
 
 /*
  * Force always-inline if the user requests it so via the .config,
-- 
1.7.3.1

[PATCH] define __must_be_array() for __CHECKER__

[KOSAKI Motohiro]

This fixes another sparse splat.


From 711131e2e16925970a67103156af1296993dbc93 Mon Sep 17 00:00:00 2001
From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Date: Fri, 15 Apr 2011 13:28:16 +0900
Subject: [PATCH] define __must_be_array() for __CHECKER__

commit c5e631cf65f (ARRAY_SIZE: check for type) added __must_be_array().
but sparse can't parse this gcc extention.

Then, now make C=2 makes following sparse errors a lot.

  kernel/futex.c:2699:25: error: No right hand side of '+'-expression

Because __must_be_array() is used for ARRAY_SIZE() macro and it is
used very widely.

This patch fixes it.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
---
 include/linux/compiler-gcc.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index cb4c1eb..59e4028 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -34,8 +34,12 @@
     __asm__ ("" : "=r"(__ptr) : "0"(ptr));		\
     (typeof(ptr)) (__ptr + (off)); })
 
+#ifdef __CHECKER__
+#define __must_be_array(arr) 0
+#else
 /* &a[0] degrades to a pointer: a different type from an array */
 #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
+#endif
 
 /*
  * Force always-inline if the user requests it so via the .config,
-- 
1.7.3.1



--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH] Undef __compiletime_{warning,error} if __CHECKER__ is defined

[KOSAKI Motohiro]

>From 4560a800c056714a7b5f9dc813460698717e6998 Mon Sep 17 00:00:00 2001
From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Date: Fri, 15 Apr 2011 13:58:07 +0900
Subject: [PATCH] Undef __compiletime_{warning,error} if __CHECKER__ is defined

sparse can't parse warning and error attribute. then they should
be hidden from sparse.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Arjan van de Ven <arjan@infradead.org>
---
 include/linux/compiler-gcc4.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/compiler-gcc4.h b/include/linux/compiler-gcc4.h
index 64b7c00..dfadc96 100644
--- a/include/linux/compiler-gcc4.h
+++ b/include/linux/compiler-gcc4.h
@@ -51,7 +51,7 @@
 #if __GNUC_MINOR__ > 0
 #define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
 #endif
-#if __GNUC_MINOR__ >= 4
+#if __GNUC_MINOR__ >= 4 && !defined(__CHECKER__)
 #define __compiletime_warning(message) __attribute__((warning(message)))
 #define __compiletime_error(message) __attribute__((error(message)))
 #endif
-- 
1.7.3.1

[PATCH] Undef __compiletime_{warning,error} if __CHECKER__ is defined

[KOSAKI Motohiro]

From 4560a800c056714a7b5f9dc813460698717e6998 Mon Sep 17 00:00:00 2001
From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Date: Fri, 15 Apr 2011 13:58:07 +0900
Subject: [PATCH] Undef __compiletime_{warning,error} if __CHECKER__ is defined

sparse can't parse warning and error attribute. then they should
be hidden from sparse.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Arjan van de Ven <arjan@infradead.org>
---
 include/linux/compiler-gcc4.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/compiler-gcc4.h b/include/linux/compiler-gcc4.h
index 64b7c00..dfadc96 100644
--- a/include/linux/compiler-gcc4.h
+++ b/include/linux/compiler-gcc4.h
@@ -51,7 +51,7 @@
 #if __GNUC_MINOR__ > 0
 #define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
 #endif
-#if __GNUC_MINOR__ >= 4
+#if __GNUC_MINOR__ >= 4 && !defined(__CHECKER__)
 #define __compiletime_warning(message) __attribute__((warning(message)))
 #define __compiletime_error(message) __attribute__((error(message)))
 #endif
-- 
1.7.3.1



--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] make sparse happy with gfp.h

[KOSAKI Motohiro]

Hello,

> On Fri, 2011-04-15 at 12:14 +0900, KOSAKI Motohiro wrote:
> > >  #ifdef CONFIG_DEBUG_VM
> > > -             BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> > > +     BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> > >  #endif
> > > -     }
> > >       return z;
> > 
> > Why don't you use VM_BUG_ON?
> 
> I was just trying to make a minimal patch that did a single thing.
> 
> Feel free to submit another one that does that.  I'm sure there are a
> couple more places that could use similar love.

I posted another approach patches a second ago. Could you please see it?

Re: [PATCH] make sparse happy with gfp.h

[KOSAKI Motohiro]

Hello,

> On Fri, 2011-04-15 at 12:14 +0900, KOSAKI Motohiro wrote:
> > >  #ifdef CONFIG_DEBUG_VM
> > > -             BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> > > +     BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> > >  #endif
> > > -     }
> > >       return z;
> > 
> > Why don't you use VM_BUG_ON?
> 
> I was just trying to make a minimal patch that did a single thing.
> 
> Feel free to submit another one that does that.  I'm sure there are a
> couple more places that could use similar love.

I posted another approach patches a second ago. Could you please see 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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] fix sparse happy borkage when including gfp.h

[Dave Hansen]

On Fri, 2011-04-15 at 14:33 +0900, KOSAKI Motohiro wrote:
> Hello,
> > On Fri, 2011-04-15 at 12:14 +0900, KOSAKI Motohiro wrote:
> > > >  #ifdef CONFIG_DEBUG_VM
> > > > -             BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> > > > +     BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> > > >  #endif
> > > > -     }
> > > >       return z;
> > > 
> > > Why don't you use VM_BUG_ON?
> > 
> > I was just trying to make a minimal patch that did a single thing.
> > 
> > Feel free to submit another one that does that.  I'm sure there are a
> > couple more places that could use similar love.
> 
> I posted another approach patches a second ago. Could you please see it?

Those both look sane to me.  Those weren't biting me in particular, and
they don't fix the issue I was seeing.  But, they do seem necessary to
reduce some of the noise.

CC'ing the sparse mailing list.  We're seeing a couple of cases where
some gcc-isms are either stopping sparse from finding real bugs:

	http://marc.info/?l=linux-mm&m=130282454732689&w=2

or creating a lot of noise on some builds:

	http://marc.info/?l=linux-mm&m=130284428614058&w=2
	http://marc.info/?l=linux-mm&m=130284431014077&w=2

-- Dave

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] fix sparse happy borkage when including gfp.h

[Dave Hansen]

On Fri, 2011-04-15 at 14:33 +0900, KOSAKI Motohiro wrote:
> Hello,
> > On Fri, 2011-04-15 at 12:14 +0900, KOSAKI Motohiro wrote:
> > > >  #ifdef CONFIG_DEBUG_VM
> > > > -             BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> > > > +     BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> > > >  #endif
> > > > -     }
> > > >       return z;
> > > 
> > > Why don't you use VM_BUG_ON?
> > 
> > I was just trying to make a minimal patch that did a single thing.
> > 
> > Feel free to submit another one that does that.  I'm sure there are a
> > couple more places that could use similar love.
> 
> I posted another approach patches a second ago. Could you please see it?

Those both look sane to me.  Those weren't biting me in particular, and
they don't fix the issue I was seeing.  But, they do seem necessary to
reduce some of the noise.

CC'ing the sparse mailing list.  We're seeing a couple of cases where
some gcc-isms are either stopping sparse from finding real bugs:

	http://marc.info/?l=linux-mm&m=130282454732689&w=2

or creating a lot of noise on some builds:

	http://marc.info/?l=linux-mm&m=130284428614058&w=2
	http://marc.info/?l=linux-mm&m=130284431014077&w=2

-- Dave

Re: [PATCH] fix sparse happy borkage when including gfp.h

[Christopher Li]

On Fri, Apr 15, 2011 at 7:27 AM, Dave Hansen <dave@linux.vnet.ibm.com> wrote:
> Those both look sane to me.  Those weren't biting me in particular, and
> they don't fix the issue I was seeing.  But, they do seem necessary to
> reduce some of the noise.
>
> CC'ing the sparse mailing list.  We're seeing a couple of cases where
> some gcc-isms are either stopping sparse from finding real bugs:
>
>        http://marc.info/?l=linux-mm&m=130282454732689&w=2

I take a look at this problem. The bug can be simplify as the following source:

typedef unsigned __attribute__((bitwise)) gfp_t;

void foo ( gfp_t flags)
{
	int bit = (__attribute__((force)) int) (flags &
(((__attribute__((force)) gfp_t)0x01u)|((__attribute__((force))
gfp_t)0x02u)|((__attribute__((force))
gfp_t)0x04u)|((__attribute__((force)) gfp_t)0x08u)));

	if (__builtin_constant_p(bit))
			((void)sizeof(char[1 - 2 * !!((( 1 << (0x01u | 0x02u) | 1 << (0x01u
| 0x04u) | 1 << (0x04u | 0x02u) | 1 << (0x01u | 0x04u | 0x02u) | 1 <<
(0x08u | 0x02u | 0x01u) | 1 << (0x08u | 0x04u | 0x01u) | 1 << (0x08u |
0x04u | 0x02u) | 1 << (0x08u | 0x04u | 0x01u | 0x02u) ) >> bit) &
1)]));
}

Sparse chock on variable length array. It is nothing new. It was
useful to warn about
the variable length array usage in the kernel, because it might
overflow the kernel stack.
In this case, it is just a clever trick to issue assert on compile time.

We should bite the bullet and just parse the variable length array as
expression. We can still issue
warning for non constant usage, without the early bail out from sparse.

Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html