I was poking around in a vmlinux the other day and was surprised at the
amount of repetitive crap text that was in there. For example, try:
strings vmlinux|grep $PWD|wc -c
which gets some 70KB in my case - depends on strlen($PWD) obviously. The
culprit is BUG() in a static inline that is in a header file. In this
case cpp expands __FILE__ to the full path of the header file in question.
(IIRC there is a __BASEFILE__ that would be a better choice than __FILE__)
There is also some 5 to 10k worth of "kernel BUG at %s:%d!\n" scattered
through a typical vmlinux. Note that neither of these show up in [b]zImage
size since they compress to something like 99%, but they do cost memory once
the kernel is booted.
Anyway this small patch makes sure there is only one "kernel BUG..." string,
and dumps __FILE__ in favour of an address value since System.map data is
needed to make full use of the BUG() dump anyways. The memory stats of two
otherwise identical kernels:
Memory: 22004k/24576k available (1163k kernel code, 2184k reserved,
309k data, 64k init, 0k highmem)
Memory: 22076k/24576k available (1163k kernel code, 2112k reserved,
238k data, 64k init, 0k highmem)
so the lightweight BUG() nets a worthwhile (IMHO) saving of 72KB for this
particular kernel. (Which is even more than the __init goodies recover.)
Patch is against 2.4.2pre3 and is fully contained within i386 files.
I can supply a 2.2.x version of the patch if there is demand for it.
Paul.
diff -ur linux~/arch/i386/kernel/i386_ksyms.c linux/arch/i386/kernel/i386_ksyms.c
--- linux-g/arch/i386/kernel/i386_ksyms.c Thu Jan 11 09:06:12 2001
+++ linux-g-bug/arch/i386/kernel/i386_ksyms.c Thu Feb 15 03:50:18 2001
@@ -78,6 +78,7 @@
EXPORT_SYMBOL_NOVERS(__down_write_failed);
EXPORT_SYMBOL_NOVERS(__down_read_failed);
EXPORT_SYMBOL_NOVERS(__rwsem_wake);
+EXPORT_SYMBOL_NOVERS(bugstring);
/* Networking helper routines. */
EXPORT_SYMBOL(csum_partial_copy_generic);
/* Delay loops */
diff -ur linux~/arch/i386/kernel/setup.c linux/arch/i386/kernel/setup.c
--- linux-g/arch/i386/kernel/setup.c Wed Feb 14 02:39:58 2001
+++ linux-g-bug/arch/i386/kernel/setup.c Thu Feb 15 04:13:31 2001
@@ -105,6 +105,8 @@
char ignore_irq13; /* set if exception 16 works */
struct cpuinfo_x86 boot_cpu_data = { 0, 0, 0, 0, -1, 1, 0, 0, -1 };
+const char *bugstring="<1>kernel BUG at 0x%p\n";
+
unsigned long mmu_cr4_features;
/*
diff -ur linux~/include/asm-i386/page.h linux/include/asm-i386/page.h
--- linux-g/include/asm-i386/page.h Sat Dec 16 04:02:20 2000
+++ linux-g-bug/include/asm-i386/page.h Thu Feb 15 04:25:29 2001
@@ -86,10 +86,13 @@
* Tell the user there is some problem. Beep too, so we can
* see^H^H^Hhear bugs in early bootup as well!
*/
-#define BUG() do { \
- printk("kernel BUG at %s:%d!\n", __FILE__, __LINE__); \
+extern const char *bugstring; /* ...is in i386/kernel/setup.c - Paul G. */
+#define BUG() ({ \
+ __label__ bugaddr; \
+bugaddr: \
+ printk(bugstring, &&bugaddr); \
__asm__ __volatile__(".byte 0x0f,0x0b"); \
-} while (0)
+})
#define PAGE_BUG(page) do { \
BUG(); \
_________________________________________________________
Do You Yahoo!?
Get your free @yahoo.com address at http://mail.yahoo.com
Paul Gortmaker wrote:
>
> Anyway this small patch makes sure there is only one "kernel BUG..." string,
> and dumps __FILE__ in favour of an address value since System.map data is
> needed to make full use of the BUG() dump anyways. The memory stats of two
> otherwise identical kernels:
>
Shouldn't the linker drop duplicate strings?
info gcc mentions that gcc merges duplicate strings, but it seems that
it doesn't work (gcc-2.96-69, binutils-2.10.0.18-1).
--
Manfred
On Sat, 17 Feb 2001 around 13:26:52 +0100, Manfred Spraul wrote:
> Paul Gortmaker wrote:
> >
> > Anyway this small patch makes sure there is only one "kernel BUG..." string,
> > and dumps __FILE__ in favour of an address value since System.map data is
> > needed to make full use of the BUG() dump anyways. The memory stats of two
> > otherwise identical kernels:
> >
>
> Shouldn't the linker drop duplicate strings?
Yes, but that wasn't his patch. He split off the
constant text in the bugline to a separate string and
made sure /that/ string was used only once.
The old way contained the 'kernel BUG at' string a
zillion times, now it only contains 'bla.c: 412'
and variants thereof a lot of times. (And most of
them are NO duplicates, or you would have had 2 BUGs
at the same line...
Met vriendelijke groet,
Pauline Middelink
--
GPG Key fingerprint = 2D5B 87A7 DDA6 0378 5DEA BD3B 9A50 B416 E2D0 C3C2
For more details look at my website http://www.polyware.nl/~middelink
On Sat, 17 Feb 2001, Paul Gortmaker wrote:
> I was poking around in a vmlinux the other day and was surprised at the
> amount of repetitive crap text that was in there. For example, try:
>
> strings vmlinux|grep $PWD|wc -c
>
> which gets some 70KB in my case - depends on strlen($PWD) obviously. The
> culprit is BUG() in a static inline that is in a header file. In this
> case cpp expands __FILE__ to the full path of the header file in question.
Well done, dammit! I've been sitting on that observation
for a couple of weeks, now you've beaten me to the patch.
gcc 2.97 (snapshot) does a much better job here, eliminating the
strings from the many objects in which those inline functions are not
used. But that still leaves quite a lot of full pathnames of build
tree header files in the resultant vmlinux. And it'll be quite some
while before gcc 3.0 becomes the choice for building the kernel.
> (IIRC there is a __BASEFILE__ that would be a better choice than __FILE__)
Not that I've found.
> There is also some 5 to 10k worth of "kernel BUG at %s:%d!\n" scattered
> through a typical vmlinux. Note that neither of these show up in [b]zImage
> size since they compress to something like 99%, but they do cost memory once
> the kernel is booted.
Indeed.
> Anyway this small patch makes sure there is only one "kernel BUG..." string,
> and dumps __FILE__ in favour of an address value since System.map data is
> needed to make full use of the BUG() dump anyways. The memory stats of two
> otherwise identical kernels:
Well, in many cases, no System.map data is needed to decipher a BUG():
a particular problem can quickly become familiar just by its file:line;
which a reliance on System.map might sometimes obscure.
I was leaving BUG()'s printk format unchanged (though shared); but
inline functions in header files using INLINE_BUG() instead, its
format "kernel BUG inlined from header:%d!\n" i.e. no __FILE__, but
peculiarly __LINE__ even so to help identify familiar bugs quickly.
That never quite satisfied me. I think the best would be to
combine approaches: keep BUG() as it was (though sharing format),
substitute INLINE_BUG() in inline functions, but use your macro
(with 0x%p address) for it instead of mine. What do others think?
Keith, does the address format need adjusting to suit ksymoops?
These changes need not be kept to i386,
but like you I hadn't yet gone further.
I'd very much like to see some such changes go into Linus' tree.
I'm fortunate that I can afford the waste of memory: what bothers
me about those strings is, I'm interested in whether objects built
with different CONFIG options are equivalent or not, and those
strings make that hard to establish (I'm thinking particularly
of CONFIG_NOHIGHMEM v. CONFIG_HIGHMEM4G v. CONFIG_HIGHMEM64G,
which pull in different header files with inlined BUG()s).
Paul, I apologize if I seem to be trying to steal your thunder:
just a subject close to my heart right now!
Hugh
On 02.17 Hugh Dickins wrote:
> On Sat, 17 Feb 2001, Paul Gortmaker wrote:
> > I was poking around in a vmlinux the other day and was surprised at the
> > amount of repetitive crap text that was in there. For example, try:
> >
> > strings vmlinux|grep $PWD|wc -c
> >
If you try
strings vmlinux|grep /usr
you get a bunch of strings like:
..
/usr/src/linux/include/asm/io.h
..
One other couple of Kb. The problem is not that, but the string comes from:
/usr/src/linux/include/asm/io.h:
..(line 110)
/*
* Temporary debugging check to catch old code using
* unmapped ISA addresses. Will be removed in 2.4.
*/
#if 1
extern void *__io_virt_debug(unsigned long x, const char *file, int line);
extern unsigned long __io_phys_debug(unsigned long x, const char *file, int li
ne);
#define __io_virt(x) __io_virt_debug((unsigned long)(x), __FILE__, __LINE__)
//#define __io_phys(x) __io_phys_debug((unsigned long)(x), __FILE__, __LINE__)
#else
#define __io_virt(x) ((void *)(x))
//#define __io_phys(x) __pa(x)
#endif
..
As you see, it was not removed in 2.4...
--
J.A. Magallon $> cd pub
mailto:[email protected] $> more beer
Linux werewolf 2.4.1-ac17 #1 SMP Sat Feb 17 01:47:56 CET 2001 i686
On 02.17 J . A . Magallon wrote:
> #if 1
> extern void *__io_virt_debug(unsigned long x, const char *file, int line);
> extern unsigned long __io_phys_debug(unsigned long x, const char *file, int
> li
> ne);
> #define __io_virt(x) __io_virt_debug((unsigned long)(x), __FILE__, __LINE__)
> //#define __io_phys(x) __io_phys_debug((unsigned long)(x), __FILE__, __LINE__)
> #else
> #define __io_virt(x) ((void *)(x))
> //#define __io_phys(x) __pa(x)
> #endif
> ..
Loking at it (arch/i386/lib/iodebug.c):
void * __io_virt_debug(unsigned long x, const char *file, int line)
{
if (x < PAGE_OFFSET) {
printk("io mapaddr 0x%05lx not valid at %s:%d!\n", x, file, line);
return __va(x);
}
return (void *)x;
}
is changed (if you turn off the #if 1), from 1_function_call+1_if+cache
pollution with the function code to nothing (just a cast).
This will make some difference in performance, won't it ?
--
J.A. Magallon $> cd pub
mailto:[email protected] $> more beer
Linux werewolf 2.4.1-ac17 #1 SMP Sat Feb 17 01:47:56 CET 2001 i686
On 02.17 Paul Gortmaker wrote:
> I was poking around in a vmlinux the other day and was surprised at the
> amount of repetitive crap text that was in there. For example, try:
>
> strings vmlinux|grep $PWD|wc -c
>
> which gets some 70KB in my case - depends on strlen($PWD) obviously. The
> culprit is BUG() in a static inline that is in a header file. In this
> case cpp expands __FILE__ to the full path of the header file in question.
> (IIRC there is a __BASEFILE__ that would be a better choice than __FILE__)
>
Or better __FUNCTION__. Or even better __func__ that is gcc and ANSI99 C
compatible.
Time to make a patch...
--
J.A. Magallon $> cd pub
mailto:[email protected] $> more beer
Linux werewolf 2.4.1-ac17 #1 SMP Sat Feb 17 01:47:56 CET 2001 i686
On Sat, 17 Feb 2001, J . A . Magallon wrote:
> On 02.17 J . A . Magallon wrote:
> > #if 1
> > extern void *__io_virt_debug(unsigned long x, const char *file, int line);
> > extern unsigned long __io_phys_debug(unsigned long x, const char *file, int
> > li
> > ne);
> > #define __io_virt(x) __io_virt_debug((unsigned long)(x), __FILE__, __LINE__)
> > //#define __io_phys(x) __io_phys_debug((unsigned long)(x), __FILE__, __LINE__)
> > #else
> > #define __io_virt(x) ((void *)(x))
> > //#define __io_phys(x) __pa(x)
> > #endif
> > ..
>
> Loking at it (arch/i386/lib/iodebug.c):
> void * __io_virt_debug(unsigned long x, const char *file, int line)
> {
> if (x < PAGE_OFFSET) {
> printk("io mapaddr 0x%05lx not valid at %s:%d!\n", x, file, line);
> return __va(x);
> }
> return (void *)x;
> }
>
> is changed (if you turn off the #if 1), from 1_function_call+1_if+cache
> pollution with the function code to nothing (just a cast).
> This will make some difference in performance, won't it ?
Speaking of io_virt_debug... This patch that makes all your MMIO
devices run faster, simply by eliminating io_virt_debug :)
(sorry for the CONFIG_VMDUMP pollution in config.in... I'm too slack
to rediff nicely)
Jeff
Index: linux_2_4/arch/i386/config.in
diff -u linux_2_4/arch/i386/config.in:1.1.1.20 linux_2_4/arch/i386/config.in:1.1.1.20.2.2
--- linux_2_4/arch/i386/config.in:1.1.1.20 Fri Feb 9 15:23:38 2001
+++ linux_2_4/arch/i386/config.in Fri Feb 9 19:15:10 2001
@@ -365,5 +365,7 @@
comment 'Kernel hacking'
#bool 'Debug kmalloc/kfree' CONFIG_DEBUG_MALLOC
+bool 'I/O read/write debugging' CONFIG_DEBUG_IOVIRT
bool 'Magic SysRq key' CONFIG_MAGIC_SYSRQ
+bool 'Support kernel crash dump capabilities' CONFIG_VMDUMP
endmenu
Index: linux_2_4/arch/i386/kernel/i386_ksyms.c
diff -u linux_2_4/arch/i386/kernel/i386_ksyms.c:1.1.1.14 linux_2_4/arch/i386/kernel/i386_ksyms.c:1.1.1.14.2.1
--- linux_2_4/arch/i386/kernel/i386_ksyms.c:1.1.1.14 Fri Feb 9 15:23:46 2001
+++ linux_2_4/arch/i386/kernel/i386_ksyms.c Fri Feb 9 15:52:23 2001
@@ -59,7 +60,9 @@
EXPORT_SYMBOL(dump_extended_fpu);
EXPORT_SYMBOL(__ioremap);
EXPORT_SYMBOL(iounmap);
+#ifdef CONFIG_DEBUG_IOVIRT
EXPORT_SYMBOL(__io_virt_debug);
+#endif
EXPORT_SYMBOL(enable_irq);
EXPORT_SYMBOL(disable_irq);
EXPORT_SYMBOL(disable_irq_nosync);
Index: linux_2_4/arch/i386/lib/Makefile
diff -u linux_2_4/arch/i386/lib/Makefile:1.1.1.11 linux_2_4/arch/i386/lib/Makefile:1.1.1.11.2.1
--- linux_2_4/arch/i386/lib/Makefile:1.1.1.11 Fri Feb 9 15:23:43 2001
+++ linux_2_4/arch/i386/lib/Makefile Fri Feb 9 15:52:23 2001
@@ -8,10 +8,11 @@
L_TARGET = lib.a
obj-y = checksum.o old-checksum.o delay.o \
- usercopy.o getuser.o putuser.o iodebug.o \
+ usercopy.o getuser.o putuser.o \
memcpy.o
obj-$(CONFIG_X86_USE_3DNOW) += mmx.o
obj-$(CONFIG_HAVE_DEC_LOCK) += dec_and_lock.o
+obj-$(CONFIG_DEBUG_IOVIRT) += iodebug.o
include $(TOPDIR)/Rules.make
Index: linux_2_4/include/asm-i386/io.h
diff -u linux_2_4/include/asm-i386/io.h:1.1.1.1 linux_2_4/include/asm-i386/io.h:1.1.1.1.118.1
--- linux_2_4/include/asm-i386/io.h:1.1.1.1 Sun Oct 22 12:36:21 2000
+++ linux_2_4/include/asm-i386/io.h Fri Feb 9 15:52:29 2001
@@ -1,6 +1,8 @@
#ifndef _ASM_IO_H
#define _ASM_IO_H
+#include <linux/config.h>
+
/*
* This file contains the definitions for the x86 IO instructions
* inb/inw/inl/outb/outw/outl and the "string versions" of the same
@@ -111,7 +113,7 @@
* Temporary debugging check to catch old code using
* unmapped ISA addresses. Will be removed in 2.4.
*/
-#if 1
+#if CONFIG_DEBUG_IOVIRT
extern void *__io_virt_debug(unsigned long x, const char *file, int line);
extern unsigned long __io_phys_debug(unsigned long x, const char *file, int line);
#define __io_virt(x) __io_virt_debug((unsigned long)(x), __FILE__, __LINE__)
On Sat, 17 Feb 2001 13:15:42 +0000 (GMT),
Hugh Dickins <[email protected]> wrote:
>On Sat, 17 Feb 2001, Paul Gortmaker wrote:
>> Anyway this small patch makes sure there is only one "kernel BUG..." string,
>> and dumps __FILE__ in favour of an address value since System.map data is
>> needed to make full use of the BUG() dump anyways. The memory stats of two
>> otherwise identical kernels:
>substitute INLINE_BUG() in inline functions, but use your macro
>(with 0x%p address) for it instead of mine. What do others think?
>Keith, does the address format need adjusting to suit ksymoops?
I would prefer to see
extern const char *kernel_bug;
printk(kernel_bug, __C_FILE__, __C_LINE__)
init/main.c contains
const char *kernel_bug = "kernel BUG at %s:%d!\n";
kernel_bug is also exported.
__C_FILE__ and __C_LINE__ refer to the .c or .s file that included the
header, so you get the exact location of the failing code instead of
the name and line number of a common header which is used all over the
place. __C_FILE__ would be replaced with the minimum string required
to uniquely identify the file, e.g. isdn_audio.c (unique at one level)
or romfs/inode.c (unique at two levels).
Standard gcc will not do this. But as part of my makefile rewrite I am
reading the pre-processed output as it is generated by cpp and before
it is read by cc1, to extract data for module symbol versions. It is
trivial for my code to look for __C_FILE__ and __C_LINE__ (cpp will
have left them alone) and replace them with the relevant string and
number.
Would people prefer the C/ASM filename in BUG() messages instead of the
include header? If so I will add it to my todo list for the makefile
rewrite. Of course you can still use __FILE__ and __LINE_ if you
really want to report the error against the include file.
On 02.18 Keith Owens wrote:
>
> __C_FILE__ and __C_LINE__ refer to the .c or .s file that included the
> header, so you get the exact location of the failing code instead of
> the name and line number of a common header which is used all over the
> place. __C_FILE__ would be replaced with the minimum string required
Are you sure about that ?
Try this:
a.h:
#define hello printf("%d at %s\n",__LINE__,__FILE__)
a.c:
#include <stdio.h>
#include "a.h"
int main()
{
hello;
hello;
return 0;
}
werewolf:~/ko> gcc a.c -o a
werewolf:~/ko> a
6 at a.c
7 at a.c
(line changes, and name is .c)
because __FILE__ and __LINE__ are expanded with respect to the current
SOURCE file (see info cpp), not header file.
As I said before, my choose would be the __func__ + __LINE__ predefined macros.
I would prefer to see 'bug in my_buggy_device_init(), line 42'. And you can
even drop the __LINE__ part.
--
J.A. Magallon $> cd pub
mailto:[email protected] $> more beer
Linux werewolf 2.4.1-ac17 #1 SMP Sat Feb 17 01:47:56 CET 2001 i686
On Sun, 18 Feb 2001 01:33:53 +0100,
"J . A . Magallon" <[email protected]> wrote:
>Try this:
>a.h:
>#define hello printf("%d at %s\n",__LINE__,__FILE__)
>
>a.c:
>#include <stdio.h>
>#include "a.h"
>
>int main()
>{
> hello;
> hello;
> return 0;
>}
>
>werewolf:~/ko> gcc a.c -o a
>werewolf:~/ko> a
>6 at a.c
>7 at a.c
But ....
a.h
static inline void hello(void) { printf("%d at %s\n",__LINE__,__FILE__); }
a.c
#include <stdio.h>
#include "a.h"
int main()
{
hello();
hello();
return 0;
}
# ./a
1 at a.h
1 at a.h
Most uses of BUG() in headers use inline functions instead of #define.
48 occurrences of BUG() in include/{linux,asm-i386}, only 2 are in
#define.
>As I said before, my choose would be the __func__ + __LINE__ predefined macros.
>I would prefer to see 'bug in my_buggy_device_init(), line 42'. And you can
>even drop the __LINE__ part.
Function names are not unique, especially when you get into modules.
Keith Owens wrote:
>
> But ....
>
> a.h
> static inline void hello(void) { printf("%d at %s\n",__LINE__,__FILE__); }
>
> a.c
> #include <stdio.h>
> #include "a.h"
>
> int main()
> {
> hello();
> hello();
> return 0;
> }
>
> # ./a
> 1 at a.h
> 1 at a.h
>
__BASE_FILE__ does this. It expands to the thing which you
typed on the `gcc' command line.
bix:/home/morton> cat a.h
static inline void hello(void)
{
printf("%d at %s\n",__LINE__,__BASE_FILE__);
}
bix:/home/morton> cat a.c
#include <stdio.h>
#include "a.h"
int main()
{
hello();
hello();
return 0;
}
bix:/home/morton> gcc -O a.c
bix:/home/morton> ./a.out
3 at a.c
3 at a.c
On 02.18 Andrew Morton wrote:
>
> __BASE_FILE__ does this. It expands to the thing which you
> typed on the `gcc' command line.
>
..
> 3 at a.c
> 3 at a.c
I also thought that, but look at the line numbers...wrong and repeated.
--
J.A. Magallon $> cd pub
mailto:[email protected] $> more beer
Linux werewolf 2.4.1-ac17 #1 SMP Sat Feb 17 01:47:56 CET 2001 i686
"J . A . Magallon" wrote:
>
> On 02.18 Andrew Morton wrote:
> >
> > __BASE_FILE__ does this. It expands to the thing which you
> > typed on the `gcc' command line.
> >
> ..
> > 3 at a.c
> > 3 at a.c
>
> I also thought that, but look at the line numbers...wrong and repeated.
Sure. There's no __BASE_LINE__.
I don't think providing file-n-line in BUG() provides much utility. Just
remove it. Replace it with "please feed the following into ksymoops".
On Sun, 18 Feb 2001 12:33:35 +1100,
Andrew Morton <[email protected]> wrote:
>__BASE_FILE__ does this. It expands to the thing which you
>typed on the `gcc' command line.
>
>bix:/home/morton> ./a.out
>3 at a.c
>3 at a.c
But __LINE__ is wrong. Forget what I said about __C_FILE__ and
__C_LINE__, __C_LINE__ would not work for inline functions. Looks like
the best option is a combination of __BASE_FILE__ and function name.
a.h
#define BUG() \
printf("kernel BUG in func %s, file %s\n",__FUNCTION__,__BASE_FILE__);
static inline void hello(void)
{
BUG();
}
a.c
#include <stdio.h>
#include <a.h>
int main()
{
hello();
hello();
return 0;
}
# gcc -I`pwd` `pwd`/a.c -o a
# ./a
kernel BUG in func hello, file /home/kaos/a.c
kernel BUG in func hello, file /home/kaos/a.c
Keith Owens <[email protected]> writes:
>
> Would people prefer the C/ASM filename in BUG() messages instead of the
> include header? If so I will add it to my todo list for the makefile
> rewrite. Of course you can still use __FILE__ and __LINE_ if you
> really want to report the error against the include file.
I think include file name makes more sense, otherwise you'll have a hard time
to find the actual BUG check. If someone wants more they can decode the oops.
-Andi