On 6/28/22 18:17, Ming Lei wrote:
On Tue, Jun 28, 2022 at 10:56:12AM -0700, Bart Van Assche wrote:
There are two .exit_cmd_priv implementations. Both implementations use the
SCSI host pointer. Make sure that the SCSI host pointer is valid when
.exit_cmd_priv is called by moving the .exit_cmd_priv calls from
scsi_device_dev_release() to scsi_forget_host(). Moving
blk_mq_free_tag_set() from scsi_device_dev_release() to scsi_forget_host()
is safe because scsi_forget_host() drains all the request queues that use
the host tag set. This guarantees that no requests are in flight and also
that no new requests will be allocated from the host tag set.
Not sure scsi_forget_host really drains all queues since it bypasses
sdev which state is SDEV_DEL, so removal for this sdev could be
in-progress, not done yet.
Ah, that's right. How about making scsi_forget_host() wait until all request
activity on the associated queues has stopped, e.g. by replacing the current
scsi_forget_host() implementation by the one below?
/**
* scsi_forget_host() - Remove all SCSI devices from a host.
* @shost: SCSI host to remove devices from.
*
* Removes all SCSI devices that have not yet been removed. For the SCSI devices
* for which removal started before scsi_forget_host(), wait until the
* associated request queue has reached the "dead" state. In that state it is
* guaranteed that no new requests will be allocated and also that no requests
* are in progress anymore.
*/
void scsi_forget_host(struct Scsi_Host *shost)
{
struct scsi_device *sdev;
might_sleep();
restart:
spin_lock_irq(shost->host_lock);
list_for_each_entry(sdev, &shost->__devices, siblings) {
if (sdev->sdev_state == SDEV_DEL &&
blk_queue_dead(sdev->request_queue)) {
continue;
}
if (sdev->sdev_state == SDEV_DEL) {
get_device(&sdev->sdev_gendev);
spin_unlock_irq(shost->host_lock);
while (!blk_queue_dead(sdev->request_queue))
msleep(10);
spin_lock_irq(shost->host_lock);
put_device(&sdev->sdev_gendev);
goto restart;
}
spin_unlock_irq(shost->host_lock);
__scsi_remove_device(sdev);
goto restart;
}
spin_unlock_irq(shost->host_lock);
}
Thanks,
Bart.
