* [PATCH 2/7] Add assertion checking macros
2011-10-12 16:47 [PATCH 1/7] Add assertion support with annotated oopsing David Howells
@ 2011-10-12 16:47 ` David Howells
2011-10-12 16:47 ` David Howells
` (2 more replies)
2011-10-12 16:47 ` [PATCH 3/7] Make shrink_dcache_for_umount_subtree() use the core assertion code David Howells
` (6 subsequent siblings)
7 siblings, 3 replies; 20+ messages in thread
From: David Howells @ 2011-10-12 16:47 UTC (permalink / raw)
To: linux-arch; +Cc: dhowells, linux-kernel
Add a range of ASSERT* macros to linux/assert.h for performing runtime
assertions. These will use assertion_failure() to cause an annotated oops if
the check fails.
The checks are only enabled under two circumstances:
(1) CONFIG_DEBUG_ENABLE_ASSERTIONS=y
(2) ENABLE_ASSERTIONS is defined prior to the #inclusion of <linux/assert.h>
There are five macros provided:
(a) ASSERT(X)
Issue an assertion failure error if X is false. In other words, require
the expression X to be true. For example:
ASSERT(val != 0);
There is no need to display val here in the case the expression fails
since it can only be 0. If this fails, it produces an error like the
following:
------------[ cut here ]------------
ASSERTION FAILED at fs/fscache/main.c:109!
invalid opcode: 0000 [#1] SMP
(b) ASSERTCMP(X, OP, Y)
Issue an assertion failure error if the expression X OP Y is false. For
example:
ASSERTCMP(x, >, 12)
If an oops is produced, then the values of X and Y will be displayed in
hex, along with OP:
------------[ cut here ]------------
ASSERTION FAILED at fs/fscache/main.c:109!
Check 2 > c is false
invalid opcode: 0000 [#1] SMP
(c) ASSERTRANGE(X, OP, Y, OP2, Z)
Issue an assertion failure error if the expression X OP Y or if the
expression Y OP2 Z is false. Typically OP and OP2 would be < or <=,
looking something like:
ASSERTRANGE(11, <, x, <=, 13);
and giving the following error:
------------[ cut here ]------------
ASSERTION FAILED at fs/fscache/main.c:109!
Check b < 2 <= d is false
invalid opcode: 0000 [#1] SMP
and for compactness, where an assertion should only apply under certain
circumstances:
(d) ASSERTIF(C, X)
If condition C is true, issue an assertion failure error if X is false.
For example:
ASSERTIF(test_bit(FSCACHE_OP_EXCLUSIVE, &op->flags),
object->n_exclusive != 0);
(e) ASSERTIFCMP(C, X, OP, Y)
This is a combination of ASSERTIF and ASSERTCMP. For example:
ASSERTIFCMP(test_bit(FSCACHE_OP_EXCLUSIVE, &op->flags),
object->n_exclusive, >, 0);
Signed-off-by: David Howells <dhowells@redhat.com>
---
include/linux/assert.h | 94 ++++++++++++++++++++++++++++++++++++++++++++++++
lib/Kconfig.debug | 8 ++++
2 files changed, 102 insertions(+), 0 deletions(-)
diff --git a/include/linux/assert.h b/include/linux/assert.h
index 739ebf7..a65d1a8 100644
--- a/include/linux/assert.h
+++ b/include/linux/assert.h
@@ -33,4 +33,98 @@ void do_assertion_failed(const char *file, int line, const char *fmt, ...)
do_assertion_failed(__FILE__, __LINE__, FMT, ## __VA_ARGS__); \
} while (0)
+/*
+ * ENABLE_ASSERTIONS can be set by an individual module to override the global
+ * setting and turn assertions on for just that module.
+ */
+#if defined(CONFIG_DEBUG_ENABLE_ASSERTIONS) || defined(ENABLE_ASSERTIONS)
+
+#define cond_assertion_failed(FMT, ...) \
+ do { \
+ do_assertion_failed(__FILE__, __LINE__, FMT, ## __VA_ARGS__); \
+ } while (0)
+
+#else
+
+#define cond_assertion_failed(FMT, ...) \
+ do { \
+ no_printk(FMT, ## __VA_ARGS__); \
+ } while (0)
+
+#endif
+
+/**
+ * ASSERT - Oops if the given expression is not true
+ * X: The expression to check
+ */
+#define ASSERT(X) \
+do { \
+ if (unlikely(!(X))) \
+ cond_assertion_failed(NULL); \
+} while (0)
+
+/**
+ * ASSERTCMP - Oops if the specified check fails
+ * X: The value to check
+ * OP: The operator to use for comparison
+ * Y: The value to compare against
+ *
+ * The two values are displayed in the oops report if the assertion fails.
+ */
+#define ASSERTCMP(X, OP, Y) \
+do { \
+ if (unlikely(!((X) OP (Y)))) \
+ cond_assertion_failed("Check %lx " #OP " %lx is false\n", \
+ (unsigned long)(X), \
+ (unsigned long)(Y)); \
+} while (0)
+
+/**
+ * ASSERTIF - If condition is true, oops if the given expression is not true
+ * C: The condition under which to perform the check
+ * X: The expression to check
+ */
+#define ASSERTIF(C, X) \
+do { \
+ if (unlikely((C) && !(X))) \
+ cond_assertion_failed(NULL); \
+} while (0)
+
+/**
+ * ASSERTIFCMP - If condition is true, oops if the specified check fails
+ * C: The condition under which to perform the check
+ * X: The value to check
+ * OP: The operator to use for comparison
+ * Y: The value to compare against
+ *
+ * The two values are displayed in the oops report if the assertion fails.
+ */
+#define ASSERTIFCMP(C, X, OP, Y) \
+do { \
+ if (unlikely((C) && !((X) OP (Y)))) \
+ cond_assertion_failed("Check %lx " #OP " %lx is false\n", \
+ (unsigned long)(X), \
+ (unsigned long)(Y)); \
+} while (0)
+
+/**
+ * ASSERTCMP - Oops if the value is outside of the specified range
+ * X: The lower bound
+ * OP: The operator to use to check against the lower bound (< or <=)
+ * Y: The value to check
+ * OP2: The operator to use to check against the upper bound (< or <=)
+ * Z: The upper bound
+ *
+ * The three values are displayed in the oops report if the assertion fails.
+ */
+#define ASSERTRANGE(X, OP, Y, OP2, Z) \
+do { \
+ if (unlikely(!((X) OP (Y)) || !((Y) OP2 (Z)))) \
+ cond_assertion_failed("Check %lx " #OP " %lx " #OP2 \
+ " %lx is false\n", \
+ (unsigned long)(X), \
+ (unsigned long)(Y), \
+ (unsigned long)(Z)); \
+} while(0)
+
#endif /* _LINUX_ASSERT_H */
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index c0cb9c4..604eede9 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -149,6 +149,14 @@ config DEBUG_KERNEL
Say Y here if you are developing drivers or trying to debug and
identify kernel problems.
+config DEBUG_ENABLE_ASSERTIONS
+ bool "Enable assertion checks"
+ depends on BUG
+ help
+ Say Y here to globally enable checks made by the ASSERT*() macros.
+ If such a check fails, BUG() processing will be invoked and an
+ annotated oops will be emitted.
+
config DEBUG_SHIRQ
bool "Debug shared IRQ handlers"
depends on DEBUG_KERNEL && GENERIC_HARDIRQS
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 2/7] Add assertion checking macros
2011-10-12 16:47 ` [PATCH 2/7] Add assertion checking macros David Howells
@ 2011-10-12 16:47 ` David Howells
2011-10-12 17:43 ` Jiri Slaby
2011-10-12 20:36 ` David Howells
2 siblings, 0 replies; 20+ messages in thread
From: David Howells @ 2011-10-12 16:47 UTC (permalink / raw)
To: linux-arch; +Cc: dhowells, linux-kernel
Add a range of ASSERT* macros to linux/assert.h for performing runtime
assertions. These will use assertion_failure() to cause an annotated oops if
the check fails.
The checks are only enabled under two circumstances:
(1) CONFIG_DEBUG_ENABLE_ASSERTIONS=y
(2) ENABLE_ASSERTIONS is defined prior to the #inclusion of <linux/assert.h>
There are five macros provided:
(a) ASSERT(X)
Issue an assertion failure error if X is false. In other words, require
the expression X to be true. For example:
ASSERT(val != 0);
There is no need to display val here in the case the expression fails
since it can only be 0. If this fails, it produces an error like the
following:
------------[ cut here ]------------
ASSERTION FAILED at fs/fscache/main.c:109!
invalid opcode: 0000 [#1] SMP
(b) ASSERTCMP(X, OP, Y)
Issue an assertion failure error if the expression X OP Y is false. For
example:
ASSERTCMP(x, >, 12)
If an oops is produced, then the values of X and Y will be displayed in
hex, along with OP:
------------[ cut here ]------------
ASSERTION FAILED at fs/fscache/main.c:109!
Check 2 > c is false
invalid opcode: 0000 [#1] SMP
(c) ASSERTRANGE(X, OP, Y, OP2, Z)
Issue an assertion failure error if the expression X OP Y or if the
expression Y OP2 Z is false. Typically OP and OP2 would be < or <=,
looking something like:
ASSERTRANGE(11, <, x, <=, 13);
and giving the following error:
------------[ cut here ]------------
ASSERTION FAILED at fs/fscache/main.c:109!
Check b < 2 <= d is false
invalid opcode: 0000 [#1] SMP
and for compactness, where an assertion should only apply under certain
circumstances:
(d) ASSERTIF(C, X)
If condition C is true, issue an assertion failure error if X is false.
For example:
ASSERTIF(test_bit(FSCACHE_OP_EXCLUSIVE, &op->flags),
object->n_exclusive != 0);
(e) ASSERTIFCMP(C, X, OP, Y)
This is a combination of ASSERTIF and ASSERTCMP. For example:
ASSERTIFCMP(test_bit(FSCACHE_OP_EXCLUSIVE, &op->flags),
object->n_exclusive, >, 0);
Signed-off-by: David Howells <dhowells@redhat.com>
---
include/linux/assert.h | 94 ++++++++++++++++++++++++++++++++++++++++++++++++
lib/Kconfig.debug | 8 ++++
2 files changed, 102 insertions(+), 0 deletions(-)
diff --git a/include/linux/assert.h b/include/linux/assert.h
index 739ebf7..a65d1a8 100644
--- a/include/linux/assert.h
+++ b/include/linux/assert.h
@@ -33,4 +33,98 @@ void do_assertion_failed(const char *file, int line, const char *fmt, ...)
do_assertion_failed(__FILE__, __LINE__, FMT, ## __VA_ARGS__); \
} while (0)
+/*
+ * ENABLE_ASSERTIONS can be set by an individual module to override the global
+ * setting and turn assertions on for just that module.
+ */
+#if defined(CONFIG_DEBUG_ENABLE_ASSERTIONS) || defined(ENABLE_ASSERTIONS)
+
+#define cond_assertion_failed(FMT, ...) \
+ do { \
+ do_assertion_failed(__FILE__, __LINE__, FMT, ## __VA_ARGS__); \
+ } while (0)
+
+#else
+
+#define cond_assertion_failed(FMT, ...) \
+ do { \
+ no_printk(FMT, ## __VA_ARGS__); \
+ } while (0)
+
+#endif
+
+/**
+ * ASSERT - Oops if the given expression is not true
+ * X: The expression to check
+ */
+#define ASSERT(X) \
+do { \
+ if (unlikely(!(X))) \
+ cond_assertion_failed(NULL); \
+} while (0)
+
+/**
+ * ASSERTCMP - Oops if the specified check fails
+ * X: The value to check
+ * OP: The operator to use for comparison
+ * Y: The value to compare against
+ *
+ * The two values are displayed in the oops report if the assertion fails.
+ */
+#define ASSERTCMP(X, OP, Y) \
+do { \
+ if (unlikely(!((X) OP (Y)))) \
+ cond_assertion_failed("Check %lx " #OP " %lx is false\n", \
+ (unsigned long)(X), \
+ (unsigned long)(Y)); \
+} while (0)
+
+/**
+ * ASSERTIF - If condition is true, oops if the given expression is not true
+ * C: The condition under which to perform the check
+ * X: The expression to check
+ */
+#define ASSERTIF(C, X) \
+do { \
+ if (unlikely((C) && !(X))) \
+ cond_assertion_failed(NULL); \
+} while (0)
+
+/**
+ * ASSERTIFCMP - If condition is true, oops if the specified check fails
+ * C: The condition under which to perform the check
+ * X: The value to check
+ * OP: The operator to use for comparison
+ * Y: The value to compare against
+ *
+ * The two values are displayed in the oops report if the assertion fails.
+ */
+#define ASSERTIFCMP(C, X, OP, Y) \
+do { \
+ if (unlikely((C) && !((X) OP (Y)))) \
+ cond_assertion_failed("Check %lx " #OP " %lx is false\n", \
+ (unsigned long)(X), \
+ (unsigned long)(Y)); \
+} while (0)
+
+/**
+ * ASSERTCMP - Oops if the value is outside of the specified range
+ * X: The lower bound
+ * OP: The operator to use to check against the lower bound (< or <=)
+ * Y: The value to check
+ * OP2: The operator to use to check against the upper bound (< or <=)
+ * Z: The upper bound
+ *
+ * The three values are displayed in the oops report if the assertion fails.
+ */
+#define ASSERTRANGE(X, OP, Y, OP2, Z) \
+do { \
+ if (unlikely(!((X) OP (Y)) || !((Y) OP2 (Z)))) \
+ cond_assertion_failed("Check %lx " #OP " %lx " #OP2 \
+ " %lx is false\n", \
+ (unsigned long)(X), \
+ (unsigned long)(Y), \
+ (unsigned long)(Z)); \
+} while(0)
+
#endif /* _LINUX_ASSERT_H */
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index c0cb9c4..604eede9 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -149,6 +149,14 @@ config DEBUG_KERNEL
Say Y here if you are developing drivers or trying to debug and
identify kernel problems.
+config DEBUG_ENABLE_ASSERTIONS
+ bool "Enable assertion checks"
+ depends on BUG
+ help
+ Say Y here to globally enable checks made by the ASSERT*() macros.
+ If such a check fails, BUG() processing will be invoked and an
+ annotated oops will be emitted.
+
config DEBUG_SHIRQ
bool "Debug shared IRQ handlers"
depends on DEBUG_KERNEL && GENERIC_HARDIRQS
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 2/7] Add assertion checking macros
2011-10-12 16:47 ` [PATCH 2/7] Add assertion checking macros David Howells
2011-10-12 16:47 ` David Howells
@ 2011-10-12 17:43 ` Jiri Slaby
2011-10-12 20:36 ` David Howells
2 siblings, 0 replies; 20+ messages in thread
From: Jiri Slaby @ 2011-10-12 17:43 UTC (permalink / raw)
To: David Howells; +Cc: linux-arch, linux-kernel
On 10/12/2011 06:47 PM, David Howells wrote:
> Add a range of ASSERT* macros to linux/assert.h for performing runtime
> assertions. These will use assertion_failure() to cause an annotated oops if
> the check fails.
>
> The checks are only enabled under two circumstances:
>
> (1) CONFIG_DEBUG_ENABLE_ASSERTIONS=y
>
> (2) ENABLE_ASSERTIONS is defined prior to the #inclusion of <linux/assert.h>
>
> There are five macros provided:
>
> (a) ASSERT(X)
>
> Issue an assertion failure error if X is false. In other words, require
> the expression X to be true. For example:
>
> ASSERT(val != 0);
>
> There is no need to display val here in the case the expression fails
> since it can only be 0. If this fails, it produces an error like the
> following:
>
> ------------[ cut here ]------------
> ASSERTION FAILED at fs/fscache/main.c:109!
> invalid opcode: 0000 [#1] SMP
>
> (b) ASSERTCMP(X, OP, Y)
>
> Issue an assertion failure error if the expression X OP Y is false. For
> example:
>
> ASSERTCMP(x, >, 12)
>
>
> If an oops is produced, then the values of X and Y will be displayed in
> hex, along with OP:
>
> ------------[ cut here ]------------
> ASSERTION FAILED at fs/fscache/main.c:109!
> Check 2 > c is false
> invalid opcode: 0000 [#1] SMP
>
> (c) ASSERTRANGE(X, OP, Y, OP2, Z)
>
> Issue an assertion failure error if the expression X OP Y or if the
> expression Y OP2 Z is false. Typically OP and OP2 would be < or <=,
> looking something like:
>
> ASSERTRANGE(11, <, x, <=, 13);
>
> and giving the following error:
>
> ------------[ cut here ]------------
> ASSERTION FAILED at fs/fscache/main.c:109!
> Check b < 2 <= d is false
> invalid opcode: 0000 [#1] SMP
Hmm, but why not have a single something-like-"ASSERT" doing the same as
in userspace:
#define ASSERT(X) do { \
if (unlikely(!(X))) \
cond_assertion_failed("Assertion '" #X "' failed"); \
} while (0)
You would not need zillion of sub-macros then.
thanks,
--
js
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 2/7] Add assertion checking macros
2011-10-12 16:47 ` [PATCH 2/7] Add assertion checking macros David Howells
2011-10-12 16:47 ` David Howells
2011-10-12 17:43 ` Jiri Slaby
@ 2011-10-12 20:36 ` David Howells
2 siblings, 0 replies; 20+ messages in thread
From: David Howells @ 2011-10-12 20:36 UTC (permalink / raw)
To: Jiri Slaby; +Cc: dhowells, linux-arch, linux-kernel
Jiri Slaby <jirislaby@gmail.com> wrote:
> Hmm, but why not have a single something-like-"ASSERT" doing the same as
> in userspace:
Because I want to print the values that were checked. Your suggestion merely
turns the unevaluated expression into a string and prints that. The values
are very useful to know in debugging the problem.
David
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/7] Make shrink_dcache_for_umount_subtree() use the core assertion code
2011-10-12 16:47 [PATCH 1/7] Add assertion support with annotated oopsing David Howells
2011-10-12 16:47 ` [PATCH 2/7] Add assertion checking macros David Howells
@ 2011-10-12 16:47 ` David Howells
2011-10-12 16:47 ` David Howells
2011-10-12 16:47 ` [PATCH 4/7] FS-Cache: Use new core assertion macros David Howells
` (5 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: David Howells @ 2011-10-12 16:47 UTC (permalink / raw)
To: linux-arch; +Cc: dhowells, linux-kernel
Make shrink_dcache_for_umount_subtree() use the core assertion code rather than
doing printk() then BUG() so that the annotation appears _after_ the "cut here"
line instead of before it.
Signed-off-by: David Howells <dhowells@redhat.com>
---
fs/dcache.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/fs/dcache.c b/fs/dcache.c
index a88948b..29f9e44 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -36,6 +36,7 @@
#include <linux/bit_spinlock.h>
#include <linux/rculist_bl.h>
#include <linux/prefetch.h>
+#include <linux/assert.h>
#include "internal.h"
/*
@@ -858,9 +859,9 @@ static void shrink_dcache_for_umount_subtree(struct dentry *dentry)
dentry_lru_del(dentry);
__d_shrink(dentry);
- if (dentry->d_count != 0) {
- printk(KERN_ERR
- "BUG: Dentry %p{i=%lx,n=%s}"
+ if (unlikely(dentry->d_count != 0))
+ assertion_failed(
+ "Dentry %p{i=%lx,n=%s}"
" still in use (%d)"
" [unmount of %s %s]\n",
dentry,
@@ -870,8 +871,6 @@ static void shrink_dcache_for_umount_subtree(struct dentry *dentry)
dentry->d_count,
dentry->d_sb->s_type->name,
dentry->d_sb->s_id);
- BUG();
- }
if (IS_ROOT(dentry)) {
parent = NULL;
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 3/7] Make shrink_dcache_for_umount_subtree() use the core assertion code
2011-10-12 16:47 ` [PATCH 3/7] Make shrink_dcache_for_umount_subtree() use the core assertion code David Howells
@ 2011-10-12 16:47 ` David Howells
0 siblings, 0 replies; 20+ messages in thread
From: David Howells @ 2011-10-12 16:47 UTC (permalink / raw)
To: linux-arch; +Cc: dhowells, linux-kernel
Make shrink_dcache_for_umount_subtree() use the core assertion code rather than
doing printk() then BUG() so that the annotation appears _after_ the "cut here"
line instead of before it.
Signed-off-by: David Howells <dhowells@redhat.com>
---
fs/dcache.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/fs/dcache.c b/fs/dcache.c
index a88948b..29f9e44 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -36,6 +36,7 @@
#include <linux/bit_spinlock.h>
#include <linux/rculist_bl.h>
#include <linux/prefetch.h>
+#include <linux/assert.h>
#include "internal.h"
/*
@@ -858,9 +859,9 @@ static void shrink_dcache_for_umount_subtree(struct dentry *dentry)
dentry_lru_del(dentry);
__d_shrink(dentry);
- if (dentry->d_count != 0) {
- printk(KERN_ERR
- "BUG: Dentry %p{i=%lx,n=%s}"
+ if (unlikely(dentry->d_count != 0))
+ assertion_failed(
+ "Dentry %p{i=%lx,n=%s}"
" still in use (%d)"
" [unmount of %s %s]\n",
dentry,
@@ -870,8 +871,6 @@ static void shrink_dcache_for_umount_subtree(struct dentry *dentry)
dentry->d_count,
dentry->d_sb->s_type->name,
dentry->d_sb->s_id);
- BUG();
- }
if (IS_ROOT(dentry)) {
parent = NULL;
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 4/7] FS-Cache: Use new core assertion macros
2011-10-12 16:47 [PATCH 1/7] Add assertion support with annotated oopsing David Howells
2011-10-12 16:47 ` [PATCH 2/7] Add assertion checking macros David Howells
2011-10-12 16:47 ` [PATCH 3/7] Make shrink_dcache_for_umount_subtree() use the core assertion code David Howells
@ 2011-10-12 16:47 ` David Howells
2011-10-12 16:47 ` David Howells
2011-10-12 16:47 ` [PATCH 5/7] CacheFiles: " David Howells
` (4 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: David Howells @ 2011-10-12 16:47 UTC (permalink / raw)
To: linux-arch; +Cc: dhowells, linux-kernel
Make FS-Cache use the new core assertion macros in place of its own.
Signed-off-by: David Howells <dhowells@redhat.com>
---
fs/fscache/debug.h | 82 +++++++++++++++++++++++++++++++
fs/fscache/internal.h | 128 +-----------------------------------------------
fs/fscache/operation.c | 3 -
3 files changed, 86 insertions(+), 127 deletions(-)
create mode 100644 fs/fscache/debug.h
diff --git a/fs/fscache/debug.h b/fs/fscache/debug.h
new file mode 100644
index 0000000..fcdbbab
--- /dev/null
+++ b/fs/fscache/debug.h
@@ -0,0 +1,82 @@
+/* Debug tracing
+ *
+ * Copyright (C) 2004-2011 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#include <linux/printk.h>
+#define ENABLE_ASSERTIONS
+#include <linux/assert.h>
+
+/*
+ * debug tracing
+ */
+#define dbgprintk(FMT, ...) \
+ printk(KERN_DEBUG "[%-6.6s] "FMT"\n", current->comm, ##__VA_ARGS__)
+
+#define kenter(FMT, ...) dbgprintk("==> %s("FMT")", __func__, ##__VA_ARGS__)
+#define kleave(FMT, ...) dbgprintk("<== %s()"FMT"", __func__, ##__VA_ARGS__)
+#define kdebug(FMT, ...) dbgprintk(FMT, ##__VA_ARGS__)
+
+#define kjournal(FMT, ...) no_printk(FMT, ##__VA_ARGS__)
+
+#ifdef __KDEBUG
+#define _enter(FMT, ...) kenter(FMT, ##__VA_ARGS__)
+#define _leave(FMT, ...) kleave(FMT, ##__VA_ARGS__)
+#define _debug(FMT, ...) kdebug(FMT, ##__VA_ARGS__)
+
+#elif defined(CONFIG_FSCACHE_DEBUG)
+#define _enter(FMT, ...) \
+do { \
+ if (__do_kdebug(ENTER)) \
+ kenter(FMT, ##__VA_ARGS__); \
+} while (0)
+
+#define _leave(FMT, ...) \
+do { \
+ if (__do_kdebug(LEAVE)) \
+ kleave(FMT, ##__VA_ARGS__); \
+} while (0)
+
+#define _debug(FMT, ...) \
+do { \
+ if (__do_kdebug(DEBUG)) \
+ kdebug(FMT, ##__VA_ARGS__); \
+} while (0)
+
+#else
+#define _enter(FMT, ...) no_printk("==> %s("FMT")", __func__, ##__VA_ARGS__)
+#define _leave(FMT, ...) no_printk("<== %s()"FMT"", __func__, ##__VA_ARGS__)
+#define _debug(FMT, ...) no_printk(FMT, ##__VA_ARGS__)
+#endif
+
+/*
+ * determine whether a particular optional debugging point should be logged
+ * - we need to go through three steps to persuade cpp to correctly join the
+ * shorthand in FSCACHE_DEBUG_LEVEL with its prefix
+ */
+#define ____do_kdebug(LEVEL, POINT) \
+ unlikely((fscache_debug & \
+ (FSCACHE_POINT_##POINT << (FSCACHE_DEBUG_ ## LEVEL * 3))))
+#define ___do_kdebug(LEVEL, POINT) \
+ ____do_kdebug(LEVEL, POINT)
+#define __do_kdebug(POINT) \
+ ___do_kdebug(FSCACHE_DEBUG_LEVEL, POINT)
+
+#define FSCACHE_DEBUG_CACHE 0
+#define FSCACHE_DEBUG_COOKIE 1
+#define FSCACHE_DEBUG_PAGE 2
+#define FSCACHE_DEBUG_OPERATION 3
+
+#define FSCACHE_POINT_ENTER 1
+#define FSCACHE_POINT_LEAVE 2
+#define FSCACHE_POINT_DEBUG 4
+
+#ifndef FSCACHE_DEBUG_LEVEL
+#define FSCACHE_DEBUG_LEVEL CACHE
+#endif
diff --git a/fs/fscache/internal.h b/fs/fscache/internal.h
index dcb3e1d..4c1a1ba 100644
--- a/fs/fscache/internal.h
+++ b/fs/fscache/internal.h
@@ -24,6 +24,7 @@
#include <linux/fscache-cache.h>
#include <linux/sched.h>
+#include "debug.h"
#define FSCACHE_MIN_THREADS 4
#define FSCACHE_MAX_THREADS 32
@@ -288,7 +289,7 @@ extern const struct file_operations fscache_stats_fops;
static inline void fscache_raise_event(struct fscache_object *object,
unsigned event)
{
- BUG_ON(event >= NR_FSCACHE_OBJECT_EVENTS);
+ ASSERTCMP(event, <, NR_FSCACHE_OBJECT_EVENTS);
if (!test_and_set_bit(event, &object->events) &&
test_bit(event, &object->event_mask))
fscache_enqueue_object(object);
@@ -299,7 +300,7 @@ static inline void fscache_raise_event(struct fscache_object *object,
*/
static inline void fscache_cookie_put(struct fscache_cookie *cookie)
{
- BUG_ON(atomic_read(&cookie->usage) <= 0);
+ ASSERTCMP(atomic_read(&cookie->usage), >, 0);
if (atomic_dec_and_test(&cookie->usage))
__fscache_cookie_put(cookie);
}
@@ -324,126 +325,3 @@ void fscache_put_context(struct fscache_cookie *cookie, void *context)
if (cookie->def->put_context)
cookie->def->put_context(cookie->netfs_data, context);
}
-
-/*****************************************************************************/
-/*
- * debug tracing
- */
-#define dbgprintk(FMT, ...) \
- printk(KERN_DEBUG "[%-6.6s] "FMT"\n", current->comm, ##__VA_ARGS__)
-
-#define kenter(FMT, ...) dbgprintk("==> %s("FMT")", __func__, ##__VA_ARGS__)
-#define kleave(FMT, ...) dbgprintk("<== %s()"FMT"", __func__, ##__VA_ARGS__)
-#define kdebug(FMT, ...) dbgprintk(FMT, ##__VA_ARGS__)
-
-#define kjournal(FMT, ...) no_printk(FMT, ##__VA_ARGS__)
-
-#ifdef __KDEBUG
-#define _enter(FMT, ...) kenter(FMT, ##__VA_ARGS__)
-#define _leave(FMT, ...) kleave(FMT, ##__VA_ARGS__)
-#define _debug(FMT, ...) kdebug(FMT, ##__VA_ARGS__)
-
-#elif defined(CONFIG_FSCACHE_DEBUG)
-#define _enter(FMT, ...) \
-do { \
- if (__do_kdebug(ENTER)) \
- kenter(FMT, ##__VA_ARGS__); \
-} while (0)
-
-#define _leave(FMT, ...) \
-do { \
- if (__do_kdebug(LEAVE)) \
- kleave(FMT, ##__VA_ARGS__); \
-} while (0)
-
-#define _debug(FMT, ...) \
-do { \
- if (__do_kdebug(DEBUG)) \
- kdebug(FMT, ##__VA_ARGS__); \
-} while (0)
-
-#else
-#define _enter(FMT, ...) no_printk("==> %s("FMT")", __func__, ##__VA_ARGS__)
-#define _leave(FMT, ...) no_printk("<== %s()"FMT"", __func__, ##__VA_ARGS__)
-#define _debug(FMT, ...) no_printk(FMT, ##__VA_ARGS__)
-#endif
-
-/*
- * determine whether a particular optional debugging point should be logged
- * - we need to go through three steps to persuade cpp to correctly join the
- * shorthand in FSCACHE_DEBUG_LEVEL with its prefix
- */
-#define ____do_kdebug(LEVEL, POINT) \
- unlikely((fscache_debug & \
- (FSCACHE_POINT_##POINT << (FSCACHE_DEBUG_ ## LEVEL * 3))))
-#define ___do_kdebug(LEVEL, POINT) \
- ____do_kdebug(LEVEL, POINT)
-#define __do_kdebug(POINT) \
- ___do_kdebug(FSCACHE_DEBUG_LEVEL, POINT)
-
-#define FSCACHE_DEBUG_CACHE 0
-#define FSCACHE_DEBUG_COOKIE 1
-#define FSCACHE_DEBUG_PAGE 2
-#define FSCACHE_DEBUG_OPERATION 3
-
-#define FSCACHE_POINT_ENTER 1
-#define FSCACHE_POINT_LEAVE 2
-#define FSCACHE_POINT_DEBUG 4
-
-#ifndef FSCACHE_DEBUG_LEVEL
-#define FSCACHE_DEBUG_LEVEL CACHE
-#endif
-
-/*
- * assertions
- */
-#if 1 /* defined(__KDEBUGALL) */
-
-#define ASSERT(X) \
-do { \
- if (unlikely(!(X))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "FS-Cache: Assertion failed\n"); \
- BUG(); \
- } \
-} while (0)
-
-#define ASSERTCMP(X, OP, Y) \
-do { \
- if (unlikely(!((X) OP (Y)))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "FS-Cache: Assertion failed\n"); \
- printk(KERN_ERR "%lx " #OP " %lx is false\n", \
- (unsigned long)(X), (unsigned long)(Y)); \
- BUG(); \
- } \
-} while (0)
-
-#define ASSERTIF(C, X) \
-do { \
- if (unlikely((C) && !(X))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "FS-Cache: Assertion failed\n"); \
- BUG(); \
- } \
-} while (0)
-
-#define ASSERTIFCMP(C, X, OP, Y) \
-do { \
- if (unlikely((C) && !((X) OP (Y)))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "FS-Cache: Assertion failed\n"); \
- printk(KERN_ERR "%lx " #OP " %lx is false\n", \
- (unsigned long)(X), (unsigned long)(Y)); \
- BUG(); \
- } \
-} while (0)
-
-#else
-
-#define ASSERT(X) do {} while (0)
-#define ASSERTCMP(X, OP, Y) do {} while (0)
-#define ASSERTIF(C, X) do {} while (0)
-#define ASSERTIFCMP(C, X, OP, Y) do {} while (0)
-
-#endif /* assert or not */
diff --git a/fs/fscache/operation.c b/fs/fscache/operation.c
index cafba83..be15b09 100644
--- a/fs/fscache/operation.c
+++ b/fs/fscache/operation.c
@@ -429,8 +429,7 @@ void fscache_put_operation(struct fscache_operation *op)
printk("FS-Cache: Asserting on %s operation\n",
fscache_op_names[op->name]);
- ASSERTIFCMP(op->state != FSCACHE_OP_ST_COMPLETE,
- op->state, ==, FSCACHE_OP_ST_CANCELLED);
+ ASSERTRANGE(FSCACHE_OP_ST_COMPLETE, <=, op->state, <=, FSCACHE_OP_ST_CANCELLED);
op->state = FSCACHE_OP_ST_DEAD;
fscache_stat(&fscache_n_op_release);
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 4/7] FS-Cache: Use new core assertion macros
2011-10-12 16:47 ` [PATCH 4/7] FS-Cache: Use new core assertion macros David Howells
@ 2011-10-12 16:47 ` David Howells
0 siblings, 0 replies; 20+ messages in thread
From: David Howells @ 2011-10-12 16:47 UTC (permalink / raw)
To: linux-arch; +Cc: dhowells, linux-kernel
Make FS-Cache use the new core assertion macros in place of its own.
Signed-off-by: David Howells <dhowells@redhat.com>
---
fs/fscache/debug.h | 82 +++++++++++++++++++++++++++++++
fs/fscache/internal.h | 128 +-----------------------------------------------
fs/fscache/operation.c | 3 -
3 files changed, 86 insertions(+), 127 deletions(-)
create mode 100644 fs/fscache/debug.h
diff --git a/fs/fscache/debug.h b/fs/fscache/debug.h
new file mode 100644
index 0000000..fcdbbab
--- /dev/null
+++ b/fs/fscache/debug.h
@@ -0,0 +1,82 @@
+/* Debug tracing
+ *
+ * Copyright (C) 2004-2011 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#include <linux/printk.h>
+#define ENABLE_ASSERTIONS
+#include <linux/assert.h>
+
+/*
+ * debug tracing
+ */
+#define dbgprintk(FMT, ...) \
+ printk(KERN_DEBUG "[%-6.6s] "FMT"\n", current->comm, ##__VA_ARGS__)
+
+#define kenter(FMT, ...) dbgprintk("==> %s("FMT")", __func__, ##__VA_ARGS__)
+#define kleave(FMT, ...) dbgprintk("<== %s()"FMT"", __func__, ##__VA_ARGS__)
+#define kdebug(FMT, ...) dbgprintk(FMT, ##__VA_ARGS__)
+
+#define kjournal(FMT, ...) no_printk(FMT, ##__VA_ARGS__)
+
+#ifdef __KDEBUG
+#define _enter(FMT, ...) kenter(FMT, ##__VA_ARGS__)
+#define _leave(FMT, ...) kleave(FMT, ##__VA_ARGS__)
+#define _debug(FMT, ...) kdebug(FMT, ##__VA_ARGS__)
+
+#elif defined(CONFIG_FSCACHE_DEBUG)
+#define _enter(FMT, ...) \
+do { \
+ if (__do_kdebug(ENTER)) \
+ kenter(FMT, ##__VA_ARGS__); \
+} while (0)
+
+#define _leave(FMT, ...) \
+do { \
+ if (__do_kdebug(LEAVE)) \
+ kleave(FMT, ##__VA_ARGS__); \
+} while (0)
+
+#define _debug(FMT, ...) \
+do { \
+ if (__do_kdebug(DEBUG)) \
+ kdebug(FMT, ##__VA_ARGS__); \
+} while (0)
+
+#else
+#define _enter(FMT, ...) no_printk("==> %s("FMT")", __func__, ##__VA_ARGS__)
+#define _leave(FMT, ...) no_printk("<== %s()"FMT"", __func__, ##__VA_ARGS__)
+#define _debug(FMT, ...) no_printk(FMT, ##__VA_ARGS__)
+#endif
+
+/*
+ * determine whether a particular optional debugging point should be logged
+ * - we need to go through three steps to persuade cpp to correctly join the
+ * shorthand in FSCACHE_DEBUG_LEVEL with its prefix
+ */
+#define ____do_kdebug(LEVEL, POINT) \
+ unlikely((fscache_debug & \
+ (FSCACHE_POINT_##POINT << (FSCACHE_DEBUG_ ## LEVEL * 3))))
+#define ___do_kdebug(LEVEL, POINT) \
+ ____do_kdebug(LEVEL, POINT)
+#define __do_kdebug(POINT) \
+ ___do_kdebug(FSCACHE_DEBUG_LEVEL, POINT)
+
+#define FSCACHE_DEBUG_CACHE 0
+#define FSCACHE_DEBUG_COOKIE 1
+#define FSCACHE_DEBUG_PAGE 2
+#define FSCACHE_DEBUG_OPERATION 3
+
+#define FSCACHE_POINT_ENTER 1
+#define FSCACHE_POINT_LEAVE 2
+#define FSCACHE_POINT_DEBUG 4
+
+#ifndef FSCACHE_DEBUG_LEVEL
+#define FSCACHE_DEBUG_LEVEL CACHE
+#endif
diff --git a/fs/fscache/internal.h b/fs/fscache/internal.h
index dcb3e1d..4c1a1ba 100644
--- a/fs/fscache/internal.h
+++ b/fs/fscache/internal.h
@@ -24,6 +24,7 @@
#include <linux/fscache-cache.h>
#include <linux/sched.h>
+#include "debug.h"
#define FSCACHE_MIN_THREADS 4
#define FSCACHE_MAX_THREADS 32
@@ -288,7 +289,7 @@ extern const struct file_operations fscache_stats_fops;
static inline void fscache_raise_event(struct fscache_object *object,
unsigned event)
{
- BUG_ON(event >= NR_FSCACHE_OBJECT_EVENTS);
+ ASSERTCMP(event, <, NR_FSCACHE_OBJECT_EVENTS);
if (!test_and_set_bit(event, &object->events) &&
test_bit(event, &object->event_mask))
fscache_enqueue_object(object);
@@ -299,7 +300,7 @@ static inline void fscache_raise_event(struct fscache_object *object,
*/
static inline void fscache_cookie_put(struct fscache_cookie *cookie)
{
- BUG_ON(atomic_read(&cookie->usage) <= 0);
+ ASSERTCMP(atomic_read(&cookie->usage), >, 0);
if (atomic_dec_and_test(&cookie->usage))
__fscache_cookie_put(cookie);
}
@@ -324,126 +325,3 @@ void fscache_put_context(struct fscache_cookie *cookie, void *context)
if (cookie->def->put_context)
cookie->def->put_context(cookie->netfs_data, context);
}
-
-/*****************************************************************************/
-/*
- * debug tracing
- */
-#define dbgprintk(FMT, ...) \
- printk(KERN_DEBUG "[%-6.6s] "FMT"\n", current->comm, ##__VA_ARGS__)
-
-#define kenter(FMT, ...) dbgprintk("==> %s("FMT")", __func__, ##__VA_ARGS__)
-#define kleave(FMT, ...) dbgprintk("<== %s()"FMT"", __func__, ##__VA_ARGS__)
-#define kdebug(FMT, ...) dbgprintk(FMT, ##__VA_ARGS__)
-
-#define kjournal(FMT, ...) no_printk(FMT, ##__VA_ARGS__)
-
-#ifdef __KDEBUG
-#define _enter(FMT, ...) kenter(FMT, ##__VA_ARGS__)
-#define _leave(FMT, ...) kleave(FMT, ##__VA_ARGS__)
-#define _debug(FMT, ...) kdebug(FMT, ##__VA_ARGS__)
-
-#elif defined(CONFIG_FSCACHE_DEBUG)
-#define _enter(FMT, ...) \
-do { \
- if (__do_kdebug(ENTER)) \
- kenter(FMT, ##__VA_ARGS__); \
-} while (0)
-
-#define _leave(FMT, ...) \
-do { \
- if (__do_kdebug(LEAVE)) \
- kleave(FMT, ##__VA_ARGS__); \
-} while (0)
-
-#define _debug(FMT, ...) \
-do { \
- if (__do_kdebug(DEBUG)) \
- kdebug(FMT, ##__VA_ARGS__); \
-} while (0)
-
-#else
-#define _enter(FMT, ...) no_printk("==> %s("FMT")", __func__, ##__VA_ARGS__)
-#define _leave(FMT, ...) no_printk("<== %s()"FMT"", __func__, ##__VA_ARGS__)
-#define _debug(FMT, ...) no_printk(FMT, ##__VA_ARGS__)
-#endif
-
-/*
- * determine whether a particular optional debugging point should be logged
- * - we need to go through three steps to persuade cpp to correctly join the
- * shorthand in FSCACHE_DEBUG_LEVEL with its prefix
- */
-#define ____do_kdebug(LEVEL, POINT) \
- unlikely((fscache_debug & \
- (FSCACHE_POINT_##POINT << (FSCACHE_DEBUG_ ## LEVEL * 3))))
-#define ___do_kdebug(LEVEL, POINT) \
- ____do_kdebug(LEVEL, POINT)
-#define __do_kdebug(POINT) \
- ___do_kdebug(FSCACHE_DEBUG_LEVEL, POINT)
-
-#define FSCACHE_DEBUG_CACHE 0
-#define FSCACHE_DEBUG_COOKIE 1
-#define FSCACHE_DEBUG_PAGE 2
-#define FSCACHE_DEBUG_OPERATION 3
-
-#define FSCACHE_POINT_ENTER 1
-#define FSCACHE_POINT_LEAVE 2
-#define FSCACHE_POINT_DEBUG 4
-
-#ifndef FSCACHE_DEBUG_LEVEL
-#define FSCACHE_DEBUG_LEVEL CACHE
-#endif
-
-/*
- * assertions
- */
-#if 1 /* defined(__KDEBUGALL) */
-
-#define ASSERT(X) \
-do { \
- if (unlikely(!(X))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "FS-Cache: Assertion failed\n"); \
- BUG(); \
- } \
-} while (0)
-
-#define ASSERTCMP(X, OP, Y) \
-do { \
- if (unlikely(!((X) OP (Y)))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "FS-Cache: Assertion failed\n"); \
- printk(KERN_ERR "%lx " #OP " %lx is false\n", \
- (unsigned long)(X), (unsigned long)(Y)); \
- BUG(); \
- } \
-} while (0)
-
-#define ASSERTIF(C, X) \
-do { \
- if (unlikely((C) && !(X))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "FS-Cache: Assertion failed\n"); \
- BUG(); \
- } \
-} while (0)
-
-#define ASSERTIFCMP(C, X, OP, Y) \
-do { \
- if (unlikely((C) && !((X) OP (Y)))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "FS-Cache: Assertion failed\n"); \
- printk(KERN_ERR "%lx " #OP " %lx is false\n", \
- (unsigned long)(X), (unsigned long)(Y)); \
- BUG(); \
- } \
-} while (0)
-
-#else
-
-#define ASSERT(X) do {} while (0)
-#define ASSERTCMP(X, OP, Y) do {} while (0)
-#define ASSERTIF(C, X) do {} while (0)
-#define ASSERTIFCMP(C, X, OP, Y) do {} while (0)
-
-#endif /* assert or not */
diff --git a/fs/fscache/operation.c b/fs/fscache/operation.c
index cafba83..be15b09 100644
--- a/fs/fscache/operation.c
+++ b/fs/fscache/operation.c
@@ -429,8 +429,7 @@ void fscache_put_operation(struct fscache_operation *op)
printk("FS-Cache: Asserting on %s operation\n",
fscache_op_names[op->name]);
- ASSERTIFCMP(op->state != FSCACHE_OP_ST_COMPLETE,
- op->state, ==, FSCACHE_OP_ST_CANCELLED);
+ ASSERTRANGE(FSCACHE_OP_ST_COMPLETE, <=, op->state, <=, FSCACHE_OP_ST_CANCELLED);
op->state = FSCACHE_OP_ST_DEAD;
fscache_stat(&fscache_n_op_release);
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 5/7] CacheFiles: Use new core assertion macros
2011-10-12 16:47 [PATCH 1/7] Add assertion support with annotated oopsing David Howells
` (2 preceding siblings ...)
2011-10-12 16:47 ` [PATCH 4/7] FS-Cache: Use new core assertion macros David Howells
@ 2011-10-12 16:47 ` David Howells
2011-10-12 16:47 ` David Howells
2011-10-12 16:48 ` [PATCH 6/7] AFS: " David Howells
` (3 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: David Howells @ 2011-10-12 16:47 UTC (permalink / raw)
To: linux-arch; +Cc: dhowells, linux-kernel
Make CacheFiles use the new core assertion macros in place of its own.
Signed-off-by: David Howells <dhowells@redhat.com>
---
fs/cachefiles/internal.h | 53 ++--------------------------------------------
1 files changed, 2 insertions(+), 51 deletions(-)
diff --git a/fs/cachefiles/internal.h b/fs/cachefiles/internal.h
index 4938251..0b99c9e 100644
--- a/fs/cachefiles/internal.h
+++ b/fs/cachefiles/internal.h
@@ -14,6 +14,8 @@
#include <linux/wait.h>
#include <linux/workqueue.h>
#include <linux/security.h>
+#define ENABLE_ASSERTIONS
+#include <linux/assert.h>
struct cachefiles_cache;
struct cachefiles_object;
@@ -303,54 +305,3 @@ do { \
#define _leave(FMT, ...) no_printk("<== %s()"FMT"", __func__, ##__VA_ARGS__)
#define _debug(FMT, ...) no_printk(FMT, ##__VA_ARGS__)
#endif
-
-#if 1 /* defined(__KDEBUGALL) */
-
-#define ASSERT(X) \
-do { \
- if (unlikely(!(X))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "CacheFiles: Assertion failed\n"); \
- BUG(); \
- } \
-} while (0)
-
-#define ASSERTCMP(X, OP, Y) \
-do { \
- if (unlikely(!((X) OP (Y)))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "CacheFiles: Assertion failed\n"); \
- printk(KERN_ERR "%lx " #OP " %lx is false\n", \
- (unsigned long)(X), (unsigned long)(Y)); \
- BUG(); \
- } \
-} while (0)
-
-#define ASSERTIF(C, X) \
-do { \
- if (unlikely((C) && !(X))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "CacheFiles: Assertion failed\n"); \
- BUG(); \
- } \
-} while (0)
-
-#define ASSERTIFCMP(C, X, OP, Y) \
-do { \
- if (unlikely((C) && !((X) OP (Y)))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "CacheFiles: Assertion failed\n"); \
- printk(KERN_ERR "%lx " #OP " %lx is false\n", \
- (unsigned long)(X), (unsigned long)(Y)); \
- BUG(); \
- } \
-} while (0)
-
-#else
-
-#define ASSERT(X) do {} while (0)
-#define ASSERTCMP(X, OP, Y) do {} while (0)
-#define ASSERTIF(C, X) do {} while (0)
-#define ASSERTIFCMP(C, X, OP, Y) do {} while (0)
-
-#endif
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 5/7] CacheFiles: Use new core assertion macros
2011-10-12 16:47 ` [PATCH 5/7] CacheFiles: " David Howells
@ 2011-10-12 16:47 ` David Howells
0 siblings, 0 replies; 20+ messages in thread
From: David Howells @ 2011-10-12 16:47 UTC (permalink / raw)
To: linux-arch; +Cc: dhowells, linux-kernel
Make CacheFiles use the new core assertion macros in place of its own.
Signed-off-by: David Howells <dhowells@redhat.com>
---
fs/cachefiles/internal.h | 53 ++--------------------------------------------
1 files changed, 2 insertions(+), 51 deletions(-)
diff --git a/fs/cachefiles/internal.h b/fs/cachefiles/internal.h
index 4938251..0b99c9e 100644
--- a/fs/cachefiles/internal.h
+++ b/fs/cachefiles/internal.h
@@ -14,6 +14,8 @@
#include <linux/wait.h>
#include <linux/workqueue.h>
#include <linux/security.h>
+#define ENABLE_ASSERTIONS
+#include <linux/assert.h>
struct cachefiles_cache;
struct cachefiles_object;
@@ -303,54 +305,3 @@ do { \
#define _leave(FMT, ...) no_printk("<== %s()"FMT"", __func__, ##__VA_ARGS__)
#define _debug(FMT, ...) no_printk(FMT, ##__VA_ARGS__)
#endif
-
-#if 1 /* defined(__KDEBUGALL) */
-
-#define ASSERT(X) \
-do { \
- if (unlikely(!(X))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "CacheFiles: Assertion failed\n"); \
- BUG(); \
- } \
-} while (0)
-
-#define ASSERTCMP(X, OP, Y) \
-do { \
- if (unlikely(!((X) OP (Y)))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "CacheFiles: Assertion failed\n"); \
- printk(KERN_ERR "%lx " #OP " %lx is false\n", \
- (unsigned long)(X), (unsigned long)(Y)); \
- BUG(); \
- } \
-} while (0)
-
-#define ASSERTIF(C, X) \
-do { \
- if (unlikely((C) && !(X))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "CacheFiles: Assertion failed\n"); \
- BUG(); \
- } \
-} while (0)
-
-#define ASSERTIFCMP(C, X, OP, Y) \
-do { \
- if (unlikely((C) && !((X) OP (Y)))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "CacheFiles: Assertion failed\n"); \
- printk(KERN_ERR "%lx " #OP " %lx is false\n", \
- (unsigned long)(X), (unsigned long)(Y)); \
- BUG(); \
- } \
-} while (0)
-
-#else
-
-#define ASSERT(X) do {} while (0)
-#define ASSERTCMP(X, OP, Y) do {} while (0)
-#define ASSERTIF(C, X) do {} while (0)
-#define ASSERTIFCMP(C, X, OP, Y) do {} while (0)
-
-#endif
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 6/7] AFS: Use new core assertion macros
2011-10-12 16:47 [PATCH 1/7] Add assertion support with annotated oopsing David Howells
` (3 preceding siblings ...)
2011-10-12 16:47 ` [PATCH 5/7] CacheFiles: " David Howells
@ 2011-10-12 16:48 ` David Howells
2011-10-12 16:48 ` David Howells
2011-10-12 16:48 ` [PATCH 7/7] RxRPC: " David Howells
` (2 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: David Howells @ 2011-10-12 16:48 UTC (permalink / raw)
To: linux-arch; +Cc: dhowells, linux-kernel
Make AFS use the new core assertion macros in place of its own.
Signed-off-by: David Howells <dhowells@redhat.com>
---
fs/afs/internal.h | 90 +----------------------------------------------------
1 files changed, 2 insertions(+), 88 deletions(-)
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index d2b0888..36989c0 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -20,6 +20,8 @@
#include <linux/sched.h>
#include <linux/fscache.h>
#include <linux/backing-dev.h>
+#define ENABLE_ASSERTIONS
+#include <linux/assert.h>
#include "afs.h"
#include "afs_vl.h"
@@ -800,91 +802,3 @@ do { \
#define _leave(FMT,...) no_printk("<== %s()"FMT"",__func__ ,##__VA_ARGS__)
#define _debug(FMT,...) no_printk(" "FMT ,##__VA_ARGS__)
#endif
-
-/*
- * debug assertion checking
- */
-#if 1 // defined(__KDEBUGALL)
-
-#define ASSERT(X) \
-do { \
- if (unlikely(!(X))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "AFS: Assertion failed\n"); \
- BUG(); \
- } \
-} while(0)
-
-#define ASSERTCMP(X, OP, Y) \
-do { \
- if (unlikely(!((X) OP (Y)))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "AFS: Assertion failed\n"); \
- printk(KERN_ERR "%lu " #OP " %lu is false\n", \
- (unsigned long)(X), (unsigned long)(Y)); \
- printk(KERN_ERR "0x%lx " #OP " 0x%lx is false\n", \
- (unsigned long)(X), (unsigned long)(Y)); \
- BUG(); \
- } \
-} while(0)
-
-#define ASSERTRANGE(L, OP1, N, OP2, H) \
-do { \
- if (unlikely(!((L) OP1 (N)) || !((N) OP2 (H)))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "AFS: Assertion failed\n"); \
- printk(KERN_ERR "%lu "#OP1" %lu "#OP2" %lu is false\n", \
- (unsigned long)(L), (unsigned long)(N), \
- (unsigned long)(H)); \
- printk(KERN_ERR "0x%lx "#OP1" 0x%lx "#OP2" 0x%lx is false\n", \
- (unsigned long)(L), (unsigned long)(N), \
- (unsigned long)(H)); \
- BUG(); \
- } \
-} while(0)
-
-#define ASSERTIF(C, X) \
-do { \
- if (unlikely((C) && !(X))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "AFS: Assertion failed\n"); \
- BUG(); \
- } \
-} while(0)
-
-#define ASSERTIFCMP(C, X, OP, Y) \
-do { \
- if (unlikely((C) && !((X) OP (Y)))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "AFS: Assertion failed\n"); \
- printk(KERN_ERR "%lu " #OP " %lu is false\n", \
- (unsigned long)(X), (unsigned long)(Y)); \
- printk(KERN_ERR "0x%lx " #OP " 0x%lx is false\n", \
- (unsigned long)(X), (unsigned long)(Y)); \
- BUG(); \
- } \
-} while(0)
-
-#else
-
-#define ASSERT(X) \
-do { \
-} while(0)
-
-#define ASSERTCMP(X, OP, Y) \
-do { \
-} while(0)
-
-#define ASSERTRANGE(L, OP1, N, OP2, H) \
-do { \
-} while(0)
-
-#define ASSERTIF(C, X) \
-do { \
-} while(0)
-
-#define ASSERTIFCMP(C, X, OP, Y) \
-do { \
-} while(0)
-
-#endif /* __KDEBUGALL */
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 6/7] AFS: Use new core assertion macros
2011-10-12 16:48 ` [PATCH 6/7] AFS: " David Howells
@ 2011-10-12 16:48 ` David Howells
0 siblings, 0 replies; 20+ messages in thread
From: David Howells @ 2011-10-12 16:48 UTC (permalink / raw)
To: linux-arch; +Cc: dhowells, linux-kernel
Make AFS use the new core assertion macros in place of its own.
Signed-off-by: David Howells <dhowells@redhat.com>
---
fs/afs/internal.h | 90 +----------------------------------------------------
1 files changed, 2 insertions(+), 88 deletions(-)
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index d2b0888..36989c0 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -20,6 +20,8 @@
#include <linux/sched.h>
#include <linux/fscache.h>
#include <linux/backing-dev.h>
+#define ENABLE_ASSERTIONS
+#include <linux/assert.h>
#include "afs.h"
#include "afs_vl.h"
@@ -800,91 +802,3 @@ do { \
#define _leave(FMT,...) no_printk("<== %s()"FMT"",__func__ ,##__VA_ARGS__)
#define _debug(FMT,...) no_printk(" "FMT ,##__VA_ARGS__)
#endif
-
-/*
- * debug assertion checking
- */
-#if 1 // defined(__KDEBUGALL)
-
-#define ASSERT(X) \
-do { \
- if (unlikely(!(X))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "AFS: Assertion failed\n"); \
- BUG(); \
- } \
-} while(0)
-
-#define ASSERTCMP(X, OP, Y) \
-do { \
- if (unlikely(!((X) OP (Y)))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "AFS: Assertion failed\n"); \
- printk(KERN_ERR "%lu " #OP " %lu is false\n", \
- (unsigned long)(X), (unsigned long)(Y)); \
- printk(KERN_ERR "0x%lx " #OP " 0x%lx is false\n", \
- (unsigned long)(X), (unsigned long)(Y)); \
- BUG(); \
- } \
-} while(0)
-
-#define ASSERTRANGE(L, OP1, N, OP2, H) \
-do { \
- if (unlikely(!((L) OP1 (N)) || !((N) OP2 (H)))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "AFS: Assertion failed\n"); \
- printk(KERN_ERR "%lu "#OP1" %lu "#OP2" %lu is false\n", \
- (unsigned long)(L), (unsigned long)(N), \
- (unsigned long)(H)); \
- printk(KERN_ERR "0x%lx "#OP1" 0x%lx "#OP2" 0x%lx is false\n", \
- (unsigned long)(L), (unsigned long)(N), \
- (unsigned long)(H)); \
- BUG(); \
- } \
-} while(0)
-
-#define ASSERTIF(C, X) \
-do { \
- if (unlikely((C) && !(X))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "AFS: Assertion failed\n"); \
- BUG(); \
- } \
-} while(0)
-
-#define ASSERTIFCMP(C, X, OP, Y) \
-do { \
- if (unlikely((C) && !((X) OP (Y)))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "AFS: Assertion failed\n"); \
- printk(KERN_ERR "%lu " #OP " %lu is false\n", \
- (unsigned long)(X), (unsigned long)(Y)); \
- printk(KERN_ERR "0x%lx " #OP " 0x%lx is false\n", \
- (unsigned long)(X), (unsigned long)(Y)); \
- BUG(); \
- } \
-} while(0)
-
-#else
-
-#define ASSERT(X) \
-do { \
-} while(0)
-
-#define ASSERTCMP(X, OP, Y) \
-do { \
-} while(0)
-
-#define ASSERTRANGE(L, OP1, N, OP2, H) \
-do { \
-} while(0)
-
-#define ASSERTIF(C, X) \
-do { \
-} while(0)
-
-#define ASSERTIFCMP(C, X, OP, Y) \
-do { \
-} while(0)
-
-#endif /* __KDEBUGALL */
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 7/7] RxRPC: Use new core assertion macros
2011-10-12 16:47 [PATCH 1/7] Add assertion support with annotated oopsing David Howells
` (4 preceding siblings ...)
2011-10-12 16:48 ` [PATCH 6/7] AFS: " David Howells
@ 2011-10-12 16:48 ` David Howells
2011-10-12 16:48 ` David Howells
2011-10-12 16:57 ` [PATCH 1/7] Add assertion support with annotated oopsing Ingo Molnar
2011-10-12 17:23 ` David Howells
7 siblings, 1 reply; 20+ messages in thread
From: David Howells @ 2011-10-12 16:48 UTC (permalink / raw)
To: linux-arch; +Cc: dhowells, linux-kernel
Make RxRPC use the new core assertion macros in place of its own.
Signed-off-by: David Howells <dhowells@redhat.com>
---
net/rxrpc/ar-internal.h | 71 +----------------------------------------------
1 files changed, 2 insertions(+), 69 deletions(-)
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 8e22bd3..b86fe78 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -10,6 +10,8 @@
*/
#include <rxrpc/packet.h>
+#define ENABLE_ASSERTIONS
+#include <linux/assert.h>
#if 0
#define CHECK_SLAB_OKAY(X) \
@@ -657,75 +659,6 @@ do { \
#endif
/*
- * debug assertion checking
- */
-#if 1 // defined(__KDEBUGALL)
-
-#define ASSERT(X) \
-do { \
- if (unlikely(!(X))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "RxRPC: Assertion failed\n"); \
- BUG(); \
- } \
-} while(0)
-
-#define ASSERTCMP(X, OP, Y) \
-do { \
- if (unlikely(!((X) OP (Y)))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "RxRPC: Assertion failed\n"); \
- printk(KERN_ERR "%lu " #OP " %lu is false\n", \
- (unsigned long)(X), (unsigned long)(Y)); \
- printk(KERN_ERR "0x%lx " #OP " 0x%lx is false\n", \
- (unsigned long)(X), (unsigned long)(Y)); \
- BUG(); \
- } \
-} while(0)
-
-#define ASSERTIF(C, X) \
-do { \
- if (unlikely((C) && !(X))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "RxRPC: Assertion failed\n"); \
- BUG(); \
- } \
-} while(0)
-
-#define ASSERTIFCMP(C, X, OP, Y) \
-do { \
- if (unlikely((C) && !((X) OP (Y)))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "RxRPC: Assertion failed\n"); \
- printk(KERN_ERR "%lu " #OP " %lu is false\n", \
- (unsigned long)(X), (unsigned long)(Y)); \
- printk(KERN_ERR "0x%lx " #OP " 0x%lx is false\n", \
- (unsigned long)(X), (unsigned long)(Y)); \
- BUG(); \
- } \
-} while(0)
-
-#else
-
-#define ASSERT(X) \
-do { \
-} while(0)
-
-#define ASSERTCMP(X, OP, Y) \
-do { \
-} while(0)
-
-#define ASSERTIF(C, X) \
-do { \
-} while(0)
-
-#define ASSERTIFCMP(C, X, OP, Y) \
-do { \
-} while(0)
-
-#endif /* __KDEBUGALL */
-
-/*
* socket buffer accounting / leak finding
*/
static inline void __rxrpc_new_skb(struct sk_buff *skb, const char *fn)
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 7/7] RxRPC: Use new core assertion macros
2011-10-12 16:48 ` [PATCH 7/7] RxRPC: " David Howells
@ 2011-10-12 16:48 ` David Howells
0 siblings, 0 replies; 20+ messages in thread
From: David Howells @ 2011-10-12 16:48 UTC (permalink / raw)
To: linux-arch; +Cc: dhowells, linux-kernel
Make RxRPC use the new core assertion macros in place of its own.
Signed-off-by: David Howells <dhowells@redhat.com>
---
net/rxrpc/ar-internal.h | 71 +----------------------------------------------
1 files changed, 2 insertions(+), 69 deletions(-)
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 8e22bd3..b86fe78 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -10,6 +10,8 @@
*/
#include <rxrpc/packet.h>
+#define ENABLE_ASSERTIONS
+#include <linux/assert.h>
#if 0
#define CHECK_SLAB_OKAY(X) \
@@ -657,75 +659,6 @@ do { \
#endif
/*
- * debug assertion checking
- */
-#if 1 // defined(__KDEBUGALL)
-
-#define ASSERT(X) \
-do { \
- if (unlikely(!(X))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "RxRPC: Assertion failed\n"); \
- BUG(); \
- } \
-} while(0)
-
-#define ASSERTCMP(X, OP, Y) \
-do { \
- if (unlikely(!((X) OP (Y)))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "RxRPC: Assertion failed\n"); \
- printk(KERN_ERR "%lu " #OP " %lu is false\n", \
- (unsigned long)(X), (unsigned long)(Y)); \
- printk(KERN_ERR "0x%lx " #OP " 0x%lx is false\n", \
- (unsigned long)(X), (unsigned long)(Y)); \
- BUG(); \
- } \
-} while(0)
-
-#define ASSERTIF(C, X) \
-do { \
- if (unlikely((C) && !(X))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "RxRPC: Assertion failed\n"); \
- BUG(); \
- } \
-} while(0)
-
-#define ASSERTIFCMP(C, X, OP, Y) \
-do { \
- if (unlikely((C) && !((X) OP (Y)))) { \
- printk(KERN_ERR "\n"); \
- printk(KERN_ERR "RxRPC: Assertion failed\n"); \
- printk(KERN_ERR "%lu " #OP " %lu is false\n", \
- (unsigned long)(X), (unsigned long)(Y)); \
- printk(KERN_ERR "0x%lx " #OP " 0x%lx is false\n", \
- (unsigned long)(X), (unsigned long)(Y)); \
- BUG(); \
- } \
-} while(0)
-
-#else
-
-#define ASSERT(X) \
-do { \
-} while(0)
-
-#define ASSERTCMP(X, OP, Y) \
-do { \
-} while(0)
-
-#define ASSERTIF(C, X) \
-do { \
-} while(0)
-
-#define ASSERTIFCMP(C, X, OP, Y) \
-do { \
-} while(0)
-
-#endif /* __KDEBUGALL */
-
-/*
* socket buffer accounting / leak finding
*/
static inline void __rxrpc_new_skb(struct sk_buff *skb, const char *fn)
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/7] Add assertion support with annotated oopsing
2011-10-12 16:47 [PATCH 1/7] Add assertion support with annotated oopsing David Howells
` (5 preceding siblings ...)
2011-10-12 16:48 ` [PATCH 7/7] RxRPC: " David Howells
@ 2011-10-12 16:57 ` Ingo Molnar
2011-10-12 16:57 ` Ingo Molnar
2011-10-12 17:23 ` David Howells
7 siblings, 1 reply; 20+ messages in thread
From: Ingo Molnar @ 2011-10-12 16:57 UTC (permalink / raw)
To: David Howells; +Cc: linux-arch, linux-kernel, Andrew Morton, Linus Torvalds
* David Howells <dhowells@redhat.com> wrote:
> Add the ability to create an annotated oops report. This is useful for
> displaying the output of assertion failures where direct display of the values
> being checked is of greater value than the register dump.
>
> This could technically be done simply by issuing one or more printk() calls
> followed by a BUG() but in practice this has a serious disadvantage in that
> people reporting a bug usually seem to take the "cut here" line literally and
> discard everything prior to it when making a report - thus eliminating the most
> important bit of information after the file/line number.
>
> There are number of possible solutions to this. I've used the last in this
> list:
>
> (1) Emit the "cut here" line early, suppressing the one produced by the BUG()
> handler. This would allow the annotation to be formed of multiple
> printk() calls.
>
> (2) Get rid of the "cut here" line entirely.
>
> (3) Pass the annotation through to the exception handler. For practical
> reasons, this limits the number of annotations to a single format string
> and parameters. This means that a va_list has to be passed through and
> thence to vprintk() - which should be okay. It also requires arch support
> to retrieve the annotation data.
>
>
> This facility can be made use of by #including <linux/assert.h> and then
> calling:
>
> void assertion_failed(const char *fmt, ...);
>
> This prints a report that looks like:
>
> ------------[ cut here ]------------
> ASSERTION FAILED at fs/dcache.c:863!
> invalid opcode: 0000 [#1] SMP
> ...
>
> if fmt is NULL and:
>
> ------------[ cut here ]------------
> ASSERTION FAILED at fs/dcache.c:863!
> Dentry 0xffff880032675ed8{i=242,n=Documents} still in use (1) [unmount of nfs 12:01]
> invalid opcode: 0000 [#1] SMP
> ...
>
> otherwise.
>
> For this to work the arch code must provide two things:
>
> #define arch_assertion_failed(struct assertion_failure *desc)
>
> to perform the oops and:
>
> #define arch_assertion_failure(struct pt_regs *regs)
>
> for report_bug() to find whether or not an assertion failure occurred and, if
> so, return a pointer to its description as passed to arch_assertion_failure().
>
> If arch_assertion_failed() is not defined, then the code will fall back to
> doing a printk() and a BUG().
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
>
> arch/x86/include/asm/bug.h | 14 ++++++++++++++
> include/asm-generic/bug.h | 1 +
> include/linux/assert.h | 36 ++++++++++++++++++++++++++++++++++++
> kernel/panic.c | 31 +++++++++++++++++++++++++++++++
> lib/bug.c | 16 ++++++++++++++++
> 5 files changed, 98 insertions(+), 0 deletions(-)
> create mode 100644 include/linux/assert.h
Looks useful, but i'd suggest to make this a variant of the standard
BUG_ON()/WARN_ON() checks we already have, not an explicit assert().
BUG_ON_VERBOSE() or such.
I find assert()'s inversion confusing when mixed with
WARN_ON()/BUG_ON().
Likewise, the message of:
ASSERTION FAILED at fs/dcache.c:863!
is rather confusing to me (i never know how the condition printed is
to be interpreted) - why not use the usual 'BUG: ...' message
convention?
Thanks,
Ingo
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 1/7] Add assertion support with annotated oopsing
2011-10-12 16:57 ` [PATCH 1/7] Add assertion support with annotated oopsing Ingo Molnar
@ 2011-10-12 16:57 ` Ingo Molnar
0 siblings, 0 replies; 20+ messages in thread
From: Ingo Molnar @ 2011-10-12 16:57 UTC (permalink / raw)
To: David Howells; +Cc: linux-arch, linux-kernel, Andrew Morton, Linus Torvalds
* David Howells <dhowells@redhat.com> wrote:
> Add the ability to create an annotated oops report. This is useful for
> displaying the output of assertion failures where direct display of the values
> being checked is of greater value than the register dump.
>
> This could technically be done simply by issuing one or more printk() calls
> followed by a BUG() but in practice this has a serious disadvantage in that
> people reporting a bug usually seem to take the "cut here" line literally and
> discard everything prior to it when making a report - thus eliminating the most
> important bit of information after the file/line number.
>
> There are number of possible solutions to this. I've used the last in this
> list:
>
> (1) Emit the "cut here" line early, suppressing the one produced by the BUG()
> handler. This would allow the annotation to be formed of multiple
> printk() calls.
>
> (2) Get rid of the "cut here" line entirely.
>
> (3) Pass the annotation through to the exception handler. For practical
> reasons, this limits the number of annotations to a single format string
> and parameters. This means that a va_list has to be passed through and
> thence to vprintk() - which should be okay. It also requires arch support
> to retrieve the annotation data.
>
>
> This facility can be made use of by #including <linux/assert.h> and then
> calling:
>
> void assertion_failed(const char *fmt, ...);
>
> This prints a report that looks like:
>
> ------------[ cut here ]------------
> ASSERTION FAILED at fs/dcache.c:863!
> invalid opcode: 0000 [#1] SMP
> ...
>
> if fmt is NULL and:
>
> ------------[ cut here ]------------
> ASSERTION FAILED at fs/dcache.c:863!
> Dentry 0xffff880032675ed8{i=242,n=Documents} still in use (1) [unmount of nfs 12:01]
> invalid opcode: 0000 [#1] SMP
> ...
>
> otherwise.
>
> For this to work the arch code must provide two things:
>
> #define arch_assertion_failed(struct assertion_failure *desc)
>
> to perform the oops and:
>
> #define arch_assertion_failure(struct pt_regs *regs)
>
> for report_bug() to find whether or not an assertion failure occurred and, if
> so, return a pointer to its description as passed to arch_assertion_failure().
>
> If arch_assertion_failed() is not defined, then the code will fall back to
> doing a printk() and a BUG().
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
>
> arch/x86/include/asm/bug.h | 14 ++++++++++++++
> include/asm-generic/bug.h | 1 +
> include/linux/assert.h | 36 ++++++++++++++++++++++++++++++++++++
> kernel/panic.c | 31 +++++++++++++++++++++++++++++++
> lib/bug.c | 16 ++++++++++++++++
> 5 files changed, 98 insertions(+), 0 deletions(-)
> create mode 100644 include/linux/assert.h
Looks useful, but i'd suggest to make this a variant of the standard
BUG_ON()/WARN_ON() checks we already have, not an explicit assert().
BUG_ON_VERBOSE() or such.
I find assert()'s inversion confusing when mixed with
WARN_ON()/BUG_ON().
Likewise, the message of:
ASSERTION FAILED at fs/dcache.c:863!
is rather confusing to me (i never know how the condition printed is
to be interpreted) - why not use the usual 'BUG: ...' message
convention?
Thanks,
Ingo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/7] Add assertion support with annotated oopsing
2011-10-12 16:47 [PATCH 1/7] Add assertion support with annotated oopsing David Howells
` (6 preceding siblings ...)
2011-10-12 16:57 ` [PATCH 1/7] Add assertion support with annotated oopsing Ingo Molnar
@ 2011-10-12 17:23 ` David Howells
2011-10-12 17:23 ` David Howells
7 siblings, 1 reply; 20+ messages in thread
From: David Howells @ 2011-10-12 17:23 UTC (permalink / raw)
To: Ingo Molnar
Cc: dhowells, linux-arch, linux-kernel, Andrew Morton, Linus Torvalds
Ingo Molnar <mingo@elte.hu> wrote:
> Looks useful, but i'd suggest to make this a variant of the standard
> BUG_ON()/WARN_ON() checks we already have, not an explicit assert().
>
> BUG_ON_VERBOSE() or such.
I personally prefer the positive check (ASSERT() saying that this expression
must be true) as opposed to the negative check (BUG_ON() saying that this must
be false). I find it easier to think about the logic (I expect value X to be
like this, value Y to be like that, etc.).
That said, I could make the base bit BUG_VERBOSE(FMT, ...) and wrap ASSERT*()
around that.
However, I'd _much_ rather make it so that I can post the "cut here" message
early - but, IIRC, Linus hated that idea.
> I find assert()'s inversion confusing when mixed with WARN_ON()/BUG_ON().
Why did we do it this way originally, rather than using assert, I wonder?
Especially since the concept of assert already exists in userspace.
> Likewise, the message of:
>
> ASSERTION FAILED at fs/dcache.c:863!
>
> is rather confusing to me (i never know how the condition printed is
> to be interpreted) - why not use the usual 'BUG: ...' message
> convention?
I don't see why it should be confusing. Something bad happened at file:line.
I could make it print "BUG" instead. That's a minor matter. The ASSERT
macros in patch 2 could then generate a report that looks like this:
------------[ cut here ]------------
kernel BUG at fs/fscache/main.c:109!
Assertion failed: 2 > c is false
invalid opcode: 0000 [#1] SMP
David
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 1/7] Add assertion support with annotated oopsing
2011-10-12 17:23 ` David Howells
@ 2011-10-12 17:23 ` David Howells
0 siblings, 0 replies; 20+ messages in thread
From: David Howells @ 2011-10-12 17:23 UTC (permalink / raw)
To: Ingo Molnar
Cc: dhowells, linux-arch, linux-kernel, Andrew Morton, Linus Torvalds
Ingo Molnar <mingo@elte.hu> wrote:
> Looks useful, but i'd suggest to make this a variant of the standard
> BUG_ON()/WARN_ON() checks we already have, not an explicit assert().
>
> BUG_ON_VERBOSE() or such.
I personally prefer the positive check (ASSERT() saying that this expression
must be true) as opposed to the negative check (BUG_ON() saying that this must
be false). I find it easier to think about the logic (I expect value X to be
like this, value Y to be like that, etc.).
That said, I could make the base bit BUG_VERBOSE(FMT, ...) and wrap ASSERT*()
around that.
However, I'd _much_ rather make it so that I can post the "cut here" message
early - but, IIRC, Linus hated that idea.
> I find assert()'s inversion confusing when mixed with WARN_ON()/BUG_ON().
Why did we do it this way originally, rather than using assert, I wonder?
Especially since the concept of assert already exists in userspace.
> Likewise, the message of:
>
> ASSERTION FAILED at fs/dcache.c:863!
>
> is rather confusing to me (i never know how the condition printed is
> to be interpreted) - why not use the usual 'BUG: ...' message
> convention?
I don't see why it should be confusing. Something bad happened at file:line.
I could make it print "BUG" instead. That's a minor matter. The ASSERT
macros in patch 2 could then generate a report that looks like this:
------------[ cut here ]------------
kernel BUG at fs/fscache/main.c:109!
Assertion failed: 2 > c is false
invalid opcode: 0000 [#1] SMP
David
^ permalink raw reply [flat|nested] 20+ messages in thread