scst: Free removed LUNs asynchronously

Since scst_free_tgt_dev() is called as soon as the tgt_dev refcount
drops to zero and since scst_del_tgt_dev() decrements that reference
count, remove all scst_free_tgt_dev() calls that follow a
scst_del_tgt_dev() call. Set nr_deleted_tgt_devs in scst_acg_add_lun()
to -1 to avoid that all scst_del_tgt_dev() calls try to free the
associated acg_dev.


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

scst/include/scst.h

@@ -3104,6 +3104,11 @@ struct scst_tgt_dev {
 	/* List entry in sess->sess_tgt_dev_list */
 	struct list_head sess_tgt_dev_list_entry;
 
+	struct rcu_head rcu;
+	struct work_struct free_work;
+	atomic_t *a;
+	bool dec_acg_refcnt;
+
 	struct scst_tgt_template *tgtt; /* to avoid use-after-free issues */
 	struct scst_device *dev; /* to save extra dereferences */
 	uint64_t lun;		 /* to save extra dereferences */
@@ -3260,6 +3265,13 @@ struct scst_acg_dev {
 	/* sysfs release completion */
 	struct completion *acg_dev_kobj_release_cmpl;
 
+	/*
+	 * Number of deleted tgt_devs associated with this acg_dev. Set if an
+	 * acg_dev is no longer visible and will be freed as soon as all
+	 * associated tgt_dev instances have been freed.
+	 */
+	int nr_deleted_tgt_devs;
+
 	/* Name of the link to the corresponding LUN */
 	char acg_dev_link_name[20];
 };

scst/src/scst_lib.c

@@ -2961,8 +2961,6 @@ retry_add:
 					TRACE_MGMT_DBG("Replacing LUN %lld",
 						(long long)tgt_dev->lun);
 					scst_del_tgt_dev(tgt_dev);
-					synchronize_rcu();
-					scst_free_tgt_dev(tgt_dev);
 					inq_changed_ua_needed = 1;
 					break;
 				}
@@ -3006,8 +3004,6 @@ next:
 				luns_changed = true;
 				something_freed = true;
 				scst_del_tgt_dev(tgt_dev);
-				synchronize_rcu();
-				scst_free_tgt_dev(tgt_dev);
 			}
 		}
 	}
@@ -4560,10 +4556,40 @@ out_free:
 	goto out;
 }
 
+static void scst_tgt_dev_free_workfn(struct work_struct *work)
+{
+	struct scst_tgt_dev *tgt_dev = container_of(work, typeof(*tgt_dev),
+						    free_work);
+	struct scst_acg_dev *acg_dev = tgt_dev->acg_dev;
+	struct scst_device *dev = tgt_dev->dev;
+	bool dec_acg_refcnt = tgt_dev->dec_acg_refcnt;
+	atomic_t *a = tgt_dev->a;
+
+	mutex_lock(&scst_mutex);
+	scst_free_tgt_dev(tgt_dev);
+	if (dec_acg_refcnt) {
+		WARN_ON_ONCE(acg_dev->nr_deleted_tgt_devs < 0);
+		if (--acg_dev->nr_deleted_tgt_devs == 0)
+			scst_free_acg_dev(acg_dev);
+	}
+	mutex_unlock(&scst_mutex);
+
+	percpu_ref_put(&dev->refcnt);
+	scst_put(a);
+}
+
+void scst_free_tgt_dev_rcu(struct rcu_head *rcu)
+{
+	struct scst_tgt_dev *tgt_dev = container_of(rcu, typeof(*tgt_dev), rcu);
+
+	tgt_dev->a = scst_get();
+	percpu_ref_get(&tgt_dev->dev->refcnt);
+	WARN_ON_ONCE(!schedule_work(&tgt_dev->free_work));
+}
+
 /* Delete a LUN without generating a unit attention. */
 static struct scst_acg_dev *__scst_acg_del_lun(struct scst_acg *acg,
 					       uint64_t lun,
-					       struct list_head *tgt_dev_list,
 					       bool *report_luns_changed)
 {
 	struct scst_acg_dev *acg_dev = NULL, *a;
@@ -4572,8 +4598,6 @@ static struct scst_acg_dev *__scst_acg_del_lun(struct scst_acg *acg,
 
 	lockdep_assert_held(&scst_mutex);
 
-	INIT_LIST_HEAD(tgt_dev_list);
-
 	list_for_each_entry(a, &acg->acg_dev_list, acg_dev_list_entry) {
 		if (a->lun == lun) {
 			acg_dev = a;
@@ -4586,17 +4610,18 @@ static struct scst_acg_dev *__scst_acg_del_lun(struct scst_acg *acg,
 	*report_luns_changed = scst_cm_on_del_lun(acg_dev,
 						  *report_luns_changed);
 
+	acg_dev->nr_deleted_tgt_devs = 1;
+
 	list_for_each_entry_safe(tgt_dev, tt, &acg_dev->dev->dev_tgt_dev_list,
 			 dev_tgt_dev_list_entry) {
 		if (tgt_dev->acg_dev == acg_dev) {
 			sess = tgt_dev->sess;
 
 			mutex_lock(&sess->tgt_dev_list_mutex);
+			acg_dev->nr_deleted_tgt_devs++;
+			tgt_dev->dec_acg_refcnt = true;
 			scst_del_tgt_dev(tgt_dev);
 			mutex_unlock(&sess->tgt_dev_list_mutex);
-
-			list_add_tail(&tgt_dev->extra_tgt_dev_list_entry,
-				      tgt_dev_list);
 		}
 	}
 
@@ -4609,37 +4634,17 @@ out:
 	return acg_dev;
 }
 
-static int scst_tgt_devs_cmds(struct list_head *tgt_dev_list)
-{
-	struct scst_tgt_dev *tgt_dev;
-	int res = 0;
-
-	list_for_each_entry(tgt_dev, tgt_dev_list, extra_tgt_dev_list_entry)
-		res += atomic_read(&tgt_dev->tgt_dev_cmd_count);
-
-	return res;
-}
-
-static void scst_wait_for_tgt_devs(struct list_head *tgt_dev_list)
-{
-	while (scst_tgt_devs_cmds(tgt_dev_list) > 0)
-		mdelay(100);
-}
-
 int scst_acg_del_lun(struct scst_acg *acg, uint64_t lun,
 		     bool gen_report_luns_changed)
 {
 	int res = 0;
 	struct scst_acg_dev *acg_dev;
-	struct scst_tgt_dev *tgt_dev, *tt;
-	struct list_head tgt_dev_list;
 
 	TRACE_ENTRY();
 
 	lockdep_assert_held(&scst_mutex);
 
-	acg_dev = __scst_acg_del_lun(acg, lun, &tgt_dev_list,
-				     &gen_report_luns_changed);
+	acg_dev = __scst_acg_del_lun(acg, lun, &gen_report_luns_changed);
 	if (acg_dev == NULL) {
 		PRINT_ERROR("Device is not found in group %s", acg->acg_name);
 		res = -EINVAL;
@@ -4651,16 +4656,11 @@ int scst_acg_del_lun(struct scst_acg *acg, uint64_t lun,
 
 	mutex_unlock(&scst_mutex);
 
-	scst_wait_for_tgt_devs(&tgt_dev_list);
 	synchronize_rcu();
 
 	mutex_lock(&scst_mutex);
-
-	list_for_each_entry_safe(tgt_dev, tt, &tgt_dev_list,
-				 extra_tgt_dev_list_entry) {
-		scst_free_tgt_dev(tgt_dev);
-	}
-	scst_free_acg_dev(acg_dev);
+	if (--acg_dev->nr_deleted_tgt_devs == 0)
+		scst_free_acg_dev(acg_dev);
 
 out:
 	TRACE_EXIT_RES(res);
@@ -4674,13 +4674,12 @@ int scst_acg_repl_lun(struct scst_acg *acg, struct kobject *parent,
 {
 	struct scst_acg_dev *acg_dev;
 	bool del_gen_ua = false;
-	struct scst_tgt_dev *tgt_dev, *tt;
-	struct list_head tgt_dev_list;
+	struct scst_tgt_dev *tgt_dev;
 	int res = -EINVAL;
 
 	lockdep_assert_held(&scst_mutex);
 
-	acg_dev = __scst_acg_del_lun(acg, lun, &tgt_dev_list, &del_gen_ua);
+	acg_dev = __scst_acg_del_lun(acg, lun, &del_gen_ua);
 	if (!acg_dev)
 		flags |= SCST_ADD_LUN_GEN_UA;
 	res = scst_acg_add_lun(acg, parent, dev, lun, flags, NULL);
@@ -4701,15 +4700,10 @@ int scst_acg_repl_lun(struct scst_acg *acg, struct kobject *parent,
 	}
 	mutex_unlock(&scst_mutex);
 
-	scst_wait_for_tgt_devs(&tgt_dev_list);
 	synchronize_rcu();
 
 	mutex_lock(&scst_mutex);
-	list_for_each_entry_safe(tgt_dev, tt, &tgt_dev_list,
-				 extra_tgt_dev_list_entry) {
-		scst_free_tgt_dev(tgt_dev);
-	}
-	if (acg_dev)
+	if (acg_dev && --acg_dev->nr_deleted_tgt_devs == 0)
 		scst_free_acg_dev(acg_dev);
 
 out:
@@ -4820,11 +4814,7 @@ static void scst_del_acg(struct scst_acg *acg)
 	}
 }
 
-/*
- * scst_free_acg - free an ACG
- *
- * The caller must hold scst_mutex and activity must have been suspended.
- */
+/* scst_free_acg - free an ACG */
 static void scst_free_acg(struct scst_acg *acg)
 {
 	struct scst_acg_dev *acg_dev, *acg_dev_tmp;
@@ -4835,10 +4825,14 @@ static void scst_free_acg(struct scst_acg *acg)
 	/* For procfs acg->tgt could be NULL */
 	TRACE_DBG("Freeing acg %s/%s", tgt ? tgt->tgt_name : "(tgt=NULL)", acg->acg_name);
 
+	mutex_lock(&scst_mutex);
+
 	list_for_each_entry_safe(acg_dev, acg_dev_tmp, &acg->acg_dev_list,
 			acg_dev_list_entry) {
 		struct scst_tgt_dev *tgt_dev, *tt;
 
+		acg_dev->nr_deleted_tgt_devs = 1;
+
 		list_for_each_entry_safe(tgt_dev, tt,
 				 &acg_dev->dev->dev_tgt_dev_list,
 				 dev_tgt_dev_list_entry) {
@@ -4846,15 +4840,21 @@ static void scst_free_acg(struct scst_acg *acg)
 				sess = tgt_dev->sess;
 
 				mutex_lock(&sess->tgt_dev_list_mutex);
+				acg_dev->nr_deleted_tgt_devs++;
+				tgt_dev->dec_acg_refcnt = true;
 				scst_del_tgt_dev(tgt_dev);
 				mutex_unlock(&sess->tgt_dev_list_mutex);
-
-				synchronize_rcu();
-				scst_free_tgt_dev(tgt_dev);
 			}
 		}
-		scst_free_acg_dev(acg_dev);
+		mutex_unlock(&scst_mutex);
+
+		synchronize_rcu();
+
+		mutex_lock(&scst_mutex);
+		if (--acg_dev->nr_deleted_tgt_devs == 0)
+			scst_free_acg_dev(acg_dev);
 	}
+	mutex_unlock(&scst_mutex);
 
 	list_for_each_entry_safe(acn, acnt, &acg->acn_list, acn_list_entry) {
 		scst_free_acn(acn,
@@ -5290,9 +5290,6 @@ void scst_tgt_dev_stop_threads(struct scst_tgt_dev *tgt_dev)
 
 	lockdep_assert_held(&scst_mutex);
 
-	if (tgt_dev->dev->threads_num < 0)
-		goto out_deinit;
-
 	if (tgt_dev->active_cmd_threads == &scst_main_cmd_threads) {
 		/* Global async threads */
 		kref_put(&tgt_dev->aic_keeper->aic_keeper_kref,
@@ -5309,7 +5306,6 @@ void scst_tgt_dev_stop_threads(struct scst_tgt_dev *tgt_dev)
 		scst_deinit_threads(&tgt_dev->tgt_dev_cmd_threads);
 	} /* else no threads (not yet initialized, e.g.) */
 
-out_deinit:
 	tm_dbg_deinit_tgt_dev(tgt_dev);
 	tgt_dev->active_cmd_threads = NULL;
 
@@ -5347,6 +5343,8 @@ static int scst_alloc_add_tgt_dev(struct scst_session *sess,
 	}
 
 	INIT_LIST_HEAD(&tgt_dev->sess_tgt_dev_list_entry);
+	init_rcu_head(&tgt_dev->rcu);
+	INIT_WORK(&tgt_dev->free_work, scst_tgt_dev_free_workfn);
 	tgt_dev->tgtt = tgtt;
 	tgt_dev->dev = dev;
 	tgt_dev->lun = acg_dev->lun;
@@ -5512,6 +5510,8 @@ out_dec_free:
 out_free_ua:
 	scst_free_all_UA(tgt_dev);
 
+	destroy_rcu_head(&tgt_dev->rcu);
+
 	kmem_cache_free(scst_tgtd_cachep, tgt_dev);
 	goto out;
 }
@@ -5559,7 +5559,8 @@ static void scst_del_tgt_dev(struct scst_tgt_dev *tgt_dev)
 	if (tgtt->get_initiator_port_transport_id == NULL)
 		dev->not_pr_supporting_tgt_devs_num--;
 
-	atomic_dec(&tgt_dev->tgt_dev_cmd_count);
+	if (atomic_dec_return(&tgt_dev->tgt_dev_cmd_count) == 0)
+		call_rcu(&tgt_dev->rcu, scst_free_tgt_dev_rcu);
 }
 
 /*
@@ -5574,9 +5575,6 @@ static void scst_free_tgt_dev(struct scst_tgt_dev *tgt_dev)
 
 	TRACE_ENTRY();
 
-#ifdef CONFIG_SCST_EXTRACHECKS
-	WARN_ON_ONCE(scst_is_active_tgt_dev(tgt_dev));
-#endif
 	WARN_ON_ONCE(atomic_read(&tgt_dev->tgt_dev_cmd_count) != 0);
 
 	scst_clear_reservation(tgt_dev);
@@ -5592,6 +5590,8 @@ static void scst_free_tgt_dev(struct scst_tgt_dev *tgt_dev)
 
 	scst_tgt_dev_stop_threads(tgt_dev);
 
+	destroy_rcu_head(&tgt_dev->rcu);
+
 	kmem_cache_free(scst_tgtd_cachep, tgt_dev);
 
 	percpu_ref_put(&dev->refcnt);
@@ -5641,8 +5641,6 @@ void scst_sess_free_tgt_devs(struct scst_session *sess)
 		list_for_each_entry_safe(tgt_dev, t, head,
 				sess_tgt_dev_list_entry) {
 			scst_del_tgt_dev(tgt_dev);
-			synchronize_rcu();
-			scst_free_tgt_dev(tgt_dev);
 		}
 		INIT_LIST_HEAD(head);
 	}

scst/src/scst_priv.h

@@ -382,6 +382,7 @@ void scst_check_reassign_sessions(void);
 int scst_sess_alloc_tgt_devs(struct scst_session *sess);
 void scst_sess_free_tgt_devs(struct scst_session *sess);
 struct scst_tgt_dev *scst_lookup_tgt_dev(struct scst_session *sess, u64 lun);
+void scst_free_tgt_dev_rcu(struct rcu_head *rcu);
 void scst_nexus_loss(struct scst_tgt_dev *tgt_dev, bool queue_UA);
 
 #define SCST_ADD_LUN_READ_ONLY	1

scst/src/scst_targ.c

@@ -4504,7 +4504,8 @@ static int scst_pre_xmit_response1(struct scst_cmd *cmd)
 		 * latency, so we should decrement them after cmd completed.
 		 */
 		smp_mb__before_atomic_dec();
-		atomic_dec(&cmd->tgt_dev->tgt_dev_cmd_count);
+		if (atomic_dec_return(&cmd->tgt_dev->tgt_dev_cmd_count) == 0)
+			call_rcu(&cmd->tgt_dev->rcu, scst_free_tgt_dev_rcu);
 		percpu_ref_put(&cmd->dev->refcnt);
 #ifdef CONFIG_SCST_PER_DEVICE_CMD_COUNT_LIMIT
 		atomic_dec(&cmd->dev->dev_cmd_count);