Replaced save_flags()/cli() with spin_lock alternatives
Signed-off-by: Amol Lad <[email protected]>
---
Andrew,
Please add this to -mm
---
--- linux-2.6.19-rc4-orig/drivers/scsi/mca_53c9x.c 2006-08-24 02:46:33.000000000 +0530
+++ linux-2.6.19-rc4/drivers/scsi/mca_53c9x.c 2006-11-06 18:03:22.000000000 +0530
@@ -341,9 +341,7 @@ static void dma_init_read(struct NCR_ESP
{
unsigned long flags;
-
- save_flags(flags);
- cli();
+ spin_lock_irqsave(esp->ehost->host_lock, flags);
mca_disable_dma(esp->dma);
mca_set_dma_mode(esp->dma, MCA_DMA_MODE_XFER | MCA_DMA_MODE_16 |
@@ -352,16 +350,14 @@ static void dma_init_read(struct NCR_ESP
mca_set_dma_count(esp->dma, length / 2); /* !!! */
mca_enable_dma(esp->dma);
- restore_flags(flags);
+ spin_unlock_irqrestore(esp->ehost->host_lock, flags);
}
static void dma_init_write(struct NCR_ESP *esp, __u32 addr, int length)
{
unsigned long flags;
-
- save_flags(flags);
- cli();
+ spin_lock_irqsave(esp->ehost->host_lock, flags);
mca_disable_dma(esp->dma);
mca_set_dma_mode(esp->dma, MCA_DMA_MODE_XFER | MCA_DMA_MODE_WRITE |
@@ -370,7 +366,7 @@ static void dma_init_write(struct NCR_ES
mca_set_dma_count(esp->dma, length / 2); /* !!! */
mca_enable_dma(esp->dma);
- restore_flags(flags);
+ spin_unlock_irqrestore(esp->ehost->host_lock, flags);
}
static void dma_ints_off(struct NCR_ESP *esp)
On Mon, 06 Nov 2006 18:12:11 +0530
Amol Lad <[email protected]> wrote:
> Replaced save_flags()/cli() with spin_lock alternatives
>
> Signed-off-by: Amol Lad <[email protected]>
> ---
> Andrew,
> Please add this to -mm
> ---
> --- linux-2.6.19-rc4-orig/drivers/scsi/mca_53c9x.c 2006-08-24 02:46:33.000000000 +0530
> +++ linux-2.6.19-rc4/drivers/scsi/mca_53c9x.c 2006-11-06 18:03:22.000000000 +0530
> @@ -341,9 +341,7 @@ static void dma_init_read(struct NCR_ESP
> {
> unsigned long flags;
>
> -
> - save_flags(flags);
> - cli();
> + spin_lock_irqsave(esp->ehost->host_lock, flags);
>
> mca_disable_dma(esp->dma);
> mca_set_dma_mode(esp->dma, MCA_DMA_MODE_XFER | MCA_DMA_MODE_16 |
> @@ -352,16 +350,14 @@ static void dma_init_read(struct NCR_ESP
> mca_set_dma_count(esp->dma, length / 2); /* !!! */
> mca_enable_dma(esp->dma);
>
> - restore_flags(flags);
> + spin_unlock_irqrestore(esp->ehost->host_lock, flags);
> }
>
> static void dma_init_write(struct NCR_ESP *esp, __u32 addr, int length)
> {
> unsigned long flags;
>
> -
> - save_flags(flags);
> - cli();
> + spin_lock_irqsave(esp->ehost->host_lock, flags);
>
> mca_disable_dma(esp->dma);
> mca_set_dma_mode(esp->dma, MCA_DMA_MODE_XFER | MCA_DMA_MODE_WRITE |
> @@ -370,7 +366,7 @@ static void dma_init_write(struct NCR_ES
> mca_set_dma_count(esp->dma, length / 2); /* !!! */
> mca_enable_dma(esp->dma);
>
> - restore_flags(flags);
> + spin_unlock_irqrestore(esp->ehost->host_lock, flags);
> }
>
> static void dma_ints_off(struct NCR_ESP *esp)
hm. How do we find out if this works?
If it _does_ work then we can now remove the Kconfig BROKEN_ON_SMP dependency
for this driver.
On Mon, Nov 06, 2006 at 06:12:11PM +0530, Amol Lad wrote:
> Replaced save_flags()/cli() with spin_lock alternatives
This patch has very little chance to work as-is because it only replaces
cli with spinlocks, but not the irq handler it's locking against.
On Tue, 2006-11-07 at 12:53 -0800, Andrew Morton wrote:
> hm. How do we find out if this works?
I'm not sure anyone has this type of HW anymore. It was the original
SCSI chip for the old MCA based IBM PC. I can dig through my hardware
bins to see if I have a card, but I don't think so.
> If it _does_ work then we can now remove the Kconfig BROKEN_ON_SMP dependency
> for this driver.
Theoretically, yes ... practically I don't think there was an SMP box
produced with this chip.
James
On Tue, 2006-11-07 at 22:29 +0000, Christoph Hellwig wrote:
> On Mon, Nov 06, 2006 at 06:12:11PM +0530, Amol Lad wrote:
> > Replaced save_flags()/cli() with spin_lock alternatives
>
> This patch has very little chance to work as-is because it only replaces
> cli with spinlocks, but not the irq handler it's locking against.
>
It protects against irq handler also.
drivers/scsi/NCR53C9x.c:
irqreturn_t esp_intr(int irq, void *dev_id)
{
struct NCR_ESP *esp;
unsigned long flags;
int again;
struct Scsi_Host *dev = dev_id;
/* Handle all ESP interrupts showing at this IRQ level. */
spin_lock_irqsave(dev->host_lock, flags);
...
...
}
On Wed, 2006-11-08 at 09:19 +0900, James Bottomley wrote:
> On Tue, 2006-11-07 at 12:53 -0800, Andrew Morton wrote:
>
> > hm. How do we find out if this works?
>
> I'm not sure anyone has this type of HW anymore. It was the original
> SCSI chip for the old MCA based IBM PC. I can dig through my hardware
> bins to see if I have a card, but I don't think so.
The only way I see is the code review.
Most of the drivers using save_flags()/cli() interfaces are not in use
anymore (such as drivers/char/riscom8.c). There are two possibilities:
1. Remove such drivers from kernel
2. Fix them
This would mean we can remove cli()/sti()/save_flags() interfaces from
the kernel so that any new development does not uses them accidently
>
> > If it _does_ work then we can now remove the Kconfig BROKEN_ON_SMP dependency
> > for this driver.
>
> Theoretically, yes ... practically I don't think there was an SMP box
> produced with this chip.
>
> James
>
>
>
Ar Mer, 2006-11-08 am 09:19 +0900, ysgrifennodd James Bottomley:
> On Tue, 2006-11-07 at 12:53 -0800, Andrew Morton wrote:
>
> > hm. How do we find out if this works?
>
> I'm not sure anyone has this type of HW anymore. It was the original
> SCSI chip for the old MCA based IBM PC. I can dig through my hardware
> bins to see if I have a card, but I don't think so.
There was one in the pile of MCA cards I posted you I think.