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: Wed, 25 Jul 2001 21:33:51 +0200	[thread overview]
Message-ID: <20010725213351.A32148@athlon.random> (raw)
In-Reply-To: <20010723162909.D822@athlon.random> <m15Oyat-000CD5C@localhost>
In-Reply-To: <m15Oyat-000CD5C@localhost>; from rusty@rustcorp.com.au on Tue, Jul 24, 2001 at 07:35:10PM +1000

On Tue, Jul 24, 2001 at 07:35:10PM +1000, Rusty Russell wrote:
> In message <20010723162909.D822@athlon.random> you write:
> > > 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.
> 
> No, you are wrong.  If I do (NOT in a hw interrupt handler):
> 
> 	local_irq_save(flags);
> 	...
> 	cpu_raise_softirq(smp_processor_id(), FOO_SOFTIRQ);
> 	...
> 	local_irq_restore(flags);
> 
> ksoftirqd won't get woken, and the FOO soft irq won't get run until

You are wrong, please check again all the code involved.

inline void cpu_raise_softirq(unsigned int cpu, unsigned int nr)
{
	__cpu_raise_softirq(cpu, nr);

	/*
	 * If we're in an interrupt or bh, we're done
	 * (this also catches bh-disabled code). We will
	 * actually run the softirq once we return from
	 * the irq or bh.
	 *
	 * Otherwise we wake up ksoftirqd to make sure we
	 * schedule the softirq soon.
	 */
	if (!(local_irq_count(cpu) | local_bh_count(cpu)))
		wakeup_softirqd(cpu);
}

If you are not in hw interrupt local_irq_count is zero so you will run
wakeup_softirqd().

The fact irq are enabled or disabled has no influence on the logic.

> the next interrupt comes in.  You solved (horribly) the analagous case
> for local_bh_disable/enable, but not this one.

I didn't changed at all local_bh_enable (except a fix for a missing
barrier()), local_bh_enable/disable was solved by Ingo in 2.4.6.

> Below as suggested in my previous email (x86 only, untested).  I also

It seems you're duplicating the local_irq_count functionalty plus you
break local_bh_enable, from local_bh_enable you want to run the softirq
immediatly.

> added a couple of comments.  There's still the issue of stack
> overflows if you get hit hard enough with interrupts (do_softirq is
> exposed to reentry for short periods), but that's separate.

do_softirq can be re-entered but on re-entry it will return immediatly
because local_bh_count will be non zero in such case, so the only stack
overhead of a re-entry is a few words so it cannot harm (if stack
overflows it cannot be because of do_softirq re-entry).

Andrea

  reply	other threads:[~2001-07-25 19:33 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
2001-07-24  9:35       ` Rusty Russell
2001-07-25 19:33         ` Andrea Arcangeli [this message]
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=20010725213351.A32148@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.