On 10/07/12 12:47, James Bottomley wrote:
On Mon, 2012-09-24 at 15:14 +0200, Bart Van Assche wrote:
Avoid that the sdev reference count can drop to zero before
the queue is run by scsi_run_queue(). Also avoid that the sdev
reference count can drop to zero in the same function by invoking
__blk_run_queue().
[...] if (scsi_device_set_state(sdev, SDEV_CANCEL) != 0)
@@ -977,6 +978,10 @@ void __scsi_remove_device(struct scsi_device *sdev)
blk_cleanup_queue(sdev->request_queue);
cancel_work_sync(&sdev->requeue_work);
+ spin_lock_irqsave(shost->host_lock, flags);
+ list_del(&sdev->starved_entry);
+ spin_unlock_irqrestore(shost->host_lock, flags);
+
This hunk doesn't make much sense. It seems to be orthogonal to the
problem listed in the changelog and this action is done on last put
anyway.
Removing an sdev from the starved list in __scsi_remove_device() has the
advantage that it is guaranteed that the get_device() call added in
scsi_run_queue() will succeed. A possible alternative is to leave the
starved list removal code in scsi_device_dev_release_usercontext() and
to invoke __blk_run_queue() in scsi_run_queue() only if the get_device()
call in that function succeeded. Does this mean that you prefer the
second option - something like the (untested) code below ?
if (get_device(&sdev->sdev_gendev)) {
spin_unlock(shost->host_lock);
spin_lock(sdev->request_queue->queue_lock);
__blk_run_queue(sdev->request_queue);
spin_unlock(sdev->request_queue->queue_lock);
put_device(&sdev->sdev_gendev);
spin_lock(shost->host_lock);
}
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