On 12/07/12 16:33, Bart Van Assche wrote:
On 12/07/12 16:10, Hannes Reinecke wrote:
On 12/06/2012 04:55 PM, Bart Van Assche wrote:
Currently __scsi_remove_device() skips devices that are visible and
offline. Make sure that these devices get removed by changing their
device state into SDEV_DEL at the start of __scsi_remove_device().
Also, avoid that __scsi_remove_device() gets called a second time
for devices that are in state SDEV_CANCEL when scsi_forget_host()
is invoked.
Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
Cc: James Bottomley <JBottomley@xxxxxxxxxxxxx>
Cc: Mike Christie <michaelc@xxxxxxxxxxx>
Cc: Hannes Reinecke <hare@xxxxxxx>
Cc: Tejun Heo <tj@xxxxxxxxxx>
---
drivers/scsi/scsi_scan.c | 2 +-
drivers/scsi/scsi_sysfs.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 3e58b22..0612fba 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1889,7 +1889,7 @@ void scsi_forget_host(struct Scsi_Host *shost)
restart:
spin_lock_irqsave(shost->host_lock, flags);
list_for_each_entry(sdev, &shost->__devices, siblings) {
- if (sdev->sdev_state == SDEV_DEL)
+ if (scsi_device_being_removed(sdev))
continue;
spin_unlock_irqrestore(shost->host_lock, flags);
__scsi_remove_device(sdev);
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 2ff7ba5..4348f12 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -959,8 +959,8 @@ void __scsi_remove_device(struct scsi_device *sdev)
unsigned long flags;
if (sdev->is_visible) {
- if (scsi_device_set_state(sdev, SDEV_CANCEL) != 0)
- return;
+ WARN_ON_ONCE(scsi_device_set_state(sdev, SDEV_CANCEL) != 0 &&
+ scsi_device_set_state(sdev, SDEV_DEL) != 0);
bsg_unregister_queue(sdev->request_queue);
device_unregister(&sdev->sdev_dev);
Hmm. Then we would be getting a warning if the device is already in
SDEV_DEL, wouldn't we?
And what about offlined devices?
We should be safe to remove them, or?
Hello Hannes,
The intent of this patch is that __scsi_remove_device() gets invoked
exactly once per device. This function shouldn't be invoked for devices
already in state SDEV_DEL.
Offlined devices will be transitioned directly from one of the two
offline states into state SDEV_DEL.
The above patch fixes a nasty crash by avoiding that a second
__scsi_remove_device() call queues I/O (sd_shutdown()) after
scsi_remove_host() has already finished.
(replying to my own e-mail)
Please ignore the above comment about sd_shutdown() - that didn't make
sense. What I would like to add to the above is that it's only after I
included the above patch in my tests that the following two call stacks
could no longer be triggered:
BUG: spinlock bad magic on CPU#0, kworker/0:1H/178
lock: 0xffff880177880c28, .magic: ffff8801, .owner: <none>/-1,
.owner_cpu: 2006506176
Pid: 178, comm: kworker/0:1H Tainted: G W O 3.7.0-rc7-debug+ #2
Call Trace:
[<ffffffff814120ef>] spin_dump+0x8c/0x91
[<ffffffff81412115>] spin_bug+0x21/0x26
[<ffffffff81218aef>] do_raw_spin_lock+0x13f/0x150
[<ffffffff81417bb8>] _raw_spin_lock_irqsave+0x78/0xa0
[<ffffffffa0766c6c>] srp_queuecommand+0x3c/0xc80 [ib_srp]
[<ffffffffa0002f18>] scsi_dispatch_cmd+0x148/0x310 [scsi_mod]
[<ffffffffa000a390>] scsi_request_fn+0x320/0x520 [scsi_mod]
[<ffffffff811ec427>] __blk_run_queue+0x37/0x50
[<ffffffff811ec539>] blk_delay_work+0x29/0x40
[<ffffffff81059283>] process_one_work+0x1c3/0x5c0
[<ffffffff8105b1be>] worker_thread+0x15e/0x440
[<ffffffff8106137b>] kthread+0xdb/0xe0
[<ffffffff81420d5c>] ret_from_fork+0x7c/0xb0
------------[ cut here ]------------
BUG: spinlock bad magic on CPU#1, udevd/1518
lock: 0xffff8801a2384c28, .magic: ffff8801, .owner: <none>/-1,
.owner_cpu: -1519491200
Pid: 1518, comm: udevd Not tainted 3.7.0-rc8-debug+ #2
Call Trace:
[<ffffffff81411a9d>] spin_dump+0x8c/0x91
[<ffffffff81411ac3>] spin_bug+0x21/0x26
[<ffffffff812184ff>] do_raw_spin_lock+0x13f/0x150
[<ffffffff81417568>] _raw_spin_lock_irqsave+0x78/0xa0
[<ffffffffa04a0d1c>] srp_queuecommand+0x3c/0xc80 [ib_srp]
[<ffffffffa0002f18>] scsi_dispatch_cmd+0x148/0x310 [scsi_mod]
[<ffffffffa000a6cc>] scsi_request_fn+0x46c/0x570 [scsi_mod]
[<ffffffff811ebe26>] __blk_run_queue+0x46/0x60
[<ffffffff811ebe7e>] queue_unplugged+0x3e/0xd0
[<ffffffff811ee9c3>] blk_flush_plug_list+0x1c3/0x240
[<ffffffff811eea58>] blk_finish_plug+0x18/0x50
[<ffffffff8110511c>] __do_page_cache_readahead+0x24c/0x2e0
[<ffffffff811052e9>] force_page_cache_readahead+0x79/0xb0
[<ffffffff8110573b>] page_cache_sync_readahead+0x4b/0x50
[<ffffffff810fad30>] generic_file_aio_read+0x590/0x710
[<ffffffff8114b127>] do_sync_read+0xa7/0xe0
[<ffffffff8114b878>] vfs_read+0xa8/0x170
[<ffffffff8114b995>] sys_read+0x55/0xa0
[<ffffffff81420782>] system_call_fastpath+0x16/0x1b
------------[ cut here ]------------
Bart.
--
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
