kunit: Cover task management across ACG reassignment

Issue #367 reports a TM thread dereferencing sess->acg while the session
reassignment path temporarily clears it. Add private fixtures covering
all black_hole modes for teardown TM functions, mode changes after real
reassignment, and the unchanged-group path.

Invoke the TM reader from the allocation stub during reassignment and
check command state, drop policy and event delivery for all five modes.
Keep the old and new modes equal in this case so that its expected policy
is unambiguous. The regression runs without SKIP and may crash the test
VM before a production fix; document this execution requirement.

Expose only private test entry points and task-scoped redirects; normal
build behavior is unchanged.

Link: https://github.com/SCST-project/scst/issues/367

doc/KUNIT.md

@@ -64,7 +64,7 @@ files. Base labels come directly from suite registrations:
 
 | Module | Labels |
 | --- | --- |
-| `scst.ko` | `ALUA CDB CM EVENT LIB LIFECYCLE ORDER UNMAP` |
+| `scst.ko` | `ALUA CDB CM EVENT LIB LIFECYCLE ORDER TM UNMAP` |
 | `scst_vdisk.ko` | `VDISK` |
 | `scst_user.ko` | `USER` |
 | `iscsi-scst.ko` | `ISCSI` |
@@ -153,6 +153,29 @@ preloaded KUnit is left unchanged. External dependencies such as `dlm` are
 loaded with `modprobe` and left loaded. A clean failure before SCST loads also
 releases runner-owned KUnit; incomplete execution can retain modules.
 
+## Issue #367: task management and ACG reassignment
+
+`KUNIT_LABEL=TM` selects `scst-TM-acg`. It checks five `black_hole` modes
+for session teardown commands, mode changes across ACG reassignment, and
+unchanged-group handling. These checks should pass before a production fix.
+`tm_during_reassign_test` also invokes the TM reader from an allocation stub
+while the real reassignment is in progress, for all five modes. It checks
+command state, drop policy and event delivery during and after the transition.
+This case is enabled without a SKIP: before the production fix, its reader
+can crash the test VM. Use a disposable VM and retain its console log outside
+the guest. The fixture controls one interleaving; it does not prove the
+absence of other races.
+
+```sh
+make kunit-build KDIR=/path/to/kernel-build
+sudo make kunit-run KUNIT_LABEL=TM KUNIT_ARGS='--verbose'
+```
+
+Retain the complete printed run directory, especially `report.txt`,
+`result.json`, `messages.log`, `kernel.log`, `commands.log` and
+`ktap/scst-TM-acg.ktap`. The companion TCP reproduction attempts are described
+in the [component-test guide](../tests/ginkgo/README.md#issue-367-acg-reassignment).
+
 ## Add a suite
 
 Keep suites and fixtures under their owning component's `kunit/` directory;
@@ -190,8 +213,8 @@ redirects. Test interfaces are not installed headers or module exports.
 
 Fixtures must tolerate failed initialization and assertions. Bound worker
 waits; unfinished work must retain its state and module reference and report
-`scst_kunit_retain()`. Unsafe cases report `SKIP` pending separate production
-fixes. Storage and transport integration belongs in
+`scst_kunit_retain()`. Document known failures of enabled regression cases and
+their execution requirements. Storage and transport integration belongs in
 [component tests](../tests/ginkgo/README.md); upstream QLogic initiator/HBA
 code is out of scope.
 

scst/src/kunit/objects.mk

@@ -12,6 +12,7 @@ scst-kunit-scst := \
 	kunit/lifecycle/suites/scst_kunit_lifecycle.o \
 	kunit/lifecycle/suites/scst_kunit_blocking.o \
 	kunit/order/suites/scst_kunit_order.o \
+	kunit/tm/suites/scst_kunit_tm.o \
 	kunit/unmap/suites/scst_kunit_unmap.o
 
 scst-kunit-iscsi-scst := \

scst/src/kunit/tm/suites/scst_kunit_tm.c

@@ -0,0 +1,255 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include "../../scst_kunit.h"
+
+#ifdef INSIDE_KERNEL_TREE
+#include <scst/scst.h>
+#else
+#include "scst.h"
+#endif
+#include "../../../scst_priv.h"
+#include "../../../scst_lib_test.h"
+#include "../../../scst_targ_test.h"
+
+struct tm_fixture {
+	struct scst_tgt_template tgtt;
+	struct scst_tgt tgt;
+	struct scst_session sess;
+	struct scst_acg old_acg;
+	struct scst_acg new_acg;
+	struct scst_acg_dev acg_dev;
+	struct scst_device dev;
+	unsigned int allocations;
+	unsigned int links;
+	unsigned int events;
+	bool check_tm_during_reassign;
+	bool expected_tm_dropped;
+};
+
+static void tm_check_teardown_commands(struct kunit *test, bool dropped);
+
+static int tm_event_stub(struct scst_mgmt_cmd *mcmd)
+{
+	struct kunit *test = kunit_get_current_test();
+	struct tm_fixture *f = test->priv;
+
+	KUNIT_EXPECT_PTR_EQ(test, mcmd->sess, &f->sess);
+	KUNIT_EXPECT_EQ(test, mcmd->state, SCST_MCMD_STATE_EXEC);
+	f->events++;
+	return 0;
+}
+
+static int tm_luns_link_stub(struct scst_session *sess)
+{
+	struct kunit *test = kunit_get_current_test();
+	struct tm_fixture *f = test->priv;
+
+	lockdep_assert_held(&scst_mutex);
+	KUNIT_EXPECT_PTR_EQ(test, sess, &f->sess);
+	KUNIT_EXPECT_PTR_EQ(test, sess->acg, &f->new_acg);
+	f->links++;
+	return 0;
+}
+
+static int tm_alloc_tgt_dev_stub(struct scst_session *sess,
+				 struct scst_acg_dev *acg_dev, struct scst_tgt_dev **out)
+{
+	struct kunit *test = kunit_get_current_test();
+	struct tm_fixture *f = test->priv;
+
+	lockdep_assert_held(&scst_mutex);
+	KUNIT_EXPECT_PTR_EQ(test, sess, &f->sess);
+	KUNIT_EXPECT_PTR_EQ(test, acg_dev, &f->acg_dev);
+	f->allocations++;
+	/* Exercise the reader before the real reassignment has completed. */
+	if (f->check_tm_during_reassign)
+		tm_check_teardown_commands(test, f->expected_tm_dropped);
+	*out = NULL;
+	return -EPERM;
+}
+
+static int tm_test_init(struct kunit *test)
+{
+	struct tm_fixture *f;
+	int i;
+
+	f = kunit_kzalloc(test, sizeof(*f), GFP_KERNEL);
+	if (!f)
+		return -ENOMEM;
+	test->priv = f;
+	f->tgt.tgtt = &f->tgtt;
+	f->tgt.default_acg = &f->new_acg;
+	INIT_LIST_HEAD(&f->tgt.tgt_acg_list);
+	f->sess.tgt = &f->tgt;
+	f->sess.initiator_name = "kunit-private-tm-initiator";
+	f->sess.init_phase = SCST_SESS_IPH_READY;
+	f->sess.shut_phase = SCST_SESS_SPH_READY;
+	f->sess.acg = &f->old_acg;
+	mutex_init(&f->sess.tgt_dev_list_mutex);
+	for (i = 0; i < SESS_TGT_DEV_LIST_HASH_SIZE; i++)
+		INIT_LIST_HEAD(&f->sess.sess_tgt_dev_list[i]);
+	f->old_acg.acg_name = "kunit-private-old-acg";
+	f->new_acg.acg_name = "kunit-private-new-acg";
+	INIT_LIST_HEAD(&f->old_acg.acg_sess_list);
+	INIT_LIST_HEAD(&f->new_acg.acg_sess_list);
+	INIT_LIST_HEAD(&f->old_acg.acg_dev_list);
+	INIT_LIST_HEAD(&f->new_acg.acg_dev_list);
+	kref_init(&f->old_acg.acg_kref);
+	kref_init(&f->new_acg.acg_kref);
+	/* Keep a fixture reference: reassignment must not schedule ACG release. */
+	scst_get_acg(&f->old_acg);
+	list_add_tail(&f->sess.acg_sess_list_entry, &f->old_acg.acg_sess_list);
+	f->acg_dev.acg = &f->new_acg;
+	f->acg_dev.dev = &f->dev;
+	f->acg_dev.lun = 1;
+	list_add_tail(&f->acg_dev.acg_dev_list_entry, &f->new_acg.acg_dev_list);
+	kunit_activate_static_stub(test, scst_alloc_add_tgt_dev, tm_alloc_tgt_dev_stub);
+	kunit_activate_static_stub(test, scst_recreate_sess_luns_link, tm_luns_link_stub);
+	kunit_activate_static_stub(test, scst_event_queue_tm_fn_received, tm_event_stub);
+	return 0;
+}
+
+static void tm_test_exit(struct kunit *test)
+{
+	struct tm_fixture *f = test->priv;
+
+	list_del_init(&f->sess.acg_sess_list_entry);
+	scst_put_acg(f->sess.acg);
+	KUNIT_EXPECT_EQ(test, kref_read(&f->old_acg.acg_kref), 1U);
+	KUNIT_EXPECT_EQ(test, kref_read(&f->new_acg.acg_kref), 1U);
+	/* KUnit owns both ACG storage objects and their remaining references. */
+}
+
+struct tm_mode_case {
+	const char *name;
+	int mode;
+	bool dropped;
+};
+
+static const struct tm_mode_case tm_modes[] = {
+	{ "none", SCST_ACG_BLACK_HOLE_NONE, false },
+	{ "commands", SCST_ACG_BLACK_HOLE_CMD, false },
+	{ "all", SCST_ACG_BLACK_HOLE_ALL, true },
+	{ "data_commands", SCST_ACG_BLACK_HOLE_DATA_CMD, false },
+	{ "data_and_tm", SCST_ACG_BLACK_HOLE_DATA_MCMD, true },
+};
+
+static void tm_mode_desc(const struct tm_mode_case *p, char *desc)
+{
+	strscpy(desc, p->name, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(tm_mode, tm_modes, tm_mode_desc);
+
+static void tm_check_teardown_commands(struct kunit *test, bool dropped)
+{
+	static const int functions[] = {
+		SCST_NEXUS_LOSS_SESS, SCST_ABORT_ALL_TASKS_SESS,
+		SCST_NEXUS_LOSS, SCST_ABORT_ALL_TASKS, SCST_UNREG_SESS_TM,
+	};
+	struct tm_fixture *f = test->priv;
+	unsigned int events = f->events;
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(functions); i++) {
+		struct scst_mgmt_cmd mcmd = {
+			.sess = &f->sess,
+			.fn = functions[i],
+			.state = SCST_MCMD_STATE_INIT,
+		};
+
+		KUNIT_EXPECT_EQ_MSG(test, scst_mgmt_cmd_init(&mcmd), 0,
+				    "TM function %d", functions[i]);
+		KUNIT_EXPECT_EQ(test, mcmd.state, SCST_MCMD_STATE_EXEC);
+		KUNIT_EXPECT_EQ_MSG(test, (bool)mcmd.mcmd_dropped, dropped,
+				    "TM function %d", functions[i]);
+		KUNIT_EXPECT_PTR_EQ(test, mcmd.mcmd_tgt_dev, NULL);
+		KUNIT_EXPECT_PTR_EQ(test, mcmd.cmd_to_abort, NULL);
+	}
+	KUNIT_EXPECT_EQ(test, f->events - events, (unsigned int)ARRAY_SIZE(functions));
+}
+
+static void tm_black_hole_test(struct kunit *test)
+{
+	const struct tm_mode_case *p = test->param_value;
+	struct tm_fixture *f = test->priv;
+
+	f->old_acg.acg_black_hole_type = p->mode;
+	tm_check_teardown_commands(test, p->dropped);
+}
+
+static void tm_reassign(struct kunit *test)
+{
+	struct tm_fixture *f = test->priv;
+
+	/* Only private objects are reachable; no live activity needs suspension. */
+	mutex_lock(&scst_mutex);
+	scst_check_reassign_sess(&f->sess);
+	mutex_unlock(&scst_mutex);
+}
+
+static void tm_reassign_mode_test(struct kunit *test)
+{
+	const struct tm_mode_case *p = test->param_value;
+	struct tm_fixture *f = test->priv;
+
+	/* Cover both entering and leaving the modes which drop TM commands. */
+	f->old_acg.acg_black_hole_type = p->dropped ?
+		SCST_ACG_BLACK_HOLE_NONE : SCST_ACG_BLACK_HOLE_ALL;
+	f->new_acg.acg_black_hole_type = p->mode;
+	tm_check_teardown_commands(test, !p->dropped);
+	tm_reassign(test);
+	KUNIT_ASSERT_PTR_EQ(test, f->sess.acg, &f->new_acg);
+	KUNIT_EXPECT_EQ(test, f->allocations, 1U);
+	KUNIT_EXPECT_EQ(test, f->links, 1U);
+	KUNIT_EXPECT_TRUE(test, list_empty(&f->old_acg.acg_sess_list));
+	KUNIT_EXPECT_TRUE(test, list_is_singular(&f->new_acg.acg_sess_list));
+	KUNIT_EXPECT_EQ(test, kref_read(&f->old_acg.acg_kref), 1U);
+	KUNIT_EXPECT_EQ(test, kref_read(&f->new_acg.acg_kref), 2U);
+	tm_check_teardown_commands(test, p->dropped);
+}
+
+static void tm_during_reassign_test(struct kunit *test)
+{
+	const struct tm_mode_case *p = test->param_value;
+	struct tm_fixture *f = test->priv;
+
+	/* The expected TM policy is unambiguous throughout this transition. */
+	f->old_acg.acg_black_hole_type = p->mode;
+	f->new_acg.acg_black_hole_type = p->mode;
+	f->expected_tm_dropped = p->dropped;
+	f->check_tm_during_reassign = true;
+	tm_reassign(test);
+	f->check_tm_during_reassign = false;
+	KUNIT_ASSERT_EQ(test, f->allocations, 1U);
+	KUNIT_EXPECT_EQ(test, f->links, 1U);
+	KUNIT_ASSERT_PTR_EQ(test, f->sess.acg, &f->new_acg);
+	tm_check_teardown_commands(test, p->dropped);
+}
+
+static void tm_same_acg_test(struct kunit *test)
+{
+	struct tm_fixture *f = test->priv;
+
+	f->tgt.default_acg = &f->old_acg;
+	tm_reassign(test);
+	KUNIT_EXPECT_PTR_EQ(test, f->sess.acg, &f->old_acg);
+	KUNIT_EXPECT_EQ(test, f->allocations, 0U);
+	KUNIT_EXPECT_EQ(test, f->links, 0U);
+	KUNIT_EXPECT_TRUE(test, list_is_singular(&f->old_acg.acg_sess_list));
+	KUNIT_EXPECT_TRUE(test, list_empty(&f->new_acg.acg_sess_list));
+	tm_check_teardown_commands(test, false);
+}
+
+static struct kunit_case tm_cases[] = {
+	KUNIT_CASE_PARAM(tm_black_hole_test, tm_mode_gen_params),
+	KUNIT_CASE_PARAM(tm_reassign_mode_test, tm_mode_gen_params),
+	KUNIT_CASE(tm_same_acg_test),
+	KUNIT_CASE_PARAM(tm_during_reassign_test, tm_mode_gen_params),
+	{}
+};
+
+scst_kunit_suite(scst_tm_suite, "TM", "-acg",
+		 .init = tm_test_init,
+		 .exit = tm_test_exit,
+		 .test_cases = tm_cases,
+);

scst/src/scst_event.c

@@ -434,6 +434,8 @@ int scst_event_queue_tm_fn_received(struct scst_mgmt_cmd *mcmd)
 	struct scst_event *event;
 	struct scst_event_tm_fn_received_payload *payload;
 
+	SCST_KUNIT_REDIRECT(scst_event_queue_tm_fn_received, mcmd);
+
 	TRACE_ENTRY();
 
 	event_entry_len = sizeof(*event_entry) + sizeof(*payload);

scst/src/scst_lib.c

@@ -3003,7 +3003,7 @@ void scst_dev_inquiry_data_changed(struct scst_device *dev)
 EXPORT_SYMBOL(scst_dev_inquiry_data_changed);
 
 /* The activity supposed to be suspended and scst_mutex held */
-static void scst_check_reassign_sess(struct scst_session *sess)
+SCST_KUNIT_VISIBLE void scst_check_reassign_sess(struct scst_session *sess)
 {
 	struct scst_acg *acg, *old_acg;
 	struct scst_acg_dev *acg_dev;

scst/src/scst_lib_test.h

@@ -18,6 +18,7 @@ int scst_alloc_add_tgt_dev(struct scst_session *sess,
 			   struct scst_acg_dev *acg_dev, struct scst_tgt_dev **out_tgt_dev);
 void scst_free_tgt_dev(struct scst_tgt_dev *tgt_dev);
 void scst_sess_free_tgt_devs(struct scst_session *sess);
+void scst_check_reassign_sess(struct scst_session *sess);
 void scst_ext_blocker_put(struct scst_ext_blocker *b);
 void scst_ext_blocking_done_fn(struct work_struct *work);
 int scst_wait_ext_block_dev(struct scst_device *dev, wait_queue_head_t *w);

scst/src/scst_sysfs.c

@@ -5025,6 +5025,8 @@ static int scst_create_sess_luns_link(struct scst_session *sess)
 
 int scst_recreate_sess_luns_link(struct scst_session *sess)
 {
+	SCST_KUNIT_REDIRECT(scst_recreate_sess_luns_link, sess);
+
 	sysfs_remove_link(&sess->sess_kobj, "luns");
 	return scst_create_sess_luns_link(sess);
 }

scst/src/scst_targ.c

@@ -5642,7 +5642,7 @@ static int scst_clear_task_set(struct scst_mgmt_cmd *mcmd)
  * Returns 0 if the command processing should be continued,
  * >0, if it should be requeued, <0 otherwise.
  */
-static int scst_mgmt_cmd_init(struct scst_mgmt_cmd *mcmd)
+SCST_KUNIT_VISIBLE int scst_mgmt_cmd_init(struct scst_mgmt_cmd *mcmd)
 {
 	int res = 0, rc, t;
 

scst/src/scst_targ_test.h

@@ -7,10 +7,12 @@
 #ifdef SCST_KUNIT_ENABLED
 
 struct scst_cmd;
+struct scst_mgmt_cmd;
 
 bool scst_cmd_overlap(struct scst_cmd *chk_cmd, struct scst_cmd *cmd);
 int scst_parse_cmd(struct scst_cmd *cmd);
 void scst_cmd_set_sn(struct scst_cmd *cmd);
+int scst_mgmt_cmd_init(struct scst_mgmt_cmd *mcmd);
 
 #endif /* SCST_KUNIT_ENABLED */
 #endif /* SCST_TARG_TEST_H */