Re: [PATCH] BUG preserve registers

From: Hugh Dickins (hugh@veritas.com)
Date: Mon Feb 11 2002 - 12:19:31 EST


On Sun, 10 Feb 2002, Andrew Morton wrote:
> >
> > I think I'm done with this now.
>
> Famous last words. There's a slightly updated version at
> http://www.zip.com.au/~akpm/linux/2.4/2.4.18-pre9/

Thanks for taking this on, Andrew. A lot of nice work, but...

1. I'd be sorry to see it go in as is: you haven't noticed how this
   looks to a disassembler: not pretty! I don't see an alternative
   to wasting some more bytes: putting "push"es in front of line and
   file? or is there one instruction (some "mov"?) to encompass both
   line and file together?

2. Was change from "kernel BUG at %s:%d!\n" to "Kernel BUG at %s:%d\n"
   intentional? I'd have thought leave it as was.

3. This probably falls right outside your brief, and I do not want
   to stoke a flamewar about .config in kernel, but let me mention:
   the oops info would be significantly more helpful if it showed
   the version of compiler used, and whether CONFIG_SMP e.g. so we
   can quickly see which of the page_cache_releases in shrink_cache
   gave rise to some error, without trying out assorted compilers
   and configs - BUGVERBOSE was a nuisance for that too, now remedied.
   (Personally, I'd also like to know whether NOHIGHMEM, HIGHMEM4G or
   HIGHMEM64G, but that may reflect my own interests too much, and
   open floodgates to endless such requests.) Of course such info can
   be supplied by reporters, at the time or later, but nice if it were
   there by default like "Tainted". Ignore me unless you feel the same.

4. In your patch removing BUGs from inlines, I suggest you delete
   bug_in_interrupt: it's only used in asm/highmem.h (hmm, you check
   in_interrupt once before it, and again inside it), and those
   checks are much better moved into mm/highmem.c kmap_high and
   kunmap_high themselves - I already gave Andrea a patch with that
   change, and that part he approved. And since you're attacking
   skbuff.h, I think you can then remove interrupt.h from highmem.h,
   but include it in skbuff.h: if I remember rightly, skbuff.h is the
   only file which assumes interrupt.h has already been included by
   highmem.h. But again, ignore all these remarks unless they chime
   with your own desires - I shouldn't be playing backseat janitor
   with your time.

(Aside to Marcelo: if you're hesitant to put Andrew's changes into
2.4.18, please consider my original asm do_BUG version: it would be
a pity if 2.4.18 went out still destroying registers when BUGVERBOSE,
just because a better implementation is on the way.)

Hugh

-
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/



This archive was generated by hypermail 2b29 : Fri Feb 15 2002 - 21:00:40 EST