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

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

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

[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.