Re: 2.4.7 softirq incorrectness.

[Andrea Arcangeli]

On Mon, Jul 23, 2001 at 07:06:40PM +1000, Rusty Russell wrote:
> Aside: why does it do a local_irq_save() if it's always run from an
> interrupt handler?

to avoid corrupting the backlog with nested irqs.

However if you want a microoptimization is to sti before
__cpu_raise_softirq, __cpu_raise_softirq from 2.4.7 is required to be
atomic with respect of irqs (but it doesn't need to be atomic with
respect of SMP). in the x86 port is handled as a single not locked bts
instruction. So it can be run with irq enabled.

Here the optimization:

--- 2.4.7aa1/net/core/dev.c.~1~	Sat Jul 21 00:04:34 2001
+++ 2.4.7aa1/net/core/dev.c	Mon Jul 23 16:21:35 2001
@@ -1217,10 +1217,10 @@
 enqueue:
 			dev_hold(skb->dev);
 			__skb_queue_tail(&queue->input_pkt_queue,skb);
+			local_irq_restore(flags);
 
 			/* Runs from irqs or BH's, no need to wake BH */
 			__cpu_raise_softirq(this_cpu, NET_RX_SOFTIRQ);
-			local_irq_restore(flags);
 #ifndef OFFLINE_SAMPLE
 			get_sample_stats(this_cpu);
 #endif
@@ -1529,10 +1529,10 @@
 
 	local_irq_disable();
 	netdev_rx_stat[this_cpu].time_squeeze++;
+	local_irq_enable();
 
 	/* This already runs in BH context, no need to wake up BH's */
 	__cpu_raise_softirq(this_cpu, NET_RX_SOFTIRQ);
-	local_irq_enable();
 
 	NET_PROFILE_LEAVE(softnet_process);
 	return;

> > I cannot see any problem.
> 
> Why not fix all the cases?  Why have this wierd secret rule that
> cpu_raise_softirq() should not be called with irqs disabled?

cpu_raise_softirq _can_ be called with irq disabled too just now, irq
enabled or disabled has no influence at all on cpu_raise_softirq.

The fact you are running on a irq handler or not has influence instead,
if you are running in a irq handler do_IRQ will take care of the
latency, if you are running in normal kernel code ksoftirqd will take
care of the latency, and both cases are handled perfectly right.

> Call me old-fashioned, but why not *fix* the problem, if you're going
> to rewrite this code... again...

There's no problem at all to fix, everything is just fine from 2.4.7,
period.

Andrea