NACK this patch.
Reason for NACK : Upstream code in the sdev state change
routine validates the state change and returns success
when old state and new state are same and this result in
unblocking the request queue correctly in
scsi_internal_device_unblock function, which is not what is
described here. This bug was found in an existing distro.
-bino
-----Original Message-----
From: Sebastian, Bino
Sent: Tuesday, November 28, 2006 4:20 PM
To: 'linux-scsi@xxxxxxxxxxxxxxx'
Subject: [PATCH] Fix race condition between scsi scan and sdev
block/unblock
James,
Our testing has encountered an error between sdev initialization/
scanning and the sdev block/unblock behavior. What we have seen is that
new target detection will kick off a scan, and that an sdev will be in
the creation process with the state SDEV_CREATED. At this point a link
event occurs, which blocks the sdev, changes its state to SDEV_BLOCK,
and stops its request queue. However, the creation thread is still
executing, and decides to transition the sdev state to SDEV_RUNNING.
Note that the request queue is still blocked. The sdev then gets unblocked,
attempting to change the state to SDEV_RUNNING, which fails as it is already
SDEV_RUNNING, which causes the unblock routine to bypass the call to
blk_start_queue().
This patch modifies the creation path so that it only changes to SDEV_RUNNING
if the state is SDEV_CREATED. This allows the block/unblock to work
appropriately. It does have a side effect that unblock could early-transition
the sdev to SDEV_RUNNING.
-- bino
Signed-off-by: Bino Sebastian <bino.sebastian@xxxxxxxxxx>
diff -upNr a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
--- a/drivers/scsi/scsi_scan.c 2006-11-20 16:40:21.977901000 -0500
+++ b/drivers/scsi/scsi_scan.c 2006-11-21 09:08:41.232680000 -0500
@@ -762,7 +762,8 @@ static int scsi_add_lun(struct scsi_devi
/* set the device running here so that slave configure
* may do I/O */
- scsi_device_set_state(sdev, SDEV_RUNNING);
+ if (sdev->sdev_state == SDEV_CREATED)
+ scsi_device_set_state(sdev, SDEV_RUNNING);
if (*bflags & BLIST_MS_192_BYTES_FOR_3F)
sdev->use_192_bytes_for_3f = 1;
diff -upNr a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
--- a/drivers/scsi/scsi_sysfs.c 2006-11-20 16:40:21.988903000 -0500
+++ b/drivers/scsi/scsi_sysfs.c 2006-11-21 09:08:41.245680000 -0500
@@ -676,7 +676,8 @@ int scsi_sysfs_add_sdev(struct scsi_devi
{
int error, i;
- if ((error = scsi_device_set_state(sdev, SDEV_RUNNING)) != 0)
+ if ((sdev->sdev_state == SDEV_CREATED) &&
+ ((error = scsi_device_set_state(sdev, SDEV_RUNNING)) != 0))
return error;
error = device_add(&sdev->sdev_gendev);
-
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
