public inbox for linux-arch@vger.kernel.org
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@elte.hu>, Thomas Gleixner <tglx@linutronix.de>,
	lkml <linux-kernel@vger.kernel.org>,
	linux-arch <linux-arch@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [RFC][PATCH] irq: remove IRQF_DISABLED
Date: Mon, 2 Mar 2009 10:29:12 -0800 (PST)	[thread overview]
Message-ID: <alpine.LFD.2.00.0903021012080.3111@localhost.localdomain> (raw)
In-Reply-To: <200903021855.02765.bzolnier@gmail.com>



On Mon, 2 Mar 2009, Bartlomiej Zolnierkiewicz wrote:
> > 
> > Could we make just the IDE driver itself enable interrupts? Sure. But that 
> 
> Actually it has been doing it for years (some host drivers don't do this by
> default and still need "hdparm -u" or equivalent but I was planning to change
> it for 2.6.30).

The IDE layer has the option to enable irq's during the transfer itself, 
yes.  But it actually works the reverse way from what you think: the irq 
layer will enable interrupts, and the IDE layer will then _not_ disable 
them during the transfer if you use "hdparm -u".

Look at ide_intr: it generally gets called with interrupts _enabled_ 
(because it doesn't use IRQF_DISABLED) and then it does:

        spin_lock_irqsave(&hwif->lock, flags);
	..
        spin_unlock(&hwif->lock);
	..
        if (drive->dev_flags & IDE_DFLAG_UNMASK)
                local_irq_enable_in_hardirq();
	...
	spin_lock_irq(&hwif->lock);
	...
	spin_unlock_irqrestore(&hwif->lock, flags);

where the magic thing is how it enables irqs again if the "irq unmask" 
flag is set.

The point I'm making is that 

 - as far as the generic irq layer is concerned, IDE might as well have 
   interrupts enabled all the time (and disabling them is a local issue, 
   more to do with locking and with timing-induced hardware _bugs_ rather 
   than anything else)

 - .. and more importantly, that is AS IT MUST BE. Because quite frankly, 
   if the irq handler enables interrupts (like IDE does), the generic IRQ 
   layer really _must_ know about it, because it may depend on 
   non-reentrancy of that interrupt.

(Small detail: the current irq layer actually does have that 
"IRQ_INPROGRESS" flag to handle re-entrancy issues regardless of anything 
else, so I guess we technically are robust in this regard. But that's 
partly an SMP issue, and conceptually it's still really really important 
information to know whether interrupts can nest. In practice, it does 
affect things like stack usage too, for example, so even with 
IRQ_INPROGRESS, there really is a difference between IRQF_DISABLED and not 
having it on).

		Linus

  parent reply	other threads:[~2009-03-02 18:30 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-03-02 12:21 [RFC][PATCH] irq: remove IRQF_DISABLED Peter Zijlstra
2009-03-02 14:43 ` Ingo Molnar
2009-03-02 15:47   ` Matthew Wilcox
2009-03-02 15:59     ` Peter Zijlstra
2009-03-02 16:32       ` Matthew Wilcox
2009-03-02 21:01       ` Russell King
2009-03-02 21:17         ` Ingo Molnar
2009-03-02 21:17           ` Ingo Molnar
2009-03-02 17:11 ` Linus Torvalds
2009-03-02 17:55   ` Bartlomiej Zolnierkiewicz
2009-03-02 18:02     ` Peter Zijlstra
2009-03-02 18:07     ` Peter Zijlstra
2009-03-02 18:27       ` Bartlomiej Zolnierkiewicz
2009-03-02 18:29     ` Linus Torvalds [this message]
2009-03-02 18:48       ` Bartlomiej Zolnierkiewicz
2009-03-02 19:06         ` Linus Torvalds
2009-03-02 19:18           ` Bartlomiej Zolnierkiewicz
2009-03-02 17:56   ` Peter Zijlstra
2009-03-02 18:45   ` Vadim Lobanov
2009-03-02 18:45     ` Vadim Lobanov
2009-03-02 18:54     ` Linus Torvalds
2009-03-05 15:40       ` Mark Lord
2009-03-02 21:17   ` Alan Cox
2009-03-02 21:17     ` Alan Cox
2009-03-06  8:58   ` Benjamin Herrenschmidt
2009-03-06  9:12     ` Peter Zijlstra
2009-03-06  9:59       ` Benjamin Herrenschmidt
2009-03-06 10:05         ` Benjamin Herrenschmidt
2009-03-06 10:24         ` Peter Zijlstra
2009-03-06 17:02           ` Peter Zijlstra
2009-03-06 21:40             ` Benjamin Herrenschmidt
2009-03-02 17:55 ` Andrew Morton
2009-03-02 18:10   ` Peter Zijlstra

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=alpine.LFD.2.00.0903021012080.3111@localhost.localdomain \
    --to=torvalds@linux-foundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=bzolnier@gmail.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox