* [PATCH 1/4] compiler.h: introduce unused_expression() macro
@ 2012-04-25 11:26 Konstantin Khlebnikov
2012-04-25 11:26 ` [PATCH 2/4] bug: completely remove code of disabled VM_BUG_ON() Konstantin Khlebnikov
` (8 more replies)
0 siblings, 9 replies; 24+ messages in thread
From: Konstantin Khlebnikov @ 2012-04-25 11:26 UTC (permalink / raw)
To: Linus Torvalds, linux-kernel; +Cc: linux-arch, Andrew Morton
Sometimes we want to check some expressions correctness in compile-time without
generating extra code. "(void)(e)" does not work if expression has side-effects.
This patch introduces macro unused_expression() which helps in this situation.
Cast to "long" required because sizeof does not work for bit-fields.
Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
---
include/linux/compiler.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 923d093..46fbda3 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -310,4 +310,6 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);
*/
#define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))
+#define unused_expression(e) ((void)(sizeof((__force long)(e))))
+
#endif /* __LINUX_COMPILER_H */
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/4] bug: completely remove code of disabled VM_BUG_ON()
2012-04-25 11:26 [PATCH 1/4] compiler.h: introduce unused_expression() macro Konstantin Khlebnikov
@ 2012-04-25 11:26 ` Konstantin Khlebnikov
2012-04-25 14:40 ` Geert Uytterhoeven
2012-04-25 11:26 ` [PATCH 3/4] bug: completely remove code of disabled BUG_ON() Konstantin Khlebnikov
` (7 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Konstantin Khlebnikov @ 2012-04-25 11:26 UTC (permalink / raw)
To: Linus Torvalds, linux-kernel; +Cc: linux-arch, Andrew Morton
Even if CONFIG_DEBUG_VM=n gcc genereates code for some VM_BUG_ON()
bloat-o-meter: add/remove: 0/1 grow/shrink: 10/120 up/down: 185/-1889 (-1704)
Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
---
include/linux/mmdebug.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/mmdebug.h b/include/linux/mmdebug.h
index c04ecfe..9c9a2a0 100644
--- a/include/linux/mmdebug.h
+++ b/include/linux/mmdebug.h
@@ -4,7 +4,7 @@
#ifdef CONFIG_DEBUG_VM
#define VM_BUG_ON(cond) BUG_ON(cond)
#else
-#define VM_BUG_ON(cond) do { (void)(cond); } while (0)
+#define VM_BUG_ON(cond) unused_expression(cond)
#endif
#ifdef CONFIG_DEBUG_VIRTUAL
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/4] bug: completely remove code of disabled BUG_ON()
2012-04-25 11:26 [PATCH 1/4] compiler.h: introduce unused_expression() macro Konstantin Khlebnikov
2012-04-25 11:26 ` [PATCH 2/4] bug: completely remove code of disabled VM_BUG_ON() Konstantin Khlebnikov
@ 2012-04-25 11:26 ` Konstantin Khlebnikov
2012-04-25 11:26 ` [PATCH 4/4] bug: mark disabled BUG() as unreachable() code Konstantin Khlebnikov
` (6 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: Konstantin Khlebnikov @ 2012-04-25 11:26 UTC (permalink / raw)
To: Linus Torvalds, linux-kernel; +Cc: linux-arch, Andrew Morton
Even if CONFIG_BUG=n gcc generates code for some BUG_ON()
bloat-o-meter: add/remove: 2/9 grow/shrink: 25/229 up/down: 1053/-5019 (-3966)
Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
---
include/asm-generic/bug.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 2520a6e..aadb6fc 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -112,7 +112,7 @@ extern void warn_slowpath_null(const char *file, const int line);
#endif
#ifndef HAVE_ARCH_BUG_ON
-#define BUG_ON(condition) do { if (condition) ; } while(0)
+#define BUG_ON(condition) unused_expression(condition)
#endif
#ifndef HAVE_ARCH_WARN_ON
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 4/4] bug: mark disabled BUG() as unreachable() code
2012-04-25 11:26 [PATCH 1/4] compiler.h: introduce unused_expression() macro Konstantin Khlebnikov
2012-04-25 11:26 ` [PATCH 2/4] bug: completely remove code of disabled VM_BUG_ON() Konstantin Khlebnikov
2012-04-25 11:26 ` [PATCH 3/4] bug: completely remove code of disabled BUG_ON() Konstantin Khlebnikov
@ 2012-04-25 11:26 ` Konstantin Khlebnikov
2012-04-28 5:10 ` Konstantin Khlebnikov
2012-04-25 11:51 ` [PATCH 1/4] compiler.h: introduce unused_expression() macro Cong Wang
` (5 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Konstantin Khlebnikov @ 2012-04-25 11:26 UTC (permalink / raw)
To: Linus Torvalds, linux-kernel; +Cc: linux-arch, Andrew Morton
This patch suppress some compiler warnings (if CONFIG_BUG=n)
"warning: control reaches end of non-void function"
Plus now gcc can throw out some dead code.
bloat-o-meter: add/remove: 1/1 grow/shrink: 68/173 up/down: 1569/-5607 (-4038)
Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
---
include/asm-generic/bug.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index aadb6fc..0b08199 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -108,7 +108,7 @@ extern void warn_slowpath_null(const char *file, const int line);
#else /* !CONFIG_BUG */
#ifndef HAVE_ARCH_BUG
-#define BUG() do {} while(0)
+#define BUG() unreachable()
#endif
#ifndef HAVE_ARCH_BUG_ON
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 1/4] compiler.h: introduce unused_expression() macro
2012-04-25 11:26 [PATCH 1/4] compiler.h: introduce unused_expression() macro Konstantin Khlebnikov
` (2 preceding siblings ...)
2012-04-25 11:26 ` [PATCH 4/4] bug: mark disabled BUG() as unreachable() code Konstantin Khlebnikov
@ 2012-04-25 11:51 ` Cong Wang
2012-04-25 11:54 ` Konstantin Khlebnikov
2012-04-26 22:29 ` Andrew Morton
` (4 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Cong Wang @ 2012-04-25 11:51 UTC (permalink / raw)
To: Konstantin Khlebnikov
Cc: Linus Torvalds, linux-kernel, linux-arch, Andrew Morton
On 04/25/2012 07:26 PM, Konstantin Khlebnikov wrote:
> Sometimes we want to check some expressions correctness in compile-time without
> generating extra code. "(void)(e)" does not work if expression has side-effects.
> This patch introduces macro unused_expression() which helps in this situation.
>
Interesting, I am wondering why gcc doesn't eliminate the code as we
pass either -O2 or -Os to it.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/4] compiler.h: introduce unused_expression() macro
2012-04-25 11:51 ` [PATCH 1/4] compiler.h: introduce unused_expression() macro Cong Wang
@ 2012-04-25 11:54 ` Konstantin Khlebnikov
0 siblings, 0 replies; 24+ messages in thread
From: Konstantin Khlebnikov @ 2012-04-25 11:54 UTC (permalink / raw)
To: Cong Wang; +Cc: Linus Torvalds, linux-kernel, linux-arch, Andrew Morton
Cong Wang wrote:
> On 04/25/2012 07:26 PM, Konstantin Khlebnikov wrote:
>> Sometimes we want to check some expressions correctness in compile-time without
>> generating extra code. "(void)(e)" does not work if expression has side-effects.
>> This patch introduces macro unused_expression() which helps in this situation.
>>
>
> Interesting, I am wondering why gcc doesn't eliminate the code as we
> pass either -O2 or -Os to it.
It cannot do this if expression has some side-effects, for example if it contains BUG().
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/4] bug: completely remove code of disabled VM_BUG_ON()
2012-04-25 11:26 ` [PATCH 2/4] bug: completely remove code of disabled VM_BUG_ON() Konstantin Khlebnikov
@ 2012-04-25 14:40 ` Geert Uytterhoeven
2012-04-26 22:32 ` Andrew Morton
0 siblings, 1 reply; 24+ messages in thread
From: Geert Uytterhoeven @ 2012-04-25 14:40 UTC (permalink / raw)
To: Konstantin Khlebnikov
Cc: Linus Torvalds, linux-kernel, linux-arch, Andrew Morton
On Wed, Apr 25, 2012 at 13:26, Konstantin Khlebnikov
<khlebnikov@openvz.org> wrote:
> Even if CONFIG_DEBUG_VM=n gcc genereates code for some VM_BUG_ON()
That's because of the side effects of the expression
https://lkml.org/lkml/2012/4/25/146
But IRIC, we do want them here?
> bloat-o-meter: add/remove: 0/1 grow/shrink: 10/120 up/down: 185/-1889 (-1704)
>
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
> ---
> include/linux/mmdebug.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/mmdebug.h b/include/linux/mmdebug.h
> index c04ecfe..9c9a2a0 100644
> --- a/include/linux/mmdebug.h
> +++ b/include/linux/mmdebug.h
> @@ -4,7 +4,7 @@
> #ifdef CONFIG_DEBUG_VM
> #define VM_BUG_ON(cond) BUG_ON(cond)
> #else
> -#define VM_BUG_ON(cond) do { (void)(cond); } while (0)
> +#define VM_BUG_ON(cond) unused_expression(cond)
> #endif
>
> #ifdef CONFIG_DEBUG_VIRTUAL
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/4] compiler.h: introduce unused_expression() macro
2012-04-25 11:26 [PATCH 1/4] compiler.h: introduce unused_expression() macro Konstantin Khlebnikov
` (3 preceding siblings ...)
2012-04-25 11:51 ` [PATCH 1/4] compiler.h: introduce unused_expression() macro Cong Wang
@ 2012-04-26 22:29 ` Andrew Morton
2012-04-27 9:55 ` Konstantin Khlebnikov
2012-04-26 22:34 ` Andrew Morton
` (3 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Andrew Morton @ 2012-04-26 22:29 UTC (permalink / raw)
To: Konstantin Khlebnikov; +Cc: Linus Torvalds, linux-kernel, linux-arch
On Wed, 25 Apr 2012 15:26:23 +0400
Konstantin Khlebnikov <khlebnikov@openvz.org> wrote:
> Sometimes we want to check some expressions correctness in compile-time without
> generating extra code. "(void)(e)" does not work if expression has side-effects.
> This patch introduces macro unused_expression() which helps in this situation.
>
> Cast to "long" required because sizeof does not work for bit-fields.
>
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
> ---
> include/linux/compiler.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index 923d093..46fbda3 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -310,4 +310,6 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);
> */
> #define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))
>
> +#define unused_expression(e) ((void)(sizeof((__force long)(e))))
> +
hm, maybe.
Thing is, if anyone ever has an expression-with-side-effects within
conditionally-compiled code then they probably have a bug, don't they?
I mean, as an extreme example
VM_BUG_ON(do_something_important());
is a nice little hand-grenade. Your patch will cause that (bad) code
to newly fail at runtime, but our coverage testing is so awful that it
would take a long time for the bug to be discovered.
It would be nice if we could cause the build to warn or outright fail
if the unused_expression() argument would have caused any code
generation. But I can't suggest how to do that.
Your changelogs assert that gcc is emitting code for these expressions,
but details are not presented. Please give examples - where is this
code generation coming from, what is causing it?
Bottom line: are these patches a workaround for gcc inadequacies, or
are they a bandaid covering up poor kernel code?
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/4] bug: completely remove code of disabled VM_BUG_ON()
2012-04-25 14:40 ` Geert Uytterhoeven
@ 2012-04-26 22:32 ` Andrew Morton
2012-04-27 5:17 ` Geert Uytterhoeven
0 siblings, 1 reply; 24+ messages in thread
From: Andrew Morton @ 2012-04-26 22:32 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Konstantin Khlebnikov, Linus Torvalds, linux-kernel, linux-arch
On Wed, 25 Apr 2012 16:40:32 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Wed, Apr 25, 2012 at 13:26, Konstantin Khlebnikov
> <khlebnikov@openvz.org> wrote:
> > Even if CONFIG_DEBUG_VM=n gcc genereates code for some VM_BUG_ON()
>
> That's because of the side effects of the expression
>
> https://lkml.org/lkml/2012/4/25/146
>
> But IRIC, we do want them here?
>
AFAICT (lkml.org appears to be having a meltdown), you've gone and
linked to this very thread.
Please try again, this time avoiding hyperlinks ;)
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/4] compiler.h: introduce unused_expression() macro
2012-04-25 11:26 [PATCH 1/4] compiler.h: introduce unused_expression() macro Konstantin Khlebnikov
` (4 preceding siblings ...)
2012-04-26 22:29 ` Andrew Morton
@ 2012-04-26 22:34 ` Andrew Morton
2012-04-27 7:54 ` Konstantin Khlebnikov
2012-04-27 8:16 ` H. Peter Anvin
` (2 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Andrew Morton @ 2012-04-26 22:34 UTC (permalink / raw)
To: Konstantin Khlebnikov; +Cc: Linus Torvalds, linux-kernel, linux-arch
On Wed, 25 Apr 2012 15:26:23 +0400
Konstantin Khlebnikov <khlebnikov@openvz.org> wrote:
> Sometimes we want to check some expressions correctness in compile-time without
> generating extra code. "(void)(e)" does not work if expression has side-effects.
> This patch introduces macro unused_expression() which helps in this situation.
>
> Cast to "long" required because sizeof does not work for bit-fields.
>
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
> ---
> include/linux/compiler.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index 923d093..46fbda3 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -310,4 +310,6 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);
> */
> #define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))
>
> +#define unused_expression(e) ((void)(sizeof((__force long)(e))))
> +
btw, please let's not add undocumented stuff, particularly obscure
undocumented stuff.
I typed this in:
--- a/include/linux/compiler.h~compilerh-introduce-unused_expression-macro-fix
+++ a/include/linux/compiler.h
@@ -310,6 +310,11 @@ void ftrace_likely_update(struct ftrace_
*/
#define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))
+/*
+ * unused_expression() permits the compiler to check the validity of the
+ * expression but avoids the generation of any code, even if that expression
+ * has side-effects.
+ */
#define unused_expression(e) ((void)(sizeof((__force long)(e))))
#endif /* __LINUX_COMPILER_H */
but then decided I didn't want to apply the patches (yet?).
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/4] bug: completely remove code of disabled VM_BUG_ON()
2012-04-26 22:32 ` Andrew Morton
@ 2012-04-27 5:17 ` Geert Uytterhoeven
2012-04-27 7:07 ` Andrew Morton
0 siblings, 1 reply; 24+ messages in thread
From: Geert Uytterhoeven @ 2012-04-27 5:17 UTC (permalink / raw)
To: Andrew Morton
Cc: Konstantin Khlebnikov, Linus Torvalds, linux-kernel, linux-arch
On Fri, Apr 27, 2012 at 00:32, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Wed, 25 Apr 2012 16:40:32 +0200
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
>> On Wed, Apr 25, 2012 at 13:26, Konstantin Khlebnikov
>> <khlebnikov@openvz.org> wrote:
>> > Even if CONFIG_DEBUG_VM=n gcc genereates code for some VM_BUG_ON()
>>
>> That's because of the side effects of the expression
>>
>> https://lkml.org/lkml/2012/4/25/146
>>
>> But IRIC, we do want them here?
>>
>
> AFAICT (lkml.org appears to be having a meltdown), you've gone and
> linked to this very thread.
>
> Please try again, this time avoiding hyperlinks ;)
Yeah, I noticed after the fact. I wanted to look up the definition of
unused_expression(), which obviously wasn't in my tree yet ;-)
Still, I think people started relyong on the side effects, didn't they?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/4] bug: completely remove code of disabled VM_BUG_ON()
2012-04-27 5:17 ` Geert Uytterhoeven
@ 2012-04-27 7:07 ` Andrew Morton
0 siblings, 0 replies; 24+ messages in thread
From: Andrew Morton @ 2012-04-27 7:07 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Konstantin Khlebnikov, Linus Torvalds, linux-kernel, linux-arch
On Fri, 27 Apr 2012 07:17:50 +0200 Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Fri, Apr 27, 2012 at 00:32, Andrew Morton <akpm@linux-foundation.org> wrote:
> > On Wed, 25 Apr 2012 16:40:32 +0200
> > Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >
> >> On Wed, Apr 25, 2012 at 13:26, Konstantin Khlebnikov
> >> <khlebnikov@openvz.org> wrote:
> >> > Even if CONFIG_DEBUG_VM=n gcc genereates code for some VM_BUG_ON()
> >>
> >> That's because of the side effects of the expression
> >>
> >> https://lkml.org/lkml/2012/4/25/146
> >>
> >> But IRIC, we do want them here?
> >>
> >
> > AFAICT (lkml.org appears to be having a meltdown), you've gone and
> > linked to this very thread.
> >
> > Please try again, this time avoiding hyperlinks ;)
>
> Yeah, I noticed after the fact. I wanted to look up the definition of
> unused_expression(), which obviously wasn't in my tree yet ;-)
>
> Still, I think people started relyong on the side effects, didn't they?
I hope not. If they are, they snuck it past me cleverly! A quick grep
of mm/*.c looks clean.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/4] compiler.h: introduce unused_expression() macro
2012-04-26 22:34 ` Andrew Morton
@ 2012-04-27 7:54 ` Konstantin Khlebnikov
0 siblings, 0 replies; 24+ messages in thread
From: Konstantin Khlebnikov @ 2012-04-27 7:54 UTC (permalink / raw)
To: Andrew Morton; +Cc: Linus Torvalds, linux-kernel, linux-arch, Hugh Dickins
Andrew Morton wrote:
> On Wed, 25 Apr 2012 15:26:23 +0400
> Konstantin Khlebnikov<khlebnikov@openvz.org> wrote:
>
>> Sometimes we want to check some expressions correctness in compile-time without
>> generating extra code. "(void)(e)" does not work if expression has side-effects.
>> This patch introduces macro unused_expression() which helps in this situation.
>>
>> Cast to "long" required because sizeof does not work for bit-fields.
>>
>> Signed-off-by: Konstantin Khlebnikov<khlebnikov@openvz.org>
>> ---
>> include/linux/compiler.h | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
>> index 923d093..46fbda3 100644
>> --- a/include/linux/compiler.h
>> +++ b/include/linux/compiler.h
>> @@ -310,4 +310,6 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);
>> */
>> #define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))
>>
>> +#define unused_expression(e) ((void)(sizeof((__force long)(e))))
>> +
>
> btw, please let's not add undocumented stuff, particularly obscure
> undocumented stuff.
>
> I typed this in:
Thanks!
>
> --- a/include/linux/compiler.h~compilerh-introduce-unused_expression-macro-fix
> +++ a/include/linux/compiler.h
> @@ -310,6 +310,11 @@ void ftrace_likely_update(struct ftrace_
> */
> #define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))
>
> +/*
> + * unused_expression() permits the compiler to check the validity of the
> + * expression but avoids the generation of any code, even if that expression
> + * has side-effects.
> + */
> #define unused_expression(e) ((void)(sizeof((__force long)(e))))
>
> #endif /* __LINUX_COMPILER_H */
>
> but then decided I didn't want to apply the patches (yet?).
I'm OK with that. But probably Hugh Dickins (now in CC) aggressively wants to
remove side-effects of VM_BUG_ON's in case CONFIG_DEBUG_VM=n
> --
> 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/
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/4] compiler.h: introduce unused_expression() macro
2012-04-25 11:26 [PATCH 1/4] compiler.h: introduce unused_expression() macro Konstantin Khlebnikov
` (5 preceding siblings ...)
2012-04-26 22:34 ` Andrew Morton
@ 2012-04-27 8:16 ` H. Peter Anvin
2012-04-28 3:50 ` Konstantin Khlebnikov
2012-04-28 7:06 ` Konstantin Khlebnikov
2012-04-28 7:06 ` Konstantin Khlebnikov
8 siblings, 1 reply; 24+ messages in thread
From: H. Peter Anvin @ 2012-04-27 8:16 UTC (permalink / raw)
To: Konstantin Khlebnikov
Cc: Linus Torvalds, linux-kernel, linux-arch, Andrew Morton
On 04/25/2012 04:26 AM, Konstantin Khlebnikov wrote:
> Sometimes we want to check some expressions correctness in compile-time without
> generating extra code. "(void)(e)" does not work if expression has side-effects.
> This patch introduces macro unused_expression() which helps in this situation.
>
> Cast to "long" required because sizeof does not work for bit-fields.
>
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
> ---
> include/linux/compiler.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index 923d093..46fbda3 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -310,4 +310,6 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);
> */
> #define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))
>
> +#define unused_expression(e) ((void)(sizeof((__force long)(e))))
> +
OK, bikeshedding a bit:
"unused_expression()" doesn't sound like something that inhibits side
effects, especially since "unused" applied to an argument or variable
means "don't make a fuss over this not being used."
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/4] compiler.h: introduce unused_expression() macro
2012-04-26 22:29 ` Andrew Morton
@ 2012-04-27 9:55 ` Konstantin Khlebnikov
2012-04-27 21:53 ` Andrew Morton
0 siblings, 1 reply; 24+ messages in thread
From: Konstantin Khlebnikov @ 2012-04-27 9:55 UTC (permalink / raw)
To: Andrew Morton; +Cc: Linus Torvalds, linux-kernel, linux-arch
Andrew Morton wrote:
> On Wed, 25 Apr 2012 15:26:23 +0400
> Konstantin Khlebnikov<khlebnikov@openvz.org> wrote:
>
>> Sometimes we want to check some expressions correctness in compile-time without
>> generating extra code. "(void)(e)" does not work if expression has side-effects.
>> This patch introduces macro unused_expression() which helps in this situation.
>>
>> Cast to "long" required because sizeof does not work for bit-fields.
>>
>> Signed-off-by: Konstantin Khlebnikov<khlebnikov@openvz.org>
>> ---
>> include/linux/compiler.h | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
>> index 923d093..46fbda3 100644
>> --- a/include/linux/compiler.h
>> +++ b/include/linux/compiler.h
>> @@ -310,4 +310,6 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);
>> */
>> #define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))
>>
>> +#define unused_expression(e) ((void)(sizeof((__force long)(e))))
>> +
>
> hm, maybe.
>
> Thing is, if anyone ever has an expression-with-side-effects within
> conditionally-compiled code then they probably have a bug, don't they?
> I mean, as an extreme example
>
> VM_BUG_ON(do_something_important());
>
> is a nice little hand-grenade. Your patch will cause that (bad) code
> to newly fail at runtime, but our coverage testing is so awful that it
> would take a long time for the bug to be discovered.
>
> It would be nice if we could cause the build to warn or outright fail
> if the unused_expression() argument would have caused any code
> generation. But I can't suggest how to do that.
>
>
> Your changelogs assert that gcc is emitting code for these expressions,
> but details are not presented. Please give examples - where is this
> code generation coming from, what is causing it?
for example VM_BUG_ON(!PageCompound(page) || !PageHead(page)); in
do_huge_pmd_wp_page() generates 114 bytes.
But they mostly disappears if I replace it with
-VM_BUG_ON(!PageCompound(page) || !PageHead(page));
+VM_BUG_ON(!PageCompound(page));
+VM_BUG_ON(!PageHead(page));
weird...
add/remove: 0/0 grow/shrink: 7/97 up/down: 135/-1784 (-1649)
function old new delta
make_alloc_exact 160 210 +50
deactivate_slab 1302 1350 +48
get_page_from_freelist 2043 2059 +16
split_free_page 348 356 +8
add_to_swap 113 118 +5
__do_fault 1152 1157 +5
page_referenced_one 415 418 +3
lru_cache_add_lru 66 65 -1
static.get_partial_node 593 591 -2
static.copy_user_highpage 93 91 -2
unuse_mm 1559 1556 -3
unlock_page 49 46 -3
try_to_merge_with_ksm_page 1540 1537 -3
static.update_isolated_counts 335 332 -3
special_mapping_fault 130 127 -3
set_page_dirty_balance 98 95 -3
mem_cgroup_uncharge_cache_page 21 18 -3
mem_cgroup_newpage_charge 38 35 -3
add_page_to_unevictable_list 189 186 -3
__get_user_pages 1287 1284 -3
putback_lru_page 216 210 -6
follow_page 1020 1014 -6
__activate_page 375 369 -6
update_and_free_page 121 114 -7
try_to_munlock 72 65 -7
shmem_truncate_range 1586 1579 -7
put_page 56 49 -7
page_evictable 145 138 -7
copy_huge_pmd 411 404 -7
add_to_page_cache_locked 292 285 -7
__free_pages_bootmem 122 115 -7
zap_huge_pmd 244 236 -8
try_get_mem_cgroup_from_page 326 318 -8
static.__page_cache_release 277 269 -8
shmem_find_get_pages_and_swap 353 345 -8
shmem_file_aio_read 886 878 -8
shmem_add_to_page_cache 340 332 -8
reuse_swap_page 239 231 -8
new_page_node 100 92 -8
mem_cgroup_move_account 425 417 -8
ksm_migrate_page 69 61 -8
invalidate_inode_page 186 178 -8
hugetlb_cow 1145 1137 -8
get_page 50 42 -8
find_get_pages_tag 449 441 -8
find_get_pages 402 394 -8
enabled_show 184 176 -8
dequeue_huge_page_node 149 141 -8
defrag_show 184 176 -8
__alloc_pages_nodemask 2215 2207 -8
follow_trans_huge_pmd 149 140 -9
__delete_from_swap_cache 91 82 -9
swap_readpage 95 85 -10
static.get_mctgt_type_thp 178 167 -11
free_pages 74 63 -11
__pagevec_lru_add_fn 243 231 -12
new_node_page 57 43 -14
__put_anon_vma 161 147 -14
rmap_walk_ksm 321 305 -16
rmap_walk 575 559 -16
replace_page_cache_page 310 294 -16
remove_migration_pte 632 616 -16
release_pages 484 468 -16
page_add_new_anon_rmap 237 221 -16
move_active_pages_to_lru 382 366 -16
migrate_pages 1283 1267 -16
migrate_page_copy 469 453 -16
mem_cgroup_charge_common 169 153 -16
ksm_scan_thread 3164 3148 -16
follow_hugetlb_page 825 809 -16
find_get_pages_contig 431 415 -16
do_wp_page 1829 1813 -16
clear_page_dirty_for_io 269 253 -16
alloc_fresh_huge_page 254 238 -16
__mem_cgroup_try_charge 2425 2409 -16
__isolate_lru_page 213 197 -16
__add_to_swap_cache 203 187 -16
lru_add_page_tail 392 373 -19
__mem_cgroup_begin_update_page_stat 161 140 -21
remove_mapping 69 46 -23
__split_huge_page_pmd 180 157 -23
alloc_buddy_huge_page 333 309 -24
static.isolate_lru_pages 399 373 -26
split_page 101 73 -28
__remove_mapping 311 283 -28
__mem_cgroup_uncharge_common 787 757 -30
putback_inactive_pages 641 609 -32
do_page_add_anon_rmap 247 215 -32
__get_page_tail 274 242 -32
try_to_unmap 135 100 -35
mem_cgroup_prepare_migration 443 408 -35
new_slab 763 725 -38
hugetlb_acct_memory 827 786 -41
put_compound_page 343 301 -42
__rmqueue 1093 1050 -43
page_move_anon_rmap 71 26 -45
free_pcppages_bulk 956 911 -45
static.migrate_page_move_mapping 563 513 -50
migrate_huge_page_move_mapping 372 322 -50
free_one_page 819 769 -50
isolate_lru_page 383 326 -57
shrink_page_list 2313 2233 -80
khugepaged 5028 4947 -81
do_huge_pmd_wp_page 1747 1628 -119
>
>
> Bottom line: are these patches a workaround for gcc inadequacies, or
> are they a bandaid covering up poor kernel code?
>
gcc do weird things, but sometimes it really cannot throw out code.
I hope we can do this safely for VM_BUG_ON(), anyway nobody disables real BUG_ON()
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/4] compiler.h: introduce unused_expression() macro
2012-04-27 9:55 ` Konstantin Khlebnikov
@ 2012-04-27 21:53 ` Andrew Morton
0 siblings, 0 replies; 24+ messages in thread
From: Andrew Morton @ 2012-04-27 21:53 UTC (permalink / raw)
To: Konstantin Khlebnikov; +Cc: Linus Torvalds, linux-kernel, linux-arch
On Fri, 27 Apr 2012 13:55:06 +0400
Konstantin Khlebnikov <khlebnikov@openvz.org> wrote:
> Andrew Morton wrote:
> > On Wed, 25 Apr 2012 15:26:23 +0400
> > Konstantin Khlebnikov<khlebnikov@openvz.org> wrote:
>
> for example VM_BUG_ON(!PageCompound(page) || !PageHead(page)); in
> do_huge_pmd_wp_page() generates 114 bytes.
>
> But they mostly disappears if I replace it with
> -VM_BUG_ON(!PageCompound(page) || !PageHead(page));
> +VM_BUG_ON(!PageCompound(page));
> +VM_BUG_ON(!PageHead(page));
> weird...
>
> add/remove: 0/0 grow/shrink: 7/97 up/down: 135/-1784 (-1649)
OK, thanks. I'm inclined to apply the patchset as-is. If the
apparently mythical use of side-effects in VM_BUG_ON() really exist
then we deserve everything which happens to us as a result ;)
Please update the changelogs so they cover all the points which have been
discussed, add my little code comment then send out a v2, being sure to
cc everyone who has been involved in the discussion?
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/4] compiler.h: introduce unused_expression() macro
2012-04-27 8:16 ` H. Peter Anvin
@ 2012-04-28 3:50 ` Konstantin Khlebnikov
0 siblings, 0 replies; 24+ messages in thread
From: Konstantin Khlebnikov @ 2012-04-28 3:50 UTC (permalink / raw)
To: H. Peter Anvin; +Cc: Linus Torvalds, linux-kernel, linux-arch, Andrew Morton
H. Peter Anvin wrote:
> On 04/25/2012 04:26 AM, Konstantin Khlebnikov wrote:
>> Sometimes we want to check some expressions correctness in compile-time without
>> generating extra code. "(void)(e)" does not work if expression has side-effects.
>> This patch introduces macro unused_expression() which helps in this situation.
>>
>> Cast to "long" required because sizeof does not work for bit-fields.
>>
>> Signed-off-by: Konstantin Khlebnikov<khlebnikov@openvz.org>
>> ---
>> include/linux/compiler.h | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
>> index 923d093..46fbda3 100644
>> --- a/include/linux/compiler.h
>> +++ b/include/linux/compiler.h
>> @@ -310,4 +310,6 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);
>> */
>> #define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))
>>
>> +#define unused_expression(e) ((void)(sizeof((__force long)(e))))
>> +
>
> OK, bikeshedding a bit:
>
> "unused_expression()" doesn't sound like something that inhibits side
> effects, especially since "unused" applied to an argument or variable
> means "don't make a fuss over this not being used."
>
> -hpa
>
Ok, in v2 I'll rename it into BUILD_BUG_ON_INVALID()
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/4] bug: mark disabled BUG() as unreachable() code
2012-04-25 11:26 ` [PATCH 4/4] bug: mark disabled BUG() as unreachable() code Konstantin Khlebnikov
@ 2012-04-28 5:10 ` Konstantin Khlebnikov
2012-04-28 5:21 ` Linus Torvalds
2012-04-28 6:14 ` Andrew Morton
0 siblings, 2 replies; 24+ messages in thread
From: Konstantin Khlebnikov @ 2012-04-28 5:10 UTC (permalink / raw)
To: Linus Torvalds, linux-kernel; +Cc: linux-arch, Andrew Morton
Konstantin Khlebnikov wrote:
> This patch suppress some compiler warnings (if CONFIG_BUG=n)
> "warning: control reaches end of non-void function"
With this patch gcc throw out loop at the end of do_exit():
schedule();
BUG();
/* Avoid "noreturn function does return". */
for (;;)
cpu_relax(); /* For when BUG is null */
Is this ok? Probably not, and we need here some BUG_NORETURN() which
really never returns even if CONFIG_BUG=n.
>
> Plus now gcc can throw out some dead code.
> bloat-o-meter: add/remove: 1/1 grow/shrink: 68/173 up/down: 1569/-5607 (-4038)
>
> Signed-off-by: Konstantin Khlebnikov<khlebnikov@openvz.org>
> ---
> include/asm-generic/bug.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
> index aadb6fc..0b08199 100644
> --- a/include/asm-generic/bug.h
> +++ b/include/asm-generic/bug.h
> @@ -108,7 +108,7 @@ extern void warn_slowpath_null(const char *file, const int line);
>
> #else /* !CONFIG_BUG */
> #ifndef HAVE_ARCH_BUG
> -#define BUG() do {} while(0)
> +#define BUG() unreachable()
> #endif
>
> #ifndef HAVE_ARCH_BUG_ON
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/4] bug: mark disabled BUG() as unreachable() code
2012-04-28 5:10 ` Konstantin Khlebnikov
@ 2012-04-28 5:21 ` Linus Torvalds
2012-04-28 6:14 ` Andrew Morton
1 sibling, 0 replies; 24+ messages in thread
From: Linus Torvalds @ 2012-04-28 5:21 UTC (permalink / raw)
To: Konstantin Khlebnikov; +Cc: linux-kernel, linux-arch, Andrew Morton
On Fri, Apr 27, 2012 at 10:10 PM, Konstantin Khlebnikov
<khlebnikov@openvz.org> wrote:
>
> With this patch gcc throw out loop at the end of do_exit():
I think that's the least of our problems - that loop only exists to
get rid of a warning, and the return from that schedule() really is
supposed to be unreachable().
What makes me much more worried is that iirc, gcc will use
"unreachable()" to also get rid of function epilogues etc, which is
perfectly fine if it really is something entirely unreachable. But the
kernel use of BUG() is often as a kind of "assert()", and if gcc
generates actively bad code for it (and it does, with unreachable()),
it turns the turned-off BUG() into something *really* horrible that
makes for a debugging disaster.
So some pattern like
if (badness)
BUG();
would generate code that is actively insane, instead of (like now)
generating basically a no-op.
And THAT makes me go "Eww". "unreachable()" should only be used in
situations that really are very very unreachable() (eg after an "asm"
that goes off to la-la-land or a call to "exit()" in user land etc).
The kernel kinds of "assert" is *hopefully* not reachable, but if we
ever reach it, we don't want to make things worse. The BUG() was there
to give us a nicer debug message about what went wrong, and I think
your patch actively destroys that whole thing and makes for something
*worse*.
Linus
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/4] bug: mark disabled BUG() as unreachable() code
2012-04-28 5:10 ` Konstantin Khlebnikov
2012-04-28 5:21 ` Linus Torvalds
@ 2012-04-28 6:14 ` Andrew Morton
1 sibling, 0 replies; 24+ messages in thread
From: Andrew Morton @ 2012-04-28 6:14 UTC (permalink / raw)
To: Konstantin Khlebnikov; +Cc: Linus Torvalds, linux-kernel, linux-arch
On Sat, 28 Apr 2012 09:10:18 +0400 Konstantin Khlebnikov <khlebnikov@openvz.org> wrote:
> Konstantin Khlebnikov wrote:
> > This patch suppress some compiler warnings (if CONFIG_BUG=n)
> > "warning: control reaches end of non-void function"
>
> With this patch gcc throw out loop at the end of do_exit():
>
> schedule();
> BUG();
> /* Avoid "noreturn function does return". */
> for (;;)
> cpu_relax(); /* For when BUG is null */
>
> Is this ok? Probably not, and we need here some BUG_NORETURN() which
> really never returns even if CONFIG_BUG=n.
Probably we should do away with CONFIG_BUG.
CONFIG_BUG=n causes various compile-time warnings to come out when the
execution proceeds into places where we didn't intend and these aren't
worth fixing.
More seriously, I rather doubt that anyone ever sets it to n. And it
would be a pretty dumb thing to do - if your kernel has seriously
malfunctioned and it *knows* that it malfunctioned, it will just
blunder on anyway not telling anyone about it.
It doesn't seem worth any effort to support this thing.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 1/2] bug: introduce BUILD_BUG_ON_INVALID() macro
2012-04-25 11:26 [PATCH 1/4] compiler.h: introduce unused_expression() macro Konstantin Khlebnikov
@ 2012-04-28 7:06 ` Konstantin Khlebnikov
2012-04-25 11:26 ` [PATCH 3/4] bug: completely remove code of disabled BUG_ON() Konstantin Khlebnikov
` (7 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: Konstantin Khlebnikov @ 2012-04-28 7:06 UTC (permalink / raw)
To: Andrew Morton, Linus Torvalds
Cc: linux-arch, linux-kernel, linux-mm, Geert Uytterhoeven,
H. Peter Anvin, Cong Wang
Sometimes we want to check some expressions correctness in compile-time.
"(void)(e);" or "if (e);" can be dangerous if expression has side-effects, and
gcc sometimes generates a lot of code, even if the expression has no effect.
This patch introduces macro BUILD_BUG_ON_INVALID() for such checks,
it forces a compilation error if expression is invalid without any extra code.
[Cast to "long" required because sizeof does not work for bit-fields.]
Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
---
include/linux/bug.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/include/linux/bug.h b/include/linux/bug.h
index 72961c3..aaac4bb 100644
--- a/include/linux/bug.h
+++ b/include/linux/bug.h
@@ -30,6 +30,13 @@ struct pt_regs;
#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
#define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
+/*
+ * BUILD_BUG_ON_INVALID() permits the compiler to check the validity of the
+ * expression but avoids the generation of any code, even if that expression
+ * has side-effects.
+ */
+#define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e))))
+
/**
* BUILD_BUG_ON - break compile if a condition is true.
* @condition: the condition which the compiler should know is false.
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 1/2] bug: introduce BUILD_BUG_ON_INVALID() macro
@ 2012-04-28 7:06 ` Konstantin Khlebnikov
0 siblings, 0 replies; 24+ messages in thread
From: Konstantin Khlebnikov @ 2012-04-28 7:06 UTC (permalink / raw)
To: Andrew Morton, Linus Torvalds
Cc: linux-arch, linux-kernel, linux-mm, Geert Uytterhoeven,
H. Peter Anvin, Cong Wang
Sometimes we want to check some expressions correctness in compile-time.
"(void)(e);" or "if (e);" can be dangerous if expression has side-effects, and
gcc sometimes generates a lot of code, even if the expression has no effect.
This patch introduces macro BUILD_BUG_ON_INVALID() for such checks,
it forces a compilation error if expression is invalid without any extra code.
[Cast to "long" required because sizeof does not work for bit-fields.]
Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
---
include/linux/bug.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/include/linux/bug.h b/include/linux/bug.h
index 72961c3..aaac4bb 100644
--- a/include/linux/bug.h
+++ b/include/linux/bug.h
@@ -30,6 +30,13 @@ struct pt_regs;
#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
#define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
+/*
+ * BUILD_BUG_ON_INVALID() permits the compiler to check the validity of the
+ * expression but avoids the generation of any code, even if that expression
+ * has side-effects.
+ */
+#define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e))))
+
/**
* BUILD_BUG_ON - break compile if a condition is true.
* @condition: the condition which the compiler should know is false.
--
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>
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 2/2] bug: completely remove code of disabled VM_BUG_ON()
2012-04-25 11:26 [PATCH 1/4] compiler.h: introduce unused_expression() macro Konstantin Khlebnikov
@ 2012-04-28 7:06 ` Konstantin Khlebnikov
2012-04-25 11:26 ` [PATCH 3/4] bug: completely remove code of disabled BUG_ON() Konstantin Khlebnikov
` (7 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: Konstantin Khlebnikov @ 2012-04-28 7:06 UTC (permalink / raw)
To: Andrew Morton, Linus Torvalds
Cc: linux-arch, linux-kernel, linux-mm, Geert Uytterhoeven,
H. Peter Anvin, Cong Wang
Even if CONFIG_DEBUG_VM=n gcc genereates code for some VM_BUG_ON()
for example VM_BUG_ON(!PageCompound(page) || !PageHead(page)); in
do_huge_pmd_wp_page() generates 114 bytes of code.
But they mostly disappears when I split this VM_BUG_ON into two:
-VM_BUG_ON(!PageCompound(page) || !PageHead(page));
+VM_BUG_ON(!PageCompound(page));
+VM_BUG_ON(!PageHead(page));
weird... but anyway after this patch code disappears completely.
add/remove: 0/0 grow/shrink: 7/97 up/down: 135/-1784 (-1649)
Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
---
add/remove: 0/0 grow/shrink: 7/97 up/down: 135/-1784 (-1649)
function old new delta
make_alloc_exact 160 210 +50
deactivate_slab 1302 1350 +48
get_page_from_freelist 2043 2059 +16
split_free_page 348 356 +8
add_to_swap 113 118 +5
__do_fault 1152 1157 +5
page_referenced_one 415 418 +3
lru_cache_add_lru 66 65 -1
static.get_partial_node 593 591 -2
static.copy_user_highpage 93 91 -2
unuse_mm 1559 1556 -3
unlock_page 49 46 -3
try_to_merge_with_ksm_page 1540 1537 -3
static.update_isolated_counts 335 332 -3
special_mapping_fault 130 127 -3
set_page_dirty_balance 98 95 -3
mem_cgroup_uncharge_cache_page 21 18 -3
mem_cgroup_newpage_charge 38 35 -3
add_page_to_unevictable_list 189 186 -3
__get_user_pages 1287 1284 -3
putback_lru_page 216 210 -6
follow_page 1020 1014 -6
__activate_page 375 369 -6
update_and_free_page 121 114 -7
try_to_munlock 72 65 -7
shmem_truncate_range 1586 1579 -7
put_page 56 49 -7
page_evictable 145 138 -7
copy_huge_pmd 411 404 -7
add_to_page_cache_locked 292 285 -7
__free_pages_bootmem 122 115 -7
zap_huge_pmd 244 236 -8
try_get_mem_cgroup_from_page 326 318 -8
static.__page_cache_release 277 269 -8
shmem_find_get_pages_and_swap 353 345 -8
shmem_file_aio_read 886 878 -8
shmem_add_to_page_cache 340 332 -8
reuse_swap_page 239 231 -8
new_page_node 100 92 -8
mem_cgroup_move_account 425 417 -8
ksm_migrate_page 69 61 -8
invalidate_inode_page 186 178 -8
hugetlb_cow 1145 1137 -8
get_page 50 42 -8
find_get_pages_tag 449 441 -8
find_get_pages 402 394 -8
enabled_show 184 176 -8
dequeue_huge_page_node 149 141 -8
defrag_show 184 176 -8
__alloc_pages_nodemask 2215 2207 -8
follow_trans_huge_pmd 149 140 -9
__delete_from_swap_cache 91 82 -9
swap_readpage 95 85 -10
static.get_mctgt_type_thp 178 167 -11
free_pages 74 63 -11
__pagevec_lru_add_fn 243 231 -12
new_node_page 57 43 -14
__put_anon_vma 161 147 -14
rmap_walk_ksm 321 305 -16
rmap_walk 575 559 -16
replace_page_cache_page 310 294 -16
remove_migration_pte 632 616 -16
release_pages 484 468 -16
page_add_new_anon_rmap 237 221 -16
move_active_pages_to_lru 382 366 -16
migrate_pages 1283 1267 -16
migrate_page_copy 469 453 -16
mem_cgroup_charge_common 169 153 -16
ksm_scan_thread 3164 3148 -16
follow_hugetlb_page 825 809 -16
find_get_pages_contig 431 415 -16
do_wp_page 1829 1813 -16
clear_page_dirty_for_io 269 253 -16
alloc_fresh_huge_page 254 238 -16
__mem_cgroup_try_charge 2425 2409 -16
__isolate_lru_page 213 197 -16
__add_to_swap_cache 203 187 -16
lru_add_page_tail 392 373 -19
__mem_cgroup_begin_update_page_stat 161 140 -21
remove_mapping 69 46 -23
__split_huge_page_pmd 180 157 -23
alloc_buddy_huge_page 333 309 -24
static.isolate_lru_pages 399 373 -26
split_page 101 73 -28
__remove_mapping 311 283 -28
__mem_cgroup_uncharge_common 787 757 -30
putback_inactive_pages 641 609 -32
do_page_add_anon_rmap 247 215 -32
__get_page_tail 274 242 -32
try_to_unmap 135 100 -35
mem_cgroup_prepare_migration 443 408 -35
new_slab 763 725 -38
hugetlb_acct_memory 827 786 -41
put_compound_page 343 301 -42
__rmqueue 1093 1050 -43
page_move_anon_rmap 71 26 -45
free_pcppages_bulk 956 911 -45
static.migrate_page_move_mapping 563 513 -50
migrate_huge_page_move_mapping 372 322 -50
free_one_page 819 769 -50
isolate_lru_page 383 326 -57
shrink_page_list 2313 2233 -80
khugepaged 5028 4947 -81
do_huge_pmd_wp_page 1747 1628 -119
---
include/linux/mmdebug.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/mmdebug.h b/include/linux/mmdebug.h
index c04ecfe..580bd58 100644
--- a/include/linux/mmdebug.h
+++ b/include/linux/mmdebug.h
@@ -4,7 +4,7 @@
#ifdef CONFIG_DEBUG_VM
#define VM_BUG_ON(cond) BUG_ON(cond)
#else
-#define VM_BUG_ON(cond) do { (void)(cond); } while (0)
+#define VM_BUG_ON(cond) BUILD_BUG_ON_INVALID(cond)
#endif
#ifdef CONFIG_DEBUG_VIRTUAL
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 2/2] bug: completely remove code of disabled VM_BUG_ON()
@ 2012-04-28 7:06 ` Konstantin Khlebnikov
0 siblings, 0 replies; 24+ messages in thread
From: Konstantin Khlebnikov @ 2012-04-28 7:06 UTC (permalink / raw)
To: Andrew Morton, Linus Torvalds
Cc: linux-arch, linux-kernel, linux-mm, Geert Uytterhoeven,
H. Peter Anvin, Cong Wang
Even if CONFIG_DEBUG_VM=n gcc genereates code for some VM_BUG_ON()
for example VM_BUG_ON(!PageCompound(page) || !PageHead(page)); in
do_huge_pmd_wp_page() generates 114 bytes of code.
But they mostly disappears when I split this VM_BUG_ON into two:
-VM_BUG_ON(!PageCompound(page) || !PageHead(page));
+VM_BUG_ON(!PageCompound(page));
+VM_BUG_ON(!PageHead(page));
weird... but anyway after this patch code disappears completely.
add/remove: 0/0 grow/shrink: 7/97 up/down: 135/-1784 (-1649)
Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
---
add/remove: 0/0 grow/shrink: 7/97 up/down: 135/-1784 (-1649)
function old new delta
make_alloc_exact 160 210 +50
deactivate_slab 1302 1350 +48
get_page_from_freelist 2043 2059 +16
split_free_page 348 356 +8
add_to_swap 113 118 +5
__do_fault 1152 1157 +5
page_referenced_one 415 418 +3
lru_cache_add_lru 66 65 -1
static.get_partial_node 593 591 -2
static.copy_user_highpage 93 91 -2
unuse_mm 1559 1556 -3
unlock_page 49 46 -3
try_to_merge_with_ksm_page 1540 1537 -3
static.update_isolated_counts 335 332 -3
special_mapping_fault 130 127 -3
set_page_dirty_balance 98 95 -3
mem_cgroup_uncharge_cache_page 21 18 -3
mem_cgroup_newpage_charge 38 35 -3
add_page_to_unevictable_list 189 186 -3
__get_user_pages 1287 1284 -3
putback_lru_page 216 210 -6
follow_page 1020 1014 -6
__activate_page 375 369 -6
update_and_free_page 121 114 -7
try_to_munlock 72 65 -7
shmem_truncate_range 1586 1579 -7
put_page 56 49 -7
page_evictable 145 138 -7
copy_huge_pmd 411 404 -7
add_to_page_cache_locked 292 285 -7
__free_pages_bootmem 122 115 -7
zap_huge_pmd 244 236 -8
try_get_mem_cgroup_from_page 326 318 -8
static.__page_cache_release 277 269 -8
shmem_find_get_pages_and_swap 353 345 -8
shmem_file_aio_read 886 878 -8
shmem_add_to_page_cache 340 332 -8
reuse_swap_page 239 231 -8
new_page_node 100 92 -8
mem_cgroup_move_account 425 417 -8
ksm_migrate_page 69 61 -8
invalidate_inode_page 186 178 -8
hugetlb_cow 1145 1137 -8
get_page 50 42 -8
find_get_pages_tag 449 441 -8
find_get_pages 402 394 -8
enabled_show 184 176 -8
dequeue_huge_page_node 149 141 -8
defrag_show 184 176 -8
__alloc_pages_nodemask 2215 2207 -8
follow_trans_huge_pmd 149 140 -9
__delete_from_swap_cache 91 82 -9
swap_readpage 95 85 -10
static.get_mctgt_type_thp 178 167 -11
free_pages 74 63 -11
__pagevec_lru_add_fn 243 231 -12
new_node_page 57 43 -14
__put_anon_vma 161 147 -14
rmap_walk_ksm 321 305 -16
rmap_walk 575 559 -16
replace_page_cache_page 310 294 -16
remove_migration_pte 632 616 -16
release_pages 484 468 -16
page_add_new_anon_rmap 237 221 -16
move_active_pages_to_lru 382 366 -16
migrate_pages 1283 1267 -16
migrate_page_copy 469 453 -16
mem_cgroup_charge_common 169 153 -16
ksm_scan_thread 3164 3148 -16
follow_hugetlb_page 825 809 -16
find_get_pages_contig 431 415 -16
do_wp_page 1829 1813 -16
clear_page_dirty_for_io 269 253 -16
alloc_fresh_huge_page 254 238 -16
__mem_cgroup_try_charge 2425 2409 -16
__isolate_lru_page 213 197 -16
__add_to_swap_cache 203 187 -16
lru_add_page_tail 392 373 -19
__mem_cgroup_begin_update_page_stat 161 140 -21
remove_mapping 69 46 -23
__split_huge_page_pmd 180 157 -23
alloc_buddy_huge_page 333 309 -24
static.isolate_lru_pages 399 373 -26
split_page 101 73 -28
__remove_mapping 311 283 -28
__mem_cgroup_uncharge_common 787 757 -30
putback_inactive_pages 641 609 -32
do_page_add_anon_rmap 247 215 -32
__get_page_tail 274 242 -32
try_to_unmap 135 100 -35
mem_cgroup_prepare_migration 443 408 -35
new_slab 763 725 -38
hugetlb_acct_memory 827 786 -41
put_compound_page 343 301 -42
__rmqueue 1093 1050 -43
page_move_anon_rmap 71 26 -45
free_pcppages_bulk 956 911 -45
static.migrate_page_move_mapping 563 513 -50
migrate_huge_page_move_mapping 372 322 -50
free_one_page 819 769 -50
isolate_lru_page 383 326 -57
shrink_page_list 2313 2233 -80
khugepaged 5028 4947 -81
do_huge_pmd_wp_page 1747 1628 -119
---
include/linux/mmdebug.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/mmdebug.h b/include/linux/mmdebug.h
index c04ecfe..580bd58 100644
--- a/include/linux/mmdebug.h
+++ b/include/linux/mmdebug.h
@@ -4,7 +4,7 @@
#ifdef CONFIG_DEBUG_VM
#define VM_BUG_ON(cond) BUG_ON(cond)
#else
-#define VM_BUG_ON(cond) do { (void)(cond); } while (0)
+#define VM_BUG_ON(cond) BUILD_BUG_ON_INVALID(cond)
#endif
#ifdef CONFIG_DEBUG_VIRTUAL
--
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>
^ permalink raw reply related [flat|nested] 24+ messages in thread
end of thread, other threads:[~2012-04-28 7:08 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-25 11:26 [PATCH 1/4] compiler.h: introduce unused_expression() macro Konstantin Khlebnikov
2012-04-25 11:26 ` [PATCH 2/4] bug: completely remove code of disabled VM_BUG_ON() Konstantin Khlebnikov
2012-04-25 14:40 ` Geert Uytterhoeven
2012-04-26 22:32 ` Andrew Morton
2012-04-27 5:17 ` Geert Uytterhoeven
2012-04-27 7:07 ` Andrew Morton
2012-04-25 11:26 ` [PATCH 3/4] bug: completely remove code of disabled BUG_ON() Konstantin Khlebnikov
2012-04-25 11:26 ` [PATCH 4/4] bug: mark disabled BUG() as unreachable() code Konstantin Khlebnikov
2012-04-28 5:10 ` Konstantin Khlebnikov
2012-04-28 5:21 ` Linus Torvalds
2012-04-28 6:14 ` Andrew Morton
2012-04-25 11:51 ` [PATCH 1/4] compiler.h: introduce unused_expression() macro Cong Wang
2012-04-25 11:54 ` Konstantin Khlebnikov
2012-04-26 22:29 ` Andrew Morton
2012-04-27 9:55 ` Konstantin Khlebnikov
2012-04-27 21:53 ` Andrew Morton
2012-04-26 22:34 ` Andrew Morton
2012-04-27 7:54 ` Konstantin Khlebnikov
2012-04-27 8:16 ` H. Peter Anvin
2012-04-28 3:50 ` Konstantin Khlebnikov
2012-04-28 7:06 ` [PATCH v2 1/2] bug: introduce BUILD_BUG_ON_INVALID() macro Konstantin Khlebnikov
2012-04-28 7:06 ` Konstantin Khlebnikov
2012-04-28 7:06 ` [PATCH v2 2/2] bug: completely remove code of disabled VM_BUG_ON() Konstantin Khlebnikov
2012-04-28 7:06 ` Konstantin Khlebnikov
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.