Re: [Patch 4/4] Tell GCC 4.1 to move unlikely() code to a separate section

Re: [Patch 4/4] Tell GCC 4.1 to move unlikely() code to a separate section

[Chuck Ebbert]

In-Reply-To: <200602271639.34776.ak@suse.de>

On Mon, 27 Feb 2006 16:39:34, Andi Kleen wrote:
> On Monday 27 February 2006 16:31, Arjan van de Ven wrote:
> > This patch is more controversial I assume; it offers the option 
> > to use the gcc 4.1 option to move unlikely() code to a separate section.
> > On the con side, this means that longer byte sequences are needed to jump
> > to this code, on the Pro side it means that the unlikely() code isn't sharing
> > icache cachelines and tlbs anymore.
> 
> I don't think this will do anything because the default Makefile
> still has
> 
> CFLAGS += -fno-reorder-blocks 

This also won't work for functions that specify an explicit section name,
e.g. __sched, __kprobes etc. -- at least that's what the gcc 4.0.2 docs say.

-- 
Chuck
"Equations are the Devil's sentences."  --Stephen Colbert

[Patch 0/4] Reordering of functions, try 2

[Arjan van de Ven]

Hi,


This is the second posting of the function reorder patches.
I've run a series of lmbench (3.0) runs on the patches earlier today on
this set, with the following results (I'll discuss 2 runs, one with the
first 3 patches, and one with all patches)


These are descriptions of the results, since the full lmbench run is a
LOT of numbers; this is the summary of it

1) There is a whole class of tests where it just doesn't matter at all.
In this class are the cpu measurements of FPU ops/second but also the
RAM bandwidth tests and the like. This is fully understandable; these
tests go straight from userspace->hardware, kernel changes don't impact
these.

2) In the "processor/processes" group, 7 tests changed behavior, and the
average of these changes was a performance increase by 10% (!!). The
exception was the signal handling test, which decreased by 6%. This
actually made me feel good, since the original function list was based
on a profile run that didn't do signals much if at all.

The second run (with all 4 patches) showed some fluctuations here but on
average it was in the noise region. Exception to this was the stat test,
which lost half the gain from the first 3 patches. Here also the signals
test lost

3) In the latency group, the 3-patches run is again in the 10% gain
range. Here the 4-patches run shows an additional gain for several of
the tests in the 5% range and for example the af_unix test showed a loss
compared to the 3-patches run

[Patch 4/4] Tell GCC 4.1 to move unlikely() code to a separate section

[Arjan van de Ven]

This patch is more controversial I assume; it offers the option 
to use the gcc 4.1 option to move unlikely() code to a separate section.
On the con side, this means that longer byte sequences are needed to jump
to this code, on the Pro side it means that the unlikely() code isn't sharing
icache cachelines and tlbs anymore.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
 
---
 arch/x86_64/Kconfig.debug |   10 ++++++++++
 arch/x86_64/Makefile      |    4 ++++
 2 files changed, 14 insertions(+)

Index: linux-reorder2/arch/x86_64/Kconfig.debug
===================================================================
--- linux-reorder2.orig/arch/x86_64/Kconfig.debug
+++ linux-reorder2/arch/x86_64/Kconfig.debug
@@ -12,6 +12,16 @@ config DEBUG_RODATA
 	 of the kernel code won't be covered by a 2MB TLB anymore.
 	 If in doubt, say "N".
 
+config REORDER_BLOCKS
+	bool "Enable gcc to move unlikely() code away"
+	depends on DEBUG_KERNEL
+	help
+	  This option will enable gcc 4.1 and later to reorder code that is
+	  marked unlikely() far away from the code to the end of the kernel
+	  image. This allows the processor to make better use of the instruction
+	  cache, however it also makes OOPses harder to decode.
+	  If in doubt, say "N"
+
 config IOMMU_DEBUG
        depends on GART_IOMMU && DEBUG_KERNEL
        bool "Enable IOMMU debugging"
Index: linux-reorder2/arch/x86_64/Makefile
===================================================================
--- linux-reorder2.orig/arch/x86_64/Makefile
+++ linux-reorder2/arch/x86_64/Makefile
@@ -47,6 +47,10 @@ ifneq ($(CONFIG_DEBUG_INFO),y)
 # -fweb shrinks the kernel a bit, but the difference is very small
 # it also messes up debugging, so don't use it for now.
 #CFLAGS += $(call cc-option,-fweb)
+# -freorder-blocks-and-partition moves all unlikely() code out of the
+# way, to increase icache density of the hot codepaths.
+# this makes oopses harder to read as well, so it's a config option
+cflags-$(CONFIG_REORDER_BLOCKS) += $(call cc-option,-freorder-blocks-and-partition)
 endif
 # -funit-at-a-time shrinks the kernel .text considerably
 # unfortunately it makes reading oopses harder.

Re: [Patch 4/4] Tell GCC 4.1 to move unlikely() code to a separate section

[Andi Kleen]

On Monday 27 February 2006 16:31, Arjan van de Ven wrote:
> This patch is more controversial I assume; it offers the option 
> to use the gcc 4.1 option to move unlikely() code to a separate section.
> On the con side, this means that longer byte sequences are needed to jump
> to this code, on the Pro side it means that the unlikely() code isn't sharing
> icache cachelines and tlbs anymore.

I don't think this will do anything because the default Makefile
still has

CFLAGS += -fno-reorder-blocks 

That was me because it made assembly debugging much easier. I would be willing
to reconsider this if you can give me some hard data just from this change:
- benchmark changes
- .text size increase

Also I don't like it being an separate CONFIG options. We already have too many
obscure ones. Either it should be on by default or not there at all.

-Andi

Re: [Patch 4/4] Tell GCC 4.1 to move unlikely() code to a separate section

[Bill Davidsen]

Andi Kleen wrote:
> On Monday 27 February 2006 16:31, Arjan van de Ven wrote:
>> This patch is more controversial I assume; it offers the option 
>> to use the gcc 4.1 option to move unlikely() code to a separate section.
>> On the con side, this means that longer byte sequences are needed to jump
>> to this code, on the Pro side it means that the unlikely() code isn't sharing
>> icache cachelines and tlbs anymore.
> 
> I don't think this will do anything because the default Makefile
> still has
> 
> CFLAGS += -fno-reorder-blocks 
> 
> That was me because it made assembly debugging much easier. I would be willing
> to reconsider this if you can give me some hard data just from this change:
> - benchmark changes
> - .text size increase
> 
> Also I don't like it being an separate CONFIG options. We already have too many
> obscure ones. Either it should be on by default or not there at all.

I think you just made the case for an option, if it does produce 
substantially better code and justify being done, you still would want a 
way to kill it for debugging.

I'd like to see what it gains in general, and how much it depends on 
processor type and cache size.
-- 
    -bill davidsen (davidsen@tmr.com)
"The secret to procrastination is to put things off until the
  last possible moment - but no longer"  -me