Re: [PATCH 2/3] Stop accepting SCSI requests before removing a device

[Bart Van Assche <bvanassche@acm.org>]

On 05/04/12 20:30, Mike Christie wrote:
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index 42c35ff..11dc3ff 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -955,24 +955,30 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
>  void __scsi_remove_device(struct scsi_device *sdev)
>  {
>  	struct device *dev = &sdev->sdev_gendev;
> +	struct request_queue *q = sdev->request_queue;
>  
>  	if (sdev->is_visible) {
>  		if (scsi_device_set_state(sdev, SDEV_CANCEL) != 0)
>  			return;
>  
> -		bsg_unregister_queue(sdev->request_queue);
> +		bsg_unregister_queue(q);
>  		device_unregister(&sdev->sdev_dev);
>  		transport_remove_device(dev);
>  		device_del(dev);
>  	} else
>  		put_device(&sdev->sdev_dev);
> +	/*
> +	 * Stop accepting new requests before tearing down the
> +	 * queue. Note: the actual queue deallocation happens in
> +	 * scsi_device_dev_release_usercontext().
> +	 */
> +	blk_cleanup_queue(q);
> +
>  	scsi_device_set_state(sdev, SDEV_DEL);
>  	if (sdev->host->hostt->slave_destroy)
>  		sdev->host->hostt->slave_destroy(sdev);
>  	transport_destroy_device(dev);
>  
> -	/* Freeing the queue signals to block that we're done */
> -	blk_cleanup_queue(sdev->request_queue);
>  	put_device(dev);
>  }

Now that we're looking at potential device removal races: since the host
lock push down scsi_dispatch_cmd() is invoked while a reference on the
device is hold but without holding the host lock or the device queue lock.
Shouldn't we make sure that invoking the SCSI device tear down code only
occurs once it is sure that hostt->queuecommand won't be invoked anymore ?

Bart.