[PATCH] Do not register the IDE interrupt handler if host->get_lock is set.

[PATCH] Do not register the IDE interrupt handler if host->get_lock is set.

[Michael Schmitz]

The m68k Atari Falcon IDE driver shares the IDE interrupt with DMA completion,
SCSI and floppy controller. The DMA controller does not provide a way to check
whether it had generated an interrupt, so the shared interrupt has to be
reserved for exclusive use by the respective driver.
To this end, host->get_lock was added to allow both locking the interrupt
and passing the relevant interrupt handler to the machine specific interrupt
code. This feature is used exclusively by the Falcon IDE driver.
Unconditionally registering the IDE interrupt handler in init_irq() when
host->get_lock is set does result in two calls being dispatched to the IDE
handler on each IDE interrupt, and one spurious call on each SCSI, DMA or 
floppy interrupt.

The following patch changes init_irq() to request the IDE interrupt only if
host->get_lock is not set.

Regards,

	Michael Schmitz

[PATCH] m68k/atari - ide: do not register interrupt if host->get_lock is set

[Michael Schmitz]

On m68k, host->get_lock is used to both lock and register the interrupt
that the IDE host shares with other device drivers. Registering the
IDE interrupt handler in ide-probe.c results in duplicating the
interrupt registered (once via host->get lock, and also via init_irq()),
and may result in IDE accepting interrupts even when another driver has
locked the interrupt hardware. This opens the whole locking scheme up
to races.

host->get_lock is set on m68k only, so other drivers' behaviour is not
changed.

Signed-off-by: Michael Schmitz <schmitz@debian.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: David S. Miller <davem@davemloft.net>
Cc: linux-ide@vger.kernel.org
---
 drivers/ide/ide-probe.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/ide/ide-probe.c b/drivers/ide/ide-probe.c
index 2a744a9..a3d3b17 100644
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -853,8 +853,9 @@ static int init_irq (ide_hwif_t *hwif)
 	if (irq_handler == NULL)
 		irq_handler = ide_intr;
 
-	if (request_irq(hwif->irq, irq_handler, sa, hwif->name, hwif))
-		goto out_up;
+	if (!host->get_lock)
+		if (request_irq(hwif->irq, irq_handler, sa, hwif->name, hwif))
+			goto out_up;
 
 #if !defined(__mc68000__)
 	printk(KERN_INFO "%s at 0x%03lx-0x%03lx,0x%03lx on irq %d", hwif->name,
@@ -1533,7 +1534,8 @@ static void ide_unregister(ide_hwif_t *hwif)
 
 	ide_proc_unregister_port(hwif);
 
-	free_irq(hwif->irq, hwif);
+	if (!hwif->host->get_lock)
+		free_irq(hwif->irq, hwif);
 
 	device_unregister(hwif->portdev);
 	device_unregister(&hwif->gendev);
-- 
1.7.0.4

Re: [PATCH] m68k/atari - ide: do not register interrupt if host->get_lock is set

[David Miller]

From: Michael Schmitz <schmitzmic@gmail.com>
Date: Sat,  1 Feb 2014 13:48:13 +1300

> On m68k, host->get_lock is used to both lock and register the interrupt
> that the IDE host shares with other device drivers. Registering the
> IDE interrupt handler in ide-probe.c results in duplicating the
> interrupt registered (once via host->get lock, and also via init_irq()),
> and may result in IDE accepting interrupts even when another driver has
> locked the interrupt hardware. This opens the whole locking scheme up
> to races.
> 
> host->get_lock is set on m68k only, so other drivers' behaviour is not
> changed.
> 
> Signed-off-by: Michael Schmitz <schmitz@debian.org>

It's a bit kludgy, but minimal and correct.

Applied, thank you.

Re: [PATCH] m68k/atari - ide: do not register interrupt if host->get_lock is set

[Michael Schmitz]

Thanks Dave,

>> On m68k, host->get_lock is used to both lock and register the 
>> interrupt
>> that the IDE host shares with other device drivers. Registering the
>> IDE interrupt handler in ide-probe.c results in duplicating the
>> interrupt registered (once via host->get lock, and also via 
>> init_irq()),
>> and may result in IDE accepting interrupts even when another driver 
>> has
>> locked the interrupt hardware. This opens the whole locking scheme up
>> to races.
>>
>> host->get_lock is set on m68k only, so other drivers' behaviour is not
>> changed.
>>
>> Signed-off-by: Michael Schmitz <schmitz@debian.org>
>
> It's a bit kludgy, but minimal and correct.

Would you have preferred to use a host flag instead?

> Applied, thank you.

Thanks again,

	Michael

Re: [PATCH] m68k/atari - ide: do not register interrupt if host->get_lock is set

[David Miller]

From: Michael Schmitz <schmitzmic@gmail.com>
Date: Thu, 6 Mar 2014 19:47:06 +1300

> Thanks Dave,
> 
>>> On m68k, host->get_lock is used to both lock and register the
>>> interrupt
>>> that the IDE host shares with other device drivers. Registering the
>>> IDE interrupt handler in ide-probe.c results in duplicating the
>>> interrupt registered (once via host->get lock, and also via
>>> init_irq()),
>>> and may result in IDE accepting interrupts even when another driver
>>> has
>>> locked the interrupt hardware. This opens the whole locking scheme up
>>> to races.
>>>
>>> host->get_lock is set on m68k only, so other drivers' behaviour is not
>>> changed.
>>>
>>> Signed-off-by: Michael Schmitz <schmitz@debian.org>
>>
>> It's a bit kludgy, but minimal and correct.
> 
> Would you have preferred to use a host flag instead?

I looked into that, we are out of host flags.  We'd either need to expand
the flags value to 64-bits or add another u32.

That's overkill for this.