scst, scst_vdisk: Revert "Simplify the code for querying the vdisk filename"

Revert this commit since it can cause a deadlock as follows:
- scst_assign_dev_handler() obtains scst_mutex and calls
  scst_devt_dev_sysfs_del(). The scst_remove_files() call in the latter
  function waits until all dev->dev_kobj sysfs callbacks have finished.
- vdev_sysfs_filename_show() obtains a dev->dev_kobj reference.
- vdev_sysfs_process_get_filename() tries to lock scst_mutex but deadlocks
  because scst_assign_dev_handler() is holding scst_mutex.

Fixes: 457d6fceed ("scst, scst_vdisk: Simplify the code for querying the vdisk filename")
Reported-by: Lev Vainblat <lev@zadarastorage.com>


git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@9375 d57e44dd-8a1f-0410-8b47-8ef2f437770f

scst/include/scst.h

@@ -2725,6 +2725,14 @@ struct scst_device {
 	/* Set, if a strictly serialized cmd is waiting blocked */
 	unsigned int strictly_serialized_cmd_waiting:1;
 
+	/*
+	 * Set, if this device is being unregistered. Useful to let sysfs
+	 * attributes know when they should exit immediately to prevent
+	 * possible deadlocks with their device unregistration waiting for
+	 * their kobj last put.
+	 */
+	unsigned int dev_unregistering:1;
+
 	/*
 	 * Set if ext blocking is pending. It is just shortcut for
 	 * !list_empty(&dev->ext_blockers_list) to save a cache miss.

scst/src/dev_handlers/scst_vdisk.c

@@ -8319,17 +8319,6 @@ static ssize_t vdisk_sysfs_rotational_show(struct kobject *kobj,
 	return pos;
 }
 
-static bool scst_dev_being_unregistered(struct scst_device *dev)
-{
-	bool res;
-
-	mutex_lock(&scst_mutex);
-	res = list_empty(&dev->dev_list_entry);
-	mutex_unlock(&scst_mutex);
-
-	return res;
-}
-
 static int vdev_sysfs_process_get_filename(struct scst_sysfs_work_item *work)
 {
 	int res = 0;
@@ -8347,7 +8336,7 @@ static int vdev_sysfs_process_get_filename(struct scst_sysfs_work_item *work)
 	 * under scst_vdisk_mutex.
 	 */
 	while (!mutex_trylock(&scst_vdisk_mutex)) {
-		if (scst_dev_being_unregistered(dev)) {
+		if (dev->dev_unregistering) {
 			TRACE_MGMT_DBG("Skipping being unregistered dev %s",
 				dev->virt_name);
 			res = -ENOENT;
@@ -8358,6 +8347,18 @@ static int vdev_sysfs_process_get_filename(struct scst_sysfs_work_item *work)
 			goto out_put;
 		}
 		msleep(100);
+		/*
+		 * We need to reread dev_unregistering from memory, hence
+		 * prevent compiler from putting it in a register. Generally,
+		 * it shouldn't happen, because the compiler isn't allowed to do
+		 * such a transformation if any functions that can cause side
+		 * effects are called between successive accesses, but let's be
+		 * on the safe side. We can't cast dev_unregistering to
+		 * volatile, because it has no effect we need, and can't cast
+		 * it to *(volatile bool*)&, because it isn't possible to get
+		 * address of a bit field.
+		 */
+		barrier();
 	}
 
 	virt_dev = dev->dh_priv;

scst/src/scst_main.c

@@ -1166,6 +1166,8 @@ static void scst_unregister_device(struct scsi_device *scsidp)
 		goto out_unlock;
 	}
 
+	dev->dev_unregistering = 1;
+
 	list_del_init(&dev->dev_list_entry);
 
 	/* For forwarding sources (see also tgt_forward_src). */
@@ -1390,7 +1392,8 @@ out:
 	return res;
 
 out_unreg:
-	list_del_init(&dev->dev_list_entry);
+	dev->dev_unregistering = 1;
+	list_del(&dev->dev_list_entry);
 	scst_assign_dev_handler(dev, &scst_null_devtype);
 	goto out_pr_clear_dev;
 
@@ -1445,6 +1448,8 @@ void scst_unregister_virtual_device(int id,
 		goto out_unlock;
 	}
 
+	dev->dev_unregistering = 1;
+
 	scst_cm_on_dev_unregister(dev);
 
 	list_del_init(&dev->dev_list_entry);