Hi,

On Monday 22 October 2007, Matti Linnanvuori wrote:
> From: Matti Linnanvuori <[EMAIL PROTECTED]>
> 
> Correct IDEPCI_SHARE_IRQ description to keeping interrupts enabled
> when calling the PCI IDE action handler. CONFIG_IDEPCI_SHARE_IRQ does not
> affect the setting of IRQF_SHARED for PCI chipsets in function init_irq, 
> file drivers/ide/ide-probe.c but only the setting of IRQF_DISABLED.
> Add IDEPCI_SHARE_IRQ and IDEDISK_MULTI_MODE help text from hdparm
> manual page.
> 
> Signed-off-by: Matti Linnanvuori <[EMAIL PROTECTED]>
> ---
> 
> Changing mention of multi-mode to affecting only some systems. Improving text.
> 
> --- a/drivers/ide/Kconfig     2007-10-21 08:53:11.826349500 +0300
> +++ b/drivers/ide/Kconfig     2007-10-22 18:45:58.715425500 +0300
> @@ -154,7 +154,19 @@ config BLK_DEV_IDEDISK
>  config IDEDISK_MULTI_MODE
>       bool "Use multi-mode by default"

Would be nice to update this to something more meaningful while at it,
i.e. "Use multiple-sector PIO mode by default"

>       help
> -       If you get this error, try to say Y here:
> +       Multiple sector mode is a feature of most modern

This still doesn't emphasize the fact the this settings affects _only_
PIO transfers.  If DMA is used this setting should have no effect.

> +       IDE drives, permitting the transfer of multiple sectors per I/O
> +       interrupt, rather than the usual one sector per interrupt.
> +       When this feature is enabled, it can reduce operating
> +       system overhead for disk I/O by 30 - 50%. On some systems, it
> +       also provides increased data throughput of anywhere from 5%
> +       to 50%. Some drives, however (most notably the WD Caviar

The hdparm text help is years old, I'm don't think that the comment
about WD Caviar disks holds for the later models.

> +       series), seem to run slower with multiple mode enabled. Some
> +       drives claim to support multiple mode, but lose data at some

drives -> very old drives

> +       settings. Under rare circumstances, such failures can result
> +       in massive filesystem corruption.
> +
> +       If you get the following error, try to say Y here:
>  
>         hda: set_multmode: status=0x51 { DriveReady SeekComplete Error }
>         hda: set_multmode: error=0x04 { DriveStatusError }
> @@ -367,11 +379,16 @@ config BLK_DEV_IDEPCI
>       bool
>  
>  config IDEPCI_SHARE_IRQ
> -     bool "Sharing PCI IDE interrupts support"
> +     bool "Keeping interrupts enabled when calling the PCI IDE action 
> handler"
>       depends on BLK_DEV_IDEPCI
>       help
> -       Some ATA/IDE chipsets have hardware support which allows for
> -       sharing a single IRQ with other cards. To enable support for
> +       A setting of Y permits the driver to enable other interrupts
> +       during processing of a disk interrupt, which greatly improves
> +       Linux's responsiveness and eliminates "serial port overrun"
> +       and buffer errors. Use this feature with caution: some
> +       drive/controller combinations do not tolerate the increased I/O
> +       latencies possible when this feature is enabled, resulting in massive
> +       filesystem corruption. To enable support for
>         this in the ATA/IDE driver, say Y here.
>  
>         It is safe to say Y to this question, in most cases.

CONFIG_IDEPCI_SHARE_IRQ has no real effect on IRQ unmasking because the first
thing that ide_intr() (IDE IRQ handler) does is calling spin_lock_irqsave()
(which disables local IRQs).

drivers/ide/ide-probe.c:

static int init_irq (ide_hwif_t *hwif)
...
#ifndef CONFIG_IDEPCI_SHARE_IRQ
                        sa |= IRQF_DISABLED;
#endif /* CONFIG_IDEPCI_SHARE_IRQ */
...

kernel/irq/handle.c:

irqreturn_t handle_IRQ_event(unsigned int irq, struct irqaction *action)
...
        if (!(action->flags & IRQF_DISABLED))
                local_irq_enable_in_hardirq();

        do {
                ret = action->handler(irq, action->dev_id);
                if (ret == IRQ_HANDLED)
                        status |= action->flags;
                retval |= ret;
                action = action->next;
        } while (action);
...

drivers/ide/ide-io.c:

irqreturn_t ide_intr (int irq, void *dev_id)
...
        spin_lock_irqsave(&ide_lock, flags);
...
        spin_unlock(&ide_lock);
...
        if (drive->unmask)
                local_irq_enable_in_hardirq();
...

Thanks for raising this question though, we see now that we may remove
IRQF_DISABLE flag completely from IRQ flags for IDE IRQ handler.

It is also a good time to kill IDEPCI_SHARE_IRQ config option
(fixing a bug alongside).

[ patches posted in separate mails ]

Coming back to this patch: please recast it dropping IDEPCI_SHARE_IRQ part.

Thanks,
Bart
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to