Re: [PATCH] m68k/atari - ide: do not register interrupt if host->get_lock is set
From: Michael Schmitz 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 >> >> 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. -- To unsubscribe from this list: send the line "unsubscribe linux-m68k" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] m68k/atari - ide: do not register interrupt if host->get_lock is set
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 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 -- To unsubscribe from this list: send the line "unsubscribe linux-m68k" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] m68k/atari - ide: do not register interrupt if host->get_lock is set
From: Michael Schmitz 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 It's a bit kludgy, but minimal and correct. Applied, thank you. -- To unsubscribe from this list: send the line "unsubscribe linux-m68k" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] m68k/atari - ide: do not register interrupt if host->get_lock is set
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 Cc: Geert Uytterhoeven Cc: David S. Miller Cc: linux-...@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 -- To unsubscribe from this list: send the line "unsubscribe linux-m68k" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] m68k/atari - ide: do not register interrupt if host->get_lock is set
Geert, On Fri, Jan 31, 2014 at 4:03 AM, Geert Uytterhoeven wrote: > On Tue, Jan 28, 2014 at 9:07 AM, Michael Schmitz wrote: >> --- 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; > > Don't you need a similar check for free_irq()? Absolutely. Thanks for spotting this. I'll send the fixed version to linux-ide. Cheers, Michael -- To unsubscribe from this list: send the line "unsubscribe linux-m68k" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] m68k/atari - ide: do not register interrupt if host->get_lock is set
On Tue, Jan 28, 2014 at 9:07 AM, Michael Schmitz wrote:
> --- 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;
Don't you need a similar check for free_irq()?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] m68k/atari - ide: do not register interrupt if host->get_lock is set
On m68k, host->get_lock may be 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, and may result in IDE accepting interrupts even when another driver has locked the interrupt hardware. Signed-off-by: Michael Schmitz --- drivers/ide/ide-probe.c |5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/ide/ide-probe.c b/drivers/ide/ide-probe.c index 2a744a9..ae691da 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, -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-m68k" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
