kunit: Test scst_user cleanup and reply ownership

Link a USER suite with private cleanup access and a task-scoped
pool-flush boundary. Check existing cleanup-yield behavior, post-unjam
flush ordering and session reply ownership without changing logic or
destroying real SGV pools.

doc/KUNIT.md

@@ -62,6 +62,7 @@ validates the selection against the supplied `.ko` files.
 | --- | --- |
 | `scst.ko` | `ALUA CDB CM EVENT LIB LIFECYCLE ORDER UNMAP` |
 | `scst_vdisk.ko` | `VDISK` |
+| `scst_user.ko` | `USER` |
 
 The runner reads suite metadata and dependencies from the modules and loads selected
 owners in dependency order. Supply all required SCST modules from the same build;

scripts/generate-kernel-patch

@@ -322,7 +322,8 @@ scst_07_pres="scst/src/scst_pres.h scst/src/scst_pres.c"
 scst_08_sysfs="scst/src/scst_sysfs.c"
 scst_09_debug="scst/include/scst_debug.h scst/src/scst_debug.c"
 scst_10_sgv="scst/include/scst_sgv.h scst/src/scst_mem.h scst/src/scst_mem.c doc/scst_pg.sgml"
-scst_user="scst/include/scst_user.h scst/src/dev_handlers/scst_user_main.c"
+scst_user="scst/include/scst_user.h scst/src/dev_handlers/scst_user_main.c \
+scst/src/dev_handlers/scst_user_test.h"
 scst_13_vdisk="scst/src/dev_handlers/scst_vdisk_main.c \
 scst/src/dev_handlers/scst_vdisk_test.h"
 scst_14_tg="scst/src/scst_tg.c"

scripts/kunit/test_make.py

@@ -24,6 +24,7 @@ ENTRY_POINTS = [entry for entry in ('', 'scst', 'scst/src', 'scst/src/dev_handle
 OWNERS = {
     'scst': ('scst/src', 'scst/kernel/in-tree/Makefile.scst'),
     'scst_vdisk': ('scst/src/dev_handlers', 'scst/kernel/in-tree/Makefile.dev_handlers'),
+    'scst_user': ('scst/src/dev_handlers', 'scst/kernel/in-tree/Makefile.dev_handlers'),
 }
 
 

scst/src/dev_handlers/kunit/user/suites/scst_user_cleanup_test.c

@@ -0,0 +1,222 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include "../../../../kunit/scst_kunit.h"
+#ifdef INSIDE_KERNEL_TREE
+#include <scst/scst.h>
+#include <scst/scst_user.h>
+#else
+#include "scst.h"
+#include "scst_user.h"
+#endif
+#include "../../../scst_user_priv.h"
+#include "../../../scst_user_test.h"
+
+struct user_cleanup_fixture {
+	struct scst_user_dev devices[2];
+	struct scst_user_cmd pending[2];
+	struct completion reply;
+	unsigned int pool_tokens[3];
+	unsigned int flushes;
+	bool return_on_second_flush;
+	bool shared;
+	bool guard_fired;
+};
+
+static void user_cleanup_flush(struct sgv_pool *pool)
+{
+	struct kunit *test = kunit_get_current_test();
+	struct user_cleanup_fixture *f = test->priv;
+	unsigned int i;
+
+	KUNIT_EXPECT_TRUE(test, pool == f->devices[0].pool || pool == f->devices[1].pool ||
+			  pool == f->devices[0].pool_clust);
+	f->flushes++;
+	/* A buggy inner retry must fail instead of hanging the KUnit executor. */
+	if (f->flushes > 16) {
+		KUNIT_FAIL(test, "cleanup did not yield to the other device");
+		f->guard_fired = true;
+		for (i = 0; i < 2; i++) {
+			list_del_init(&f->pending[i].hash_list_entry);
+			f->devices[i].cleanup_done = 1;
+		}
+	}
+	/* Model the destructor releasing A's retained reference after B returns it. */
+	if ((f->shared && completion_done(&f->reply)) ||
+	    (f->return_on_second_flush && f->flushes == 3)) {
+		for (i = 0; i < 2; i++)
+			list_del_init(&f->pending[i].hash_list_entry);
+	}
+}
+
+static int user_cleanup_init(struct kunit *test)
+{
+	struct user_cleanup_fixture *f = kunit_kzalloc(test, sizeof(*f), GFP_KERNEL);
+	unsigned int i, j;
+
+	if (!f)
+		return -ENOMEM;
+	test->priv = f;
+	init_completion(&f->reply);
+	for (i = 0; i < 2; i++) {
+		struct scst_user_dev *dev = &f->devices[i];
+		struct scst_user_cmd *u = &f->pending[i];
+
+		spin_lock_init(&dev->udev_cmd_threads.cmd_list_lock);
+		init_waitqueue_head(&dev->udev_cmd_threads.cmd_list_waitQ);
+		INIT_LIST_HEAD(&dev->udev_cmd_threads.active_cmd_list);
+		INIT_LIST_HEAD(&dev->ready_cmd_list);
+		init_completion(&dev->cleanup_cmpl);
+		for (j = 0; j < ARRAY_SIZE(dev->ucmd_hash); j++)
+			INIT_LIST_HEAD(&dev->ucmd_hash[j]);
+		/* Opaque tokens consumed only by the task-scoped pool-flush stub. */
+		dev->pool = (void *)&f->pool_tokens[i];
+		dev->pool_clust = (void *)&f->pool_tokens[2];
+		u->dev = dev;
+		u->state = UCMD_STATE_ATTACH_SESS;
+		u->cmpl = &f->reply;
+		atomic_set(&u->ucmd_ref, 2); /* Fixture reference plus outstanding reply. */
+		INIT_LIST_HEAD(&u->hash_list_entry);
+		INIT_LIST_HEAD(&u->ready_cmd_list_entry);
+	}
+	kunit_activate_static_stub(test, dev_user_flush_pool, user_cleanup_flush);
+	return 0;
+}
+
+struct user_cleanup_case {
+	const char *name;
+	bool done;
+	bool pending;
+	bool flush_return;
+};
+
+static const struct user_cleanup_case user_cleanup_cases[] = {
+	{ "empty_waiting_for_core" },
+	{ "empty_complete", .done = true },
+	{ "pending_waiting_for_core", .pending = true },
+	{ "pending_after_core_done", .done = true, .pending = true },
+	{ "post_unjam_flush_releases_reference", .done = true, .pending = true,
+	  .flush_return = true },
+};
+
+static void user_cleanup_desc(const struct user_cleanup_case *p, char *desc)
+{
+	snprintf(desc, KUNIT_PARAM_DESC_SIZE, "%s", p->name);
+}
+
+KUNIT_ARRAY_PARAM(user_cleanup, user_cleanup_cases, user_cleanup_desc);
+
+static void user_cleanup_test(struct kunit *test)
+{
+	const struct user_cleanup_case *p = test->param_value;
+	struct user_cleanup_fixture *f = test->priv;
+	struct scst_user_dev *dev = f->devices;
+	bool complete = p->done && !p->pending;
+
+	dev->cleanup_done = p->done;
+	if (p->pending)
+		list_add_tail(&f->pending[0].hash_list_entry, &dev->ucmd_hash[0]);
+	f->return_on_second_flush = p->flush_return;
+	KUNIT_EXPECT_EQ(test, dev_user_process_cleanup(dev), complete ? 0 : 1);
+	KUNIT_EXPECT_EQ(test, completion_done(&dev->cleanup_cmpl), complete);
+	KUNIT_EXPECT_EQ(test, f->flushes, 4U);
+	KUNIT_EXPECT_FALSE(test, f->guard_fired);
+	if (p->flush_return) {
+		KUNIT_EXPECT_TRUE(test, list_empty(&dev->ucmd_hash[0]));
+		KUNIT_EXPECT_EQ(test, dev_user_process_cleanup(dev), 0);
+		KUNIT_EXPECT_TRUE(test, completion_done(&dev->cleanup_cmpl));
+	}
+}
+
+static void user_cleanup_shared_test(struct kunit *test)
+{
+	struct user_cleanup_fixture *f = test->priv;
+	unsigned int i;
+
+	f->shared = true;
+	for (i = 0; i < 2; i++) {
+		f->devices[i].cleanup_done = 1;
+		list_add_tail(&f->pending[i].hash_list_entry, &f->devices[i].ucmd_hash[0]);
+	}
+	f->pending[1].sent_to_user = 1;
+	KUNIT_ASSERT_EQ(test, dev_user_process_cleanup(&f->devices[0]), 1);
+	KUNIT_EXPECT_FALSE(test, completion_done(&f->devices[0].cleanup_cmpl));
+	KUNIT_EXPECT_FALSE(test, completion_done(&f->reply));
+	KUNIT_EXPECT_EQ(test, f->flushes, 4U);
+	KUNIT_ASSERT_EQ(test, dev_user_process_cleanup(&f->devices[1]), 1);
+	KUNIT_EXPECT_TRUE(test, completion_done(&f->reply));
+	KUNIT_EXPECT_EQ(test, f->pending[1].result, -EFAULT);
+	KUNIT_EXPECT_EQ(test, atomic_read(&f->pending[1].ucmd_ref), 1);
+	KUNIT_EXPECT_EQ(test, dev_user_process_cleanup(&f->devices[0]), 0);
+	KUNIT_EXPECT_EQ(test, dev_user_process_cleanup(&f->devices[1]), 0);
+	for (i = 0; i < 2; i++)
+		KUNIT_EXPECT_TRUE(test, completion_done(&f->devices[i].cleanup_cmpl));
+	KUNIT_EXPECT_FALSE(test, f->guard_fired);
+}
+
+struct user_reply_case {
+	const char *name;
+	bool detach;
+	bool repeat;
+	bool saved_flags;
+	bool no_completion;
+};
+
+static const struct user_reply_case user_reply_cases[] = {
+	{ "attach_failure" },
+	{ "detach_failure", .detach = true },
+	{ "attach_notified_once", .repeat = true },
+	{ "detach_notified_once", .detach = true, .repeat = true },
+	{ "attach_irqsave_caller", .saved_flags = true },
+	{ "detach_irqsave_caller", .detach = true, .saved_flags = true },
+	{ "attach_without_waiter", .no_completion = true },
+	{ "detach_without_waiter", .detach = true, .no_completion = true },
+};
+
+static void user_reply_desc(const struct user_reply_case *p, char *desc)
+{
+	snprintf(desc, KUNIT_PARAM_DESC_SIZE, "%s", p->name);
+}
+
+KUNIT_ARRAY_PARAM(user_reply, user_reply_cases, user_reply_desc);
+
+static void user_reply_test(struct kunit *test)
+{
+	const struct user_reply_case *p = test->param_value;
+	struct user_cleanup_fixture *f = test->priv;
+	struct scst_user_cmd *u = f->pending;
+	spinlock_t *lock = &f->devices[0].udev_cmd_threads.cmd_list_lock;
+	unsigned long flags = 0;
+
+	u->state = p->detach ? UCMD_STATE_DETACH_SESS : UCMD_STATE_ATTACH_SESS;
+	u->sent_to_user = 1;
+	if (p->no_completion)
+		u->cmpl = NULL;
+	if (p->saved_flags)
+		spin_lock_irqsave(lock, flags);
+	else
+		spin_lock_irq(lock);
+	dev_user_unjam_cmd(u, 0, p->saved_flags ? &flags : NULL);
+	if (p->repeat)
+		dev_user_unjam_cmd(u, 0, p->saved_flags ? &flags : NULL);
+	if (p->saved_flags)
+		spin_unlock_irqrestore(lock, flags);
+	else
+		spin_unlock_irq(lock);
+	KUNIT_EXPECT_EQ(test, completion_done(&f->reply), !p->no_completion);
+	KUNIT_EXPECT_EQ(test, u->result, p->detach ? 0 : -EFAULT);
+	KUNIT_EXPECT_EQ(test, atomic_read(&u->ucmd_ref), 1);
+	KUNIT_EXPECT_TRUE(test, !!u->jammed);
+	KUNIT_EXPECT_TRUE(test, !!u->this_state_unjammed);
+	KUNIT_EXPECT_FALSE(test, !!u->sent_to_user);
+}
+
+static struct kunit_case user_cleanup_cases_kunit[] = {
+	KUNIT_CASE_PARAM(user_cleanup_test, user_cleanup_gen_params),
+	KUNIT_CASE(user_cleanup_shared_test),
+	KUNIT_CASE_PARAM(user_reply_test, user_reply_gen_params),
+	{}
+};
+
+scst_kunit_suite(user_cleanup_suite, "USER", "-cleanup",
+		 .init = user_cleanup_init,
+		 .test_cases = user_cleanup_cases_kunit,
+);

scst/src/dev_handlers/scst_user_main.c

@@ -36,6 +36,11 @@
 #endif
 #include "scst_dev_handler.h"
 #include "scst_user_priv.h"
+#include "scst_user_test.h"
+
+#ifndef SCST_KUNIT_ENABLED
+static void dev_user_unjam_cmd(struct scst_user_cmd *ucmd, int busy, unsigned long *flags);
+#endif
 
 #ifndef INSIDE_KERNEL_TREE
 #if defined(CONFIG_HIGHMEM4G) || defined(CONFIG_HIGHMEM64G)
@@ -56,8 +61,6 @@ static void dev_user_free_sg_entries(struct scatterlist *sg, int sg_count,
 
 static void dev_user_add_to_ready(struct scst_user_cmd *ucmd);
 
-static void dev_user_unjam_cmd(struct scst_user_cmd *ucmd, int busy, unsigned long *flags);
-
 static int dev_user_process_reply_on_free(struct scst_user_cmd *ucmd);
 static int dev_user_process_reply_tm_exec(struct scst_user_cmd *ucmd, int status);
 static int dev_user_process_reply_sess(struct scst_user_cmd *ucmd, int status);
@@ -2450,7 +2453,8 @@ out:
  * Called under udev_cmd_threads.cmd_list_lock, but can drop it inside,
  * then reacquire.
  */
-static void dev_user_unjam_cmd(struct scst_user_cmd *ucmd, int busy, unsigned long *flags)
+SCST_KUNIT_VISIBLE
+void dev_user_unjam_cmd(struct scst_user_cmd *ucmd, int busy, unsigned long *flags)
 	__releases(&dev->udev_cmd_threads.cmd_list_lock)
 	__acquires(&dev->udev_cmd_threads.cmd_list_lock)
 {
@@ -2575,6 +2579,13 @@ out:
 	TRACE_EXIT();
 }
 
+SCST_KUNIT_VISIBLE
+void dev_user_flush_pool(struct sgv_pool *pool)
+{
+	SCST_KUNIT_REDIRECT(dev_user_flush_pool, pool);
+	sgv_pool_flush(pool);
+}
+
 static int dev_user_unjam_dev(struct scst_user_dev *dev)
 	__releases(&dev->udev_cmd_threads.cmd_list_lock)
 	__acquires(&dev->udev_cmd_threads.cmd_list_lock)
@@ -2586,8 +2597,8 @@ static int dev_user_unjam_dev(struct scst_user_dev *dev)
 
 	TRACE_MGMT_DBG("Unjamming dev %p", dev);
 
-	sgv_pool_flush(dev->pool);
-	sgv_pool_flush(dev->pool_clust);
+	dev_user_flush_pool(dev->pool);
+	dev_user_flush_pool(dev->pool_clust);
 
 	spin_lock_irq(&dev->udev_cmd_threads.cmd_list_lock);
 
@@ -2631,8 +2642,8 @@ repeat:
 	 * indefinitely. A checked-out object cannot be flushed until its user
 	 * returns it; cleanup must allow that user to make progress.
 	 */
-	sgv_pool_flush(dev->pool);
-	sgv_pool_flush(dev->pool_clust);
+	dev_user_flush_pool(dev->pool);
+	dev_user_flush_pool(dev->pool_clust);
 
 	TRACE_EXIT_RES(res);
 	return res;
@@ -3758,7 +3769,8 @@ static void dev_user_check_lost_ucmds(struct scst_user_dev *dev)
 }
 #endif
 
-static int dev_user_process_cleanup(struct scst_user_dev *dev)
+SCST_KUNIT_VISIBLE
+int dev_user_process_cleanup(struct scst_user_dev *dev)
 {
 	struct scst_user_cmd *ucmd;
 	int rc = 0, res = 1;

scst/src/dev_handlers/scst_user_test.h

@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef SCST_USER_TEST_H
+#define SCST_USER_TEST_H
+
+#include "../scst_test.h"
+
+#ifdef SCST_KUNIT_ENABLED
+
+void dev_user_unjam_cmd(struct scst_user_cmd *ucmd, int busy, unsigned long *flags);
+void dev_user_flush_pool(struct sgv_pool *pool);
+int dev_user_process_cleanup(struct scst_user_dev *dev);
+
+#endif /* SCST_KUNIT_ENABLED */
+#endif /* SCST_USER_TEST_H */

scst/src/kunit/modules.mk

@@ -4,6 +4,7 @@ SCST_KUNIT_COMPONENTS := scst
 
 SCST_KUNIT_MODULES_scst := \
 	scst/src/scst.ko \
-	scst/src/dev_handlers/scst_vdisk.ko
+	scst/src/dev_handlers/scst_vdisk.ko \
+	scst/src/dev_handlers/scst_user.ko
 
 SCST_KUNIT_MODULES = $(foreach component,$(SCST_KUNIT_COMPONENTS),$(SCST_KUNIT_MODULES_$(component)))

scst/src/kunit/objects.mk

@@ -14,6 +14,9 @@ scst-kunit-scst := \
 	kunit/order/suites/scst_kunit_order.o \
 	kunit/unmap/suites/scst_kunit_unmap.o
 
+scst-kunit-scst_user := \
+	kunit/user/suites/scst_user_cleanup_test.o
+
 scst-kunit-scst_vdisk := \
 	kunit/vdisk/scst_vdisk_fixture.o \
 	kunit/vdisk/suites/scst_vdisk_capacity_test.o \