lkml.org 
[lkml]   [2011]   [Oct]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/7] Add assertion support with annotated oopsing

* 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


\
 
 \ /
  Last update: 2011-10-12 19:01    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site