Re: [PATCH] tmscsim: host_lock use in LLD

[James Bottomley <James.Bottomley@steeleye.com>]

On Fri, 2004-06-18 at 16:28, Guennadi Liakhovetski wrote:
> While reviewing tmscsim, I noticed something, I didn't quite like / 
> understand. The driver takes the host_lock (with irqsave) at the entry to 
> the ISR, and releases it at the exit. And inside the ISR there are 
> potentially long busy-waits... Like
> 
>  	int ctr = 6000000; /* only try for about a second */
>  	while( --ctr && !((dstate = DC390_read8 (DMA_Status)) &
>  				DMA_XFER_DONE) && pSRB->SGToBeXferLen );
> 
> The attached patch is attempting to fix those places. Not sure if this is 
> a proper fix though. In my understanding, after looking through the code, 
> the host_lock is used to protect host-specific data and host-registers. 
> The ->queuecommand is already called with it help, so, one just, 
> basically, have to protect other contexts - interrupt, timer,... So, looks 
> mostly right.

This patch is rejecting in this part:

static void __devexit dc390_remove_one(struct pci_dev *dev)
 {
        struct Scsi_Host *scsi_host = pci_get_drvdata(dev);
-       DC390_IFLAGS;
+       unsigned long iflags;
        PACB pACB = (PACB) scsi_host->hostdata;
 
        scsi_remove_host(scsi_host);
 
-       DC390_LOCK_IO(scsi_host);
+       spin_lock_irqsave(scsi_host->host_lock, iflags);
 

As best as I can tell, DC390_IFLAGS has never been part of the tmscsim.c
file ... I think this must be against some modification of the driver
you have in your tree.

James