[3/5] vhost-scsi: Remove vhost_scsi_mutex from port link/unlink

Commit Message

Mike Christie Feb. 23, 2023, 12:19 a.m. UTC

We don't need the vhost_scsi_mutex in vhost_scsi_port_link and
vhost_scsi_port_unlink because LIO has a refcount on the se_tpg for us,
so it can't be removed while these functions are called. This removes the
vhost_scsi_mutex from those functions to avoid cases where we are adding
or removing LUNs to vhost-deviceA but are stuck waiting on the
vhost_scsi_mutex because we are running vhost_scsi_clear_endpoint on
vhost-deviceB and it's stuck in vhost_scsi_flush waiting for a flakey
physical device.

Signed-off-by: Mike Christie <[e-mail]>
---
 drivers/vhost/scsi.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

Comments

Mike Christie March 19, 2023, 3:48 p.m. UTC | #1

On 2/22/23 6:19 PM, Mike Christie wrote:
> We don't need the vhost_scsi_mutex in vhost_scsi_port_link and
> vhost_scsi_port_unlink because LIO has a refcount on the se_tpg for us,
> so it can't be removed while these functions are called. This removes the
> vhost_scsi_mutex from those functions to avoid cases where we are adding
> or removing LUNs to vhost-deviceA but are stuck waiting on the
> vhost_scsi_mutex because we are running vhost_scsi_clear_endpoint on
> vhost-deviceB and it's stuck in vhost_scsi_flush waiting for a flakey
> physical device.

I'm going to self Nack these patches. This patch has a bug where we can
leak vhost_scsi_tmf structs. It's ok to drop the vhost_scsi_mutex use but
in vhost_scsi_port_unlink it needs to be replaced with the device->mutex
or we need to flush the tmf/VHOST_SCSI_VQ_CTL queue to make sure outstanding
tmfs are freed.

Patch

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 502d64b53d9c..9e154e568438 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -232,7 +232,7 @@  struct vhost_scsi_ctx {
 	struct iov_iter out_iter;
 };
 
-/* Global spinlock to protect vhost_scsi TPG list for vhost IOCTL access */
+/* Global mutex to protect vhost_scsi TPG list for vhost IOCTL access */
 static DEFINE_MUTEX(vhost_scsi_mutex);
 static LIST_HEAD(vhost_scsi_list);
 
@@ -2038,17 +2038,12 @@  static int vhost_scsi_port_link(struct se_portal_group *se_tpg,
 	INIT_LIST_HEAD(&tmf->queue_entry);
 	vhost_work_init(&tmf->vwork, vhost_scsi_tmf_resp_work);
 
-	mutex_lock(&vhost_scsi_mutex);
-
 	mutex_lock(&tpg->tv_tpg_mutex);
 	tpg->tv_tpg_port_count++;
 	list_add_tail(&tmf->queue_entry, &tpg->tmf_queue);
 	mutex_unlock(&tpg->tv_tpg_mutex);
 
 	vhost_scsi_hotplug(tpg, lun);
-
-	mutex_unlock(&vhost_scsi_mutex);
-
 	return 0;
 }
 
@@ -2059,8 +2054,6 @@  static void vhost_scsi_port_unlink(struct se_portal_group *se_tpg,
 				struct vhost_scsi_tpg, se_tpg);
 	struct vhost_scsi_tmf *tmf;
 
-	mutex_lock(&vhost_scsi_mutex);
-
 	mutex_lock(&tpg->tv_tpg_mutex);
 	tpg->tv_tpg_port_count--;
 	tmf = list_first_entry(&tpg->tmf_queue, struct vhost_scsi_tmf,
@@ -2070,8 +2063,6 @@  static void vhost_scsi_port_unlink(struct se_portal_group *se_tpg,
 	mutex_unlock(&tpg->tv_tpg_mutex);
 
 	vhost_scsi_hotunplug(tpg, lun);
-
-	mutex_unlock(&vhost_scsi_mutex);
 }
 
 static ssize_t vhost_scsi_tpg_attrib_fabric_prot_type_store(