diff mbox

[12/12] target: rename target_free_device to target_destroy_device and cleanup callers

Message ID 1498080791-13565-13-git-send-email-mchristi@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mike Christie June 21, 2017, 9:33 p.m. UTC 
This renames target_free_device to target_destroy_device
to reflect most of the function is tearing down resources
setup during target_configure_device.

I think it also makes goto/error handling nicer in that it follows
other unwinding in the kernel where the goto takes you to a block of
code that releases what was previously setup instead of having one mega
function that can detect multiple states of setup.

We now know that if you have done target_alloc_device but configure failed
or was not run you can just do target_free_device like is done in
target_core_make_subdev and core_dev_setup_virtual_lun0.
If target_configure device has been run successfully then you
run target_destroy_device. We then do not need the DF_CONFIGURED
check in what is now target_destroy_device.

Signed-off-by: Mike Christie <mchristi@redhat.com>
---
 drivers/target/target_core_configfs.c |  2 +-
 drivers/target/target_core_device.c   | 29 ++++++++++++++++-------------
 drivers/target/target_core_internal.h |  1 +
 3 files changed, 18 insertions(+), 14 deletions(-)

Comments

Mike Christie June 21, 2017, 10:11 p.m. UTC | #1 
On 06/21/2017 04:33 PM, Mike Christie wrote:
> This renames target_free_device to target_destroy_device
> to reflect most of the function is tearing down resources
> setup during target_configure_device.
> 
> I think it also makes goto/error handling nicer in that it follows
> other unwinding in the kernel where the goto takes you to a block of
> code that releases what was previously setup instead of having one mega
> function that can detect multiple states of setup.
> 
> We now know that if you have done target_alloc_device but configure failed
> or was not run you can just do target_free_device like is done in
> target_core_make_subdev and core_dev_setup_virtual_lun0.
> If target_configure device has been run successfully then you
> run target_destroy_device. We then do not need the DF_CONFIGURED
> check in what is now target_destroy_device.
> 
> Signed-off-by: Mike Christie <mchristi@redhat.com>


Bart, ignore this patch. It has a bug.

--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index 9b8abd5..15043d34 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -2233,7 +2233,7 @@  static void target_core_dev_release(struct config_item *item)
 	struct se_device *dev =
 		container_of(dev_cg, struct se_device, dev_group);
 
-	target_free_device(dev);
+	target_destroy_device(dev);
 }
 
 /*
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 3743d47..8cc2b07 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -1050,7 +1050,7 @@  int target_configure_device(struct se_device *dev)
 	return ret;
 }
 
-static void target_free_device_call_rcu(struct rcu_head *p)
+static void __target_free_device(struct rcu_head *p)
 {
 	struct se_device *dev = container_of(p, struct se_device, rcu_head);
 
@@ -1059,23 +1059,26 @@  static void target_free_device_call_rcu(struct rcu_head *p)
 
 void target_free_device(struct se_device *dev)
 {
+	call_rcu(&dev->rcu_head, __target_free_device);
+}
+
+void target_destroy_device(struct se_device *dev)
+{
 	struct se_hba *hba = dev->se_hba;
 
 	WARN_ON(!list_empty(&dev->dev_sep_list));
 
-	if (dev->dev_flags & DF_CONFIGURED) {
-		destroy_workqueue(dev->tmr_wq);
+	destroy_workqueue(dev->tmr_wq);
 
-		dev->transport->destroy_device(dev);
+	dev->transport->destroy_device(dev);
 
-		mutex_lock(&device_mutex);
-		idr_remove(&devices_idr, dev->dev_index);
-		mutex_unlock(&device_mutex);
+	mutex_lock(&device_mutex);
+	idr_remove(&devices_idr, dev->dev_index);
+	mutex_unlock(&device_mutex);
 
-		spin_lock(&hba->device_lock);
-		hba->dev_count--;
-		spin_unlock(&hba->device_lock);
-	}
+	spin_lock(&hba->device_lock);
+	hba->dev_count--;
+	spin_unlock(&hba->device_lock);
 
 	core_alua_free_lu_gp_mem(dev);
 	core_alua_set_lba_map(dev, NULL, 0, 0);
@@ -1085,7 +1088,7 @@  void target_free_device(struct se_device *dev)
 	if (dev->transport->free_prot)
 		dev->transport->free_prot(dev);
 
-	call_rcu(&dev->rcu_head, target_free_device_call_rcu);
+	target_free_device(dev);
 }
 
 int core_dev_setup_virtual_lun0(void)
@@ -1131,7 +1134,7 @@  void core_dev_release_virtual_lun0(void)
 		return;
 
 	if (g_lun0_dev)
-		target_free_device(g_lun0_dev);
+		target_destroy_device(g_lun0_dev);
 	core_delete_hba(hba);
 }
 
diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h
index f30e8ac..812527f 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -84,6 +84,7 @@  void	core_dev_free_initiator_node_lun_acl(struct se_portal_group *,
 struct se_device *target_alloc_device(struct se_hba *hba, const char *name);
 int	target_configure_device(struct se_device *dev);
 void	target_free_device(struct se_device *);
+void	target_destroy_device(struct se_device *);
 int	target_for_each_device(int (*fn)(struct se_device *dev, void *data),
 			       void *data);