fix oops when removing scsi_host while IO is outstanding (and when using scsi_target patches)

PATCH In review Mike Christie 2 participants 3 messages Oct 9, 2004 – Oct 10, 2004 SCSI

Share & tools

Mike Christie

author

2004-10-09 15:28 UTC #0 lore raw

PATCH 1/1 fix oops when removing scsi_host while IO is outstanding (and when using scsi_target patches)

When trying to remove a host while IO is running I will get the folling warning, and after these messages my log fills up with various oops.

Oct  9 04:54:44 mina kernel:  target0:0:0: Illegal state transition <NULL>->cancel
Oct  9 04:54:44 mina kernel: Badness in scsi_device_set_state at drivers/scsi/scsi_lib.c:1713
Oct  9 04:54:44 mina kernel:  [<c010585e>] dump_stack+0x1e/0x30
Oct  9 04:54:44 mina kernel:  [<f8831316>] scsi_device_set_state+0xc6/0x130 [scsi_mod]
Oct  9 04:54:44 mina kernel:  [<f88290fc>] scsi_device_cancel+0x3c/0x263 [scsi_mod]
Oct  9 04:54:44 mina kernel:  [<c02c280d>] device_for_each_child+0x4d/0x80
Oct  9 04:54:44 mina kernel:  [<f88293c2>] scsi_host_cancel+0x32/0xc0 [scsi_mod]
Oct  9 04:54:44 mina kernel:  [<f8829472>] scsi_remove_host+0x22/0x70 [scsi_mod]

Note the target dev name and NULL state.

Basically, scsi_host_cancel is doing a device_for_each_child thinking it's children are scsi_devices, but in the scsi-target-2.6 tree they are now targets. The attached patch fixes this by just adding the extra device_for_each_child loop to get to the scsi device. It was built against the current scsi-target-2.6 tree.

Mike

drivers/scsi/hosts.c +7 -1

@@ -55,6 +55,12 @@
 	return scsi_device_cancel(to_scsi_device(dev), *(int *)data);
 }
 
+static int scsi_target_cancel_cb(struct device *dev, void *data)
+{
+	device_for_each_child(dev, data, scsi_device_cancel_cb);
+	return 0;
+}
+
 /**
  * scsi_host_cancel - cancel outstanding IO to this host
  * @shost:	pointer to struct Scsi_Host

@@ -64,7 +70,7 @@
 {
 	set_bit(SHOST_CANCEL, &shost->shost_state);
 	device_for_each_child(&shost->shost_gendev, &recovery,
-			      scsi_device_cancel_cb);
+			      scsi_target_cancel_cb);
 	wait_event(shost->host_wait, (!test_bit(SHOST_RECOVERY,
 						&shost->shost_state)));
 }

Christoph Hellwig

2004-10-10 10:14 UTC #1 lore raw

Quoted 19 lines · Mike Christie

When trying to remove a host while IO is running I will get the folling warning,
and after these messages my log fills up with various oops.

Oct  9 04:54:44 mina kernel:  target0:0:0: Illegal state transition 
<NULL>->cancel
Oct  9 04:54:44 mina kernel: Badness in scsi_device_set_state at 
drivers/scsi/scsi_lib.c:1713
Oct  9 04:54:44 mina kernel:  [<c010585e>] dump_stack+0x1e/0x30
Oct  9 04:54:44 mina kernel:  [<f8831316>] scsi_device_set_state+0xc6/0x130 
[scsi_mod]
Oct  9 04:54:44 mina kernel:  [<f88290fc>] scsi_device_cancel+0x3c/0x263 
[scsi_mod]
Oct  9 04:54:44 mina kernel:  [<c02c280d>] device_for_each_child+0x4d/0x80
Oct  9 04:54:44 mina kernel:  [<f88293c2>] scsi_host_cancel+0x32/0xc0 
[scsi_mod]
Oct  9 04:54:44 mina kernel:  [<f8829472>] scsi_remove_host+0x22/0x70

[scsi_mod]

Quoted 7 lines

Note the target dev name and NULL state.
Basically, scsi_host_cancel is doing a device_for_each_child thinking it's children are scsi_devices, but in the scsi-target-2.6 tree they are now targets. The attached patch fixes this by just adding the extra device_for_each_child loop to get to the scsi device. It was built against the current scsi-target-2.6 tree.

What about using shost_for_each_device instead, so you don't have to care about the extact implementation details?

Mike Christie

author

2004-10-10 18:56 UTC #2 lore raw

Quoted 23 lines · Christoph Hellwig

Quoted 12 lines · Mike Christie
When trying to remove a host while IO is running I will get the folling warning, and after these messages my log fills up with various oops.
Oct  9 04:54:44 mina kernel:  target0:0:0: Illegal state transition <NULL>->cancel
Oct  9 04:54:44 mina kernel: Badness in scsi_device_set_state at drivers/scsi/scsi_lib.c:1713
Oct  9 04:54:44 mina kernel:  [<c010585e>] dump_stack+0x1e/0x30
Oct  9 04:54:44 mina kernel:  [<f8831316>] scsi_device_set_state+0xc6/0x130 [scsi_mod]
Oct  9 04:54:44 mina kernel:  [<f88290fc>] scsi_device_cancel+0x3c/0x263 [scsi_mod]
Oct  9 04:54:44 mina kernel:  [<c02c280d>] device_for_each_child+0x4d/0x80
Oct  9 04:54:44 mina kernel:  [<f88293c2>] scsi_host_cancel+0x32/0xc0 [scsi_mod]
Oct  9 04:54:44 mina kernel:  [<f8829472>] scsi_remove_host+0x22/0x70 [scsi_mod]
Quoted 7 lines
Note the target dev name and NULL state.
Basically, scsi_host_cancel is doing a device_for_each_child thinking it's children are scsi_devices, but in the scsi-target-2.6 tree they are now targets. The attached patch fixes this by just adding the extra device_for_each_child loop to get to the scsi device. It was built against the current scsi-target-2.6 tree.

Quoted 3 lines

What about using shost_for_each_device instead, so you don't have to care about the extact implementation details?

Ok, actually did the previous code even do anything? scsi_remove_host calls scsi_forget_host which calls scsi_remove_device which does a device_del which in turn does a list_del_init(&dev->node). Then when we used the driver model iterators in scsi_host_cancel it did a list_for_each_entry(child, &dev->children, node) but in scsi_forget_host we had just removed all the devices. I guess a fix for both scsi-misc-2.6 is needed too. I will write something up.