From d41d195c4ffded7d11c91e634c89d2425ef7f8df Mon Sep 17 00:00:00 2001 From: Gleb Chesnokov Date: Tue, 13 Oct 2026 11:55:59 +0300 Subject: [PATCH] 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 | 1 + scripts/generate-kernel-patch | 3 +- scripts/kunit/test_make.py | 1 + .../user/suites/scst_user_cleanup_test.c | 222 ++++++++++++++++++ scst/src/dev_handlers/scst_user_main.c | 28 ++- scst/src/dev_handlers/scst_user_test.h | 14 ++ scst/src/kunit/modules.mk | 3 +- scst/src/kunit/objects.mk | 3 + 8 files changed, 265 insertions(+), 10 deletions(-) create mode 100644 scst/src/dev_handlers/kunit/user/suites/scst_user_cleanup_test.c create mode 100644 scst/src/dev_handlers/scst_user_test.h diff --git a/doc/KUNIT.md b/doc/KUNIT.md index 16ade45a5..a2aaee159 100644 --- a/doc/KUNIT.md +++ b/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; diff --git a/scripts/generate-kernel-patch b/scripts/generate-kernel-patch index 636ed0e3d..812df1879 100755 --- a/scripts/generate-kernel-patch +++ b/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" diff --git a/scripts/kunit/test_make.py b/scripts/kunit/test_make.py index 9a36c2b95..79566601a 100644 --- a/scripts/kunit/test_make.py +++ b/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'), } diff --git a/scst/src/dev_handlers/kunit/user/suites/scst_user_cleanup_test.c b/scst/src/dev_handlers/kunit/user/suites/scst_user_cleanup_test.c new file mode 100644 index 000000000..b9e6f4d3b --- /dev/null +++ b/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 +#include +#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, +); diff --git a/scst/src/dev_handlers/scst_user_main.c b/scst/src/dev_handlers/scst_user_main.c index aaf817680..73c488e4f 100644 --- a/scst/src/dev_handlers/scst_user_main.c +++ b/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; diff --git a/scst/src/dev_handlers/scst_user_test.h b/scst/src/dev_handlers/scst_user_test.h new file mode 100644 index 000000000..22b79aed0 --- /dev/null +++ b/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 */ diff --git a/scst/src/kunit/modules.mk b/scst/src/kunit/modules.mk index cb0301456..ac2c12634 100644 --- a/scst/src/kunit/modules.mk +++ b/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))) diff --git a/scst/src/kunit/objects.mk b/scst/src/kunit/objects.mk index 5646c7ec3..4f9e002ed 100644 --- a/scst/src/kunit/objects.mk +++ b/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 \