Reducing local_bh_enable/disable overhead in irqtrace

Reducing local_bh_enable/disable overhead in irqtrace

[Tim Chen]

Ingo,

The recent changes from irqtrace feature has added overheads to
local_bh_disable and local_bh_enable that reduces UDP performance across
x86_64 and IA64, even though IA64 does not support the irqtrace feature.
Patch in question is

[PATCH]lockdep: irqtrace subsystem, core
http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=c
ommit;h=de30a2b355ea85350ca2f58f3b9bf4e5bc007986

Prior to this patch, local_bh_disable was a short macro. Now it is a
function which calls  __local_bh_disable with added irq flags save and
restore.  The irq flags save and restore were also added to
local_bh_enable, probably for injecting the trace irqs code.  This
overhead is on the generic code path across all architectures.  On a
IA_64 test machine (Itanium-2 1.6 GHz) running a benchmark like
netperf's UDP streaming test, the added overhead results in a drop of 3%
in throughput, as udp_sendmsg calls the local_bh_enable/disable several
times. Other workloads that have heavy usages of local_bh_enable/disable
could also be affected. The patch ideally should not have affected IA-64
performance as it does not have IRQ tracing support.  A significant
portion of the overhead is in the added irq flags save and restore,
which I think is not needed if IRQ tracing is unused. A suggested patch
is attached below that recovers the lost performance.  However, the
"ifdef"s in the patch are a bit ugly.

Tim Chen

Signed-off-by: Tim Chen <tim.c.chen@intel.com>
--- a/kernel/softirq.c.orig	2006-07-19 12:18:29.000000000 -0700
+++ b/kernel/softirq.c	2006-07-21 08:34:42.000000000 -0700
@@ -65,6 +65,7 @@
  * This one is for softirq.c-internal use,
  * where hardirqs are disabled legitimately:
  */
+#ifdef CONFIG_TRACE_IRQFLAGS
 static void __local_bh_disable(unsigned long ip)
 {
 	unsigned long flags;
@@ -80,6 +81,13 @@
 		trace_softirqs_off(ip);
 	raw_local_irq_restore(flags);
 }
+#else /* !CONFIG_TRACE_IRQFLAGS */
+static inline void __local_bh_disable(unsigned long ip) 
+{
+	add_preempt_count(SOFTIRQ_OFFSET); 
+	barrier(); 
+}
+#endif /* CONFIG_TRACE_IRQFLAGS */
 
 void local_bh_disable(void)
 {
@@ -121,12 +129,16 @@
 
 void local_bh_enable(void)
 {
+#ifdef CONFIG_TRACE_IRQFLAGS
 	unsigned long flags;
 
 	WARN_ON_ONCE(in_irq());
+#endif
 	WARN_ON_ONCE(irqs_disabled());
 
+#ifdef CONFIG_TRACE_IRQFLAGS
 	local_irq_save(flags);
+#endif
 	/*
 	 * Are softirqs going to be turned on now:
 	 */
@@ -142,18 +154,22 @@
 		do_softirq();
 
 	dec_preempt_count();
+#ifdef CONFIG_TRACE_IRQFLAGS
 	local_irq_restore(flags);
+#endif
 	preempt_check_resched();
 }
 EXPORT_SYMBOL(local_bh_enable);
 
 void local_bh_enable_ip(unsigned long ip)
 {
+#ifdef CONFIG_TRACE_IRQFLAGS
 	unsigned long flags;
 
 	WARN_ON_ONCE(in_irq());
 
 	local_irq_save(flags);
+#endif
 	/*
 	 * Are softirqs going to be turned on now:
 	 */
@@ -169,7 +185,9 @@
 		do_softirq();
 
 	dec_preempt_count();
+#ifdef CONFIG_TRACE_IRQFLAGS
 	local_irq_restore(flags);
+#endif
 	preempt_check_resched();
 }
 EXPORT_SYMBOL(local_bh_enable_ip);

Re: Reducing local_bh_enable/disable overhead in irqtrace

[Ingo Molnar]

* Tim Chen <tim.c.chen@linux.intel.com> wrote:

> Ingo,
> 
> The recent changes from irqtrace feature has added overheads to 
> local_bh_disable and local_bh_enable that reduces UDP performance 
> across x86_64 and IA64, even though IA64 does not support the irqtrace 
> feature. Patch in question is
> 
> [PATCH]lockdep: irqtrace subsystem, core
> http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=c
> ommit;h=de30a2b355ea85350ca2f58f3b9bf4e5bc007986
> 
> Prior to this patch, local_bh_disable was a short macro. Now it is a 
> function which calls __local_bh_disable with added irq flags save and 
> restore.  The irq flags save and restore were also added to 
> local_bh_enable, probably for injecting the trace irqs code.  This 
> overhead is on the generic code path across all architectures.  On a 
> IA_64 test machine (Itanium-2 1.6 GHz) running a benchmark like 
> netperf's UDP streaming test, the added overhead results in a drop of 
> 3% in throughput, as udp_sendmsg calls the local_bh_enable/disable 
> several times. Other workloads that have heavy usages of 
> local_bh_enable/disable could also be affected. The patch ideally 
> should not have affected IA-64 performance as it does not have IRQ 
> tracing support.  A significant portion of the overhead is in the 
> added irq flags save and restore, which I think is not needed if IRQ 
> tracing is unused. A suggested patch is attached below that recovers 
> the lost performance.  However, the "ifdef"s in the patch are a bit 
> ugly.

agreed - this side-effect of irqtracing should not happen. The #ifdefs 
are ugly, but i can see no better way either.

> Signed-off-by: Tim Chen <tim.c.chen@intel.com>

Acked-by: Ingo Molnar <mingo@elte.hu>

	Ingo