On 11/01/2010 01:57 PM, Andi Kleen wrote:
@@ -3186,11 +3186,14 @@ int ata_scsi_queuecmd(struct scsi_cmnd *cmd, void (*done
struct ata_device *dev;
struct scsi_device *scsidev = cmd->device;
struct Scsi_Host *shost = scsidev->host;
+ unsigned long irqflags;
int rc = 0;
ap = ata_shost_to_port(shost);
- spin_unlock(shost->host_lock);
+ spin_lock_irqsave(shost->host_lock, irqflags);
+ scsi_cmd_get_serial(shost, cmd);
+
spin_lock(ap->lock);
ata_scsi_dump_cdb(ap, cmd);
@@ -3205,6 +3208,7 @@ int ata_scsi_queuecmd(struct scsi_cmnd *cmd, void (*done)(
spin_unlock(ap->lock);
spin_lock(shost->host_lock);
+ spin_unlock_irqrestore(shost->host_lock, irqflags);
return rc;
}
It's a bit disappointing that libata's lock profile in this patch is
quite different than that of current upstream: with your patch, libata
holds the scsi host lock for a considerably longer period of time, while
also holding the ATA port/host spinlock.
IOW, it's doing the exact opposite of what the previous code did
(release the scsi host lock, before acquiring the ATA port/host
spinlock), not at all an equivalent transformation.
The following sequence would seem to better preserve the existing lock
profile, correct?
local_irq_save(flags)
spin_lock(shost->host_lock)
scsi_cmd_get_serial()
spin_unlock(shost->host_lock)
spin_lock(ap->lock)
...
spin_unlock(ap->lock)
local_irq_restore(flags)
Regards,
Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
