Re: [PATCH] x86: always_inline wrapper for x86's test_bit

["Alexander van Heukelum" <heukelum@fastmail.fm>]

On Mon, 14 Apr 2008 09:52:04 +0200, "Ingo Molnar" <mingo@elte.hu> said:
> the behavior on tiny is interesting - could you post the config you used 
> for your tiny kernel?

I use the following mini-config (../klibc.i386.mini). To expand it:
make allnoconfig KCONFIG_ALLCONFIG="../klibc.i386.mini"


CONFIG_EXPERIMENTAL=y
CONFIG_BLK_DEV_INITRD=y
CONFIG_INITRAMFS_SOURCE="../initramfs"
CONFIG_CC_OPTIMIZE_FOR_SIZE=y
CONFIG_EMBEDDED=y
CONFIG_KALLSYMS=y
CONFIG_PRINTK=y
CONFIG_BUG=y
CONFIG_SLOB=y
CONFIG_MODULES=y
CONFIG_MODULE_UNLOAD=y
CONFIG_MPENTIUMII=y
CONFIG_HZ_100=y
CONFIG_PHYSICAL_START=0x110000
CONFIG_PHYSICAL_ALIGN=0x10000
CONFIG_BINFMT_ELF=y
CONFIG_SERIAL_8250=y
CONFIG_SERIAL_8250_CONSOLE=y
CONFIG_PROC_FS=y
CONFIG_SYSFS=y
CONFIG_OPTIMIZE_INLINING=y


This references the file initramfs:


#####################
# Shared klibc initramfs
# See gen_init_cpio for syntax

dir dev 0755 0 0
dir lib 0755 0 0
dir lib/modules 0755 0 0
dir bin 0755 0 0

nod dev/console 0600 0 0 c 5 1
file lib/klibc-K9OvCTU5pCdyhCSkxwBjXuR7KA4.so
../klibc/usr/klibc/klibc-K9OvCTU5pCdyhCSkxw$
file init ../init 0755 0 0

file bin/sh ../klibc/usr/dash/sh.shared 0755 0 0
file bin/kinit ../klibc/usr/kinit/kinit.shared 0755 0 0
file bin/gzip ../klibc/usr/gzip/gzip 0755 0 0
file bin/cat ../klibc/usr/utils/shared/cat 0755 0 0
file bin/chroot ../klibc/usr/utils/shared/chroot 0755 0 0
file bin/cpio ../klibc/usr/utils/shared/cpio 0755 0 0
file bin/dd ../klibc/usr/utils/shared/dd 0755 0 0
file bin/false ../klibc/usr/utils/shared/false 0755 0 0
file bin/halt ../klibc/usr/utils/shared/halt 0755 0 0
file bin/kill ../klibc/usr/utils/shared/kill 0755 0 0
file bin/ln ../klibc/usr/utils/shared/ln 0755 0 0
file bin/ps ../klibc/usr/utils/shared/minips 0755 0 0
file bin/mkdir ../klibc/usr/utils/shared/mkdir 0755 0 0
file bin/mkfifo ../klibc/usr/utils/shared/mkfifo 0755 0 0
file bin/mknod ../klibc/usr/utils/shared/mknod 0755 0 0
file bin/mount ../klibc/usr/utils/shared/mount 0755 0 0
file bin/nuke ../klibc/usr/utils/shared/nuke 0755 0 0
file bin/pivot_root ../klibc/usr/utils/shared/pivot_root 0755 0 0
file bin/poweroff ../klibc/usr/utils/shared/poweroff 0755 0 0
file bin/readlink ../klibc/usr/utils/shared/readlink 0755 0 0
file bin/reboot ../klibc/usr/utils/shared/reboot 0755 0 0
file bin/sleep ../klibc/usr/utils/shared/sleep 0755 0 0
file bin/sync ../klibc/usr/utils/shared/sync 0755 0 0
file bin/true ../klibc/usr/utils/shared/true 0755 0 0
file bin/umount ../klibc/usr/utils/shared/umount 0755 0 0
file bin/uname ../klibc/usr/utils/shared/uname 0755 0 0


And ../init is a small script:


#! /bin/sh
mkdir proc
mount -t proc proc /proc
mkdir sys
mount -t sysfs sysfs /sys
bin/sh -i
reboot


This is usually the first config I try to build if I change something.
Klibc is here: git://git.kernel.org/pub/scm/libs/klibc/klibc.git
If all goes well you will be able to boot to a shell with a recent
qemu using: qemu -m 4 -smp 2 -cpu pentium2 -nographic -no-reboot
-serial stdio -cdrom arch/x86/boot/image.iso.

> what would be interesting to see is the effect of allowing gcc to 
> optimize for size (CONFIG_CC_OPTIMIZE_FOR_SIZE=y) - and compare 
> !CC_OPTIMIZE_FOR_SIZE+!OPTIMIZE_INLINING against 
> CC_OPTIMIZE_FOR_SIZE=y+OPTIMIZE_INLINING=y kernels - the size difference 
> should be _brutal_.

I see I had CONFIG_DEBUG_SECTION_MISMATCH enabled all the time.
This has quite a bit of influence on code generation! I did not
expect that. Here are the numbers for the configuration given
above for all combinations of the three settings.

Using: gcc (GCC) 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2)

CONFIG_DEBUG_SECTION_MISMATCH=y
   text    data     bss     dec     hex vmlinux/i386
 806039   81464   73728  961231   eaacf OPT_SIZE=y OPT_INL=y
 778053   81464   73728  933245   e3d7d OPT_SIZE=y OPT_INL=n
 929940   81464   73728 1085132  108ecc OPT_SIZE=n OPT_INL=y
 929541   81464   73728 1084733  108d3d OPT_SIZE=n OPT_INL=n

CONFIG_DEBUG_SECTION_MISMATCH=n
   text    data     bss     dec     hex vmlinux/i386
 768031   81464   73728  923223   e1657 OPT_SIZE=y OPT_INL=y
 763675   81464   73728  918867   e0553 OPT_SIZE=y OPT_INL=n
 904284   81464   73728 1059476  102a94 OPT_SIZE=n OPT_INL=y
 904764   81464   73728 1059956  102c74 OPT_SIZE=n OPT_INL=n

The patch still helps OPT_SIZE=y OPT_INL=y, though:
 765896   81464   73728  921088   e0e00 OPT_SIZE=y OPT_INL=y, patched
 763717   81464   73728  918909   e057d OPT_SIZE=y OPT_INL=n, patched
 904490   81464   73728 1059682  102b62 OPT_SIZE=n OPT_INL=y, patched
 904938   81464   73728 1060130  102d22 OPT_SIZE=n OPT_INL=n, patched

> > If gcc decides to emit a separate function instead of inlining it, the 
> > image can have multiple instances of this uninlined function. 
> > Moreover, gcc decides if a function is too big to be inlined using a 
> > heuristic and sometimes gets it wrong. In particular, it uninlined 
> > constant_bit_test which indeed looks a bit big, but should reduce to a 
> > single assembly instruction after constant folding.
> 
> yep, gcc could definitely improve here. But if we never give it the 
> possibility to do so (by always forcing inlining) then gcc (or any other 
> compiler) wont really be optimized for Linux in this area.
> 
> we saw that with CC_OPTIMIZE_FOR_SIZE=y well - when the Linux kernel 
> started using it then both the correctness and the quality of gcc's -Os 
> output improved.
> 
> > So I guess we should sprinkle some __always_inlines in the core parts 
> > of the kernel? The most problematic ones are easily spotted using: nm 
> > -S vmlinux|uniq -f2 -D
> 
> yes - butwe should use a separate (new) __hint_always_inline tag - and 
> keep __always_inline for the cases where the code _must_ be inlined for 
> correctness reasons. (there are a few places where we do that)

That's becoming quite long. What about __wrapper? It should only be
applied to functions that do nothing more than select an implementation
based on compile-time properties of the arguments of the function or on
configuration variables.

Greetings,
    Alexander

> 	Ingo
-- 
  Alexander van Heukelum
  heukelum@fastmail.fm

-- 
http://www.fastmail.fm - Or how I learned to stop worrying and
                          love email again