All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrea Arcangeli <andrea@suse.de>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: torvalds@transmeta.com, linux-kernel@vger.kernel.org
Subject: Re: 2.4.7 softirq incorrectness.
Date: Mon, 23 Jul 2001 16:29:09 +0200	[thread overview]
Message-ID: <20010723162909.D822@athlon.random> (raw)
In-Reply-To: <20010723013416.B23517@athlon.random> <m15Obfk-000CD5C@localhost>
In-Reply-To: <m15Obfk-000CD5C@localhost>; from rusty@rustcorp.com.au on Mon, Jul 23, 2001 at 07:06:40PM +1000

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

  parent reply	other threads:[~2001-07-23 14:29 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2001-07-22 20:44 2.4.7 softirq incorrectness Rusty Russell
2001-07-22 23:34 ` Andrea Arcangeli
2001-07-23  9:06   ` Rusty Russell
2001-07-23 12:05     ` David S. Miller
2001-07-23 14:31       ` Andrea Arcangeli
2001-07-23 14:29     ` Andrea Arcangeli [this message]
2001-07-24  9:35       ` Rusty Russell
2001-07-25 19:33         ` Andrea Arcangeli
2001-07-26 20:26           ` Rusty Russell
2001-07-23  9:25   ` Kai Germaschewski
2001-07-23 11:12     ` Jeff Garzik
2001-07-23 14:18     ` Andrea Arcangeli
2001-07-23 22:24   ` Alexey Kuznetsov
2001-07-25 22:23     ` Andrea Arcangeli
2001-07-26 17:46       ` kuznet
2001-07-26 18:03         ` Jeff Garzik
2001-07-26 18:29         ` Andrea Arcangeli
2001-07-27 16:48           ` kuznet
2001-07-27  0:47         ` Maksim Krasnyanskiy
2001-07-27 15:01           ` Andrea Arcangeli
2001-07-27 18:31           ` Maksim Krasnyanskiy
2001-07-27 18:59             ` kuznet
2001-07-27 19:21             ` Maksim Krasnyanskiy
2001-07-27 19:35               ` kuznet
2001-07-28  0:52               ` [PATCH] [IMPORTANT] " Maksim Krasnyanskiy
2001-07-28 17:41                 ` kuznet
2001-07-28 18:02                   ` Andrea Arcangeli
2001-07-28 19:02                     ` kuznet
2001-07-28 19:32                       ` Andrea Arcangeli
2001-07-28 23:28                         ` Alexey Kuznetsov
2001-07-29 17:07                           ` Linus Torvalds
2001-07-29 17:52                             ` kuznet
2001-07-30 18:50                               ` Ingo Molnar
2001-07-30 22:47                                 ` Nigel Gamble
2001-07-30 22:56                                   ` Christoph Hellwig
2001-07-31 18:08                                 ` kuznet
2001-07-28 17:54                 ` Andrea Arcangeli
2001-07-28 19:17                   ` Andrea Arcangeli
2001-07-30 18:32                 ` Maksim Krasnyanskiy
2001-07-27  9:34         ` David S. Miller
2001-07-27 17:01           ` kuznet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20010723162909.D822@athlon.random \
    --to=andrea@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rusty@rustcorp.com.au \
    --cc=torvalds@transmeta.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.