[PATCH] ibmvscsi: implement vio driver shutdown call back to quiesce ibmvscsi

[PATCH] ibmvscsi: implement vio driver shutdown call back to quiesce ibmvscsi

[Tyrel Datwyler]

Currently during kexec the vio driver implementation calls a drivers
remove function as a big hammer when shutdown isn't implemented. This
results in filesystem errors in the log as a result of the block device
being removed while the filesystem is still mounted. Rectify this noise
by quiescing the ibmvscsi driver and stopping short of tearing down the
full host.

Move quiesce calls to their own helper function. Add a driver shutdown
call back and implement with call to ibmvscsi_quiesce such that any
outstanding commands are purged and failed back to the midlayer with
DID_REQUEUE. Update ibmvscsi_remove to also use the new ibmvscsi_quiesce
and failback outstanding commands with DID_ERROR prior to full transport
teardown.

Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
 drivers/scsi/ibmvscsi/ibmvscsi.c | 32 +++++++++++++++++++++++---------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index 4ecd3db08875..bc8da31fdfbd 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -2341,26 +2341,39 @@ static int ibmvscsi_probe(struct vio_dev *vdev, const struct vio_device_id *id)
 	return -1;
 }
 
-static void ibmvscsi_remove(struct vio_dev *vdev)
+static void ibmvscsi_quiesce(struct ibmvscsi_host_data *hostdata, int scsi_error_code)
 {
-	struct ibmvscsi_host_data *hostdata = dev_get_drvdata(&vdev->dev);
-
-	srp_remove_host(hostdata->host);
-	scsi_remove_host(hostdata->host);
-
-	purge_requests(hostdata, DID_ERROR);
-	release_event_pool(&hostdata->pool, hostdata);
+	scsi_block_requests(hostdata->host);
+	kthread_stop(hostdata->work_thread);
 
 	ibmvscsi_release_crq_queue(&hostdata->queue, hostdata,
 					max_events);
 
-	kthread_stop(hostdata->work_thread);
+	purge_requests(hostdata, scsi_error_code);
+
+	release_event_pool(&hostdata->pool, hostdata);
 	unmap_persist_bufs(hostdata);
+}
+
+static void ibmvscsi_shutdown(struct vio_dev *vdev)
+{
+	struct ibmvscsi_host_data *hostdata = dev_get_drvdata(&vdev->dev);
+
+	ibmvscsi_quiesce(hostdata, DID_REQUEUE);
+}
+
+static void ibmvscsi_remove(struct vio_dev *vdev)
+{
+	struct ibmvscsi_host_data *hostdata = dev_get_drvdata(&vdev->dev);
+
+	ibmvscsi_quiesce(hostdata, DID_ERROR);
 
 	spin_lock(&ibmvscsi_driver_lock);
 	list_del(&hostdata->host_list);
 	spin_unlock(&ibmvscsi_driver_lock);
 
+	srp_remove_host(hostdata->host);
+	scsi_remove_host(hostdata->host);
 	scsi_host_put(hostdata->host);
 }
 
@@ -2398,6 +2411,7 @@ static struct vio_driver ibmvscsi_driver = {
 	.id_table = ibmvscsi_device_table,
 	.probe = ibmvscsi_probe,
 	.remove = ibmvscsi_remove,
+	.shutdown = ibmvscsi_shutdown,
 	.get_desired_dma = ibmvscsi_get_desired_dma,
 	.name = "ibmvscsi",
 	.pm = &ibmvscsi_pm_ops,
-- 
2.55.0

Re: [PATCH] ibmvscsi: implement vio driver shutdown call back to quiesce ibmvscsi

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] scsi: ibmvscsi: Use-After-Free in queuecommand due to premature resource free
- [High] scsi: ibmvscsi: deadlock during device removal due to blocked host
--

commit 83656c60ea18229cd02899fae2cde7d1ac88c182
Author: Tyrel Datwyler <tyreld@linux.ibm.com>

ibmvscsi: implement vio driver shutdown call back to quiesce ibmvscsi

This patch implements a driver shutdown callback to quiesce the ibmvscsi
driver and avoids tearing down the full host. It moves quiesce steps into
a new helper function, which is also used by the device remove path.

> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
> index 609bda730b3a8..77f4eac828788 100644
> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c
> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
> @@ -2341,26 +2341,39 @@ static int ibmvscsi_probe(struct vio_dev *vdev, const struct vio_device_id *id)
>  	return -1;
>  }
>  
> -static void ibmvscsi_remove(struct vio_dev *vdev)
> +static void ibmvscsi_quiesce(struct ibmvscsi_host_data *hostdata, int scsi_error_code)
>  {
> -	struct ibmvscsi_host_data *hostdata = dev_get_drvdata(&vdev->dev);
> -
> -	srp_remove_host(hostdata->host);
> -	scsi_remove_host(hostdata->host);
> -
> -	purge_requests(hostdata, DID_ERROR);
> -	release_event_pool(&hostdata->pool, hostdata);
> +	scsi_block_requests(hostdata->host);
> +	kthread_stop(hostdata->work_thread);
>  
>  	ibmvscsi_release_crq_queue(&hostdata->queue, hostdata,
>  					max_events);
>  
> -	kthread_stop(hostdata->work_thread);
> +	purge_requests(hostdata, scsi_error_code);
> +
> +	release_event_pool(&hostdata->pool, hostdata);

[Severity: Critical]
Will this lead to a use-after-free when handling concurrent SCSI commands?

If a concurrent scsi_queue_rq() thread passes the budget check before
host_self_blocked is set by scsi_block_requests(), it will block on
host_lock.

Once purge_requests() drops the host_lock, the preempted thread will
acquire it and enter ibmvscsi_queuecommand_lck(). Because the event pool
memory is freed by release_event_pool() right after purge_requests(),
get_event_struct() would directly access the freed pool->events array when
the preempted thread finally runs.

>  	unmap_persist_bufs(hostdata);
> +}
> +
> +static void ibmvscsi_shutdown(struct vio_dev *vdev)
> +{
> +	struct ibmvscsi_host_data *hostdata = dev_get_drvdata(&vdev->dev);
> +
> +	ibmvscsi_quiesce(hostdata, DID_REQUEUE);
> +}

[Severity: Critical]
Is it safe to omit scsi_remove_host() during shutdown?

During shutdown, the host is never removed here, leaving the SCSI midlayer
completely active. Could this cause crashes if I/O timeouts or commands
trigger after the driver resources have been quiesced and freed?

> +
> +static void ibmvscsi_remove(struct vio_dev *vdev)
> +{
> +	struct ibmvscsi_host_data *hostdata = dev_get_drvdata(&vdev->dev);
> +
> +	ibmvscsi_quiesce(hostdata, DID_ERROR);
>  
>  	spin_lock(&ibmvscsi_driver_lock);
>  	list_del(&hostdata->host_list);
>  	spin_unlock(&ibmvscsi_driver_lock);
>  
> +	srp_remove_host(hostdata->host);
> +	scsi_remove_host(hostdata->host);

[Severity: High]
Does this sequence cause a deadlock during device unbind or module unload?

Since ibmvscsi_quiesce() calls scsi_block_requests(), the host is put into a
blocked state before the child devices are removed.

When scsi_remove_host() synchronously unregisters child devices,
unregistering a SCSI disk calls sd_sync_cache() to flush the write cache.
This submits a bio and waits for completion via blk_execute_rq().

Because the host is blocked, the block layer budget check fails and returns
BLK_STS_RESOURCE, causing the block layer to continuously requeue the flush
request. Will the cache flush request wait indefinitely and deadlock the
rmmod or unbind process?

>  	scsi_host_put(hostdata->host);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916000626.1981569-1-tyreld@linux.ibm.com?part=1