linux-kernel - Reducing local_bh_enable/disable overhead in irqtrace

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <1153758155.10345.8.camel@localhost.localdomain>
Date:	Mon, 24 Jul 2006 09:22:35 -0700
From:	Tim Chen <tim.c.chen@...ux.intel.com>
To:	mingo@...e.hu
Cc:	arjan@...radead.org, linux-kernel@...r.kernel.org
Subject: Reducing local_bh_enable/disable overhead in irqtrace

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@...el.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);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/