kunit: Check queued VDISK sync ownership and failure paths

The issue #394 fix moves sync into sysfs work. A timed-out store may
return while that work still owns a device reference, so the handler can
disappear before backend lookup.

Exercise the real store and callback with a task-local executor, real
work allocation and device krefs. Cover allocation, suspend and backend
failures, successful and delayed completion, and handler removal
before delayed work runs. Check reference balance, suspend/resume
pairing and lockdep assertions at backend entry.

Use native task-scoped stubs at the core queue and suspension
boundaries; normal builds compile them out. Acquire scst_mutex directly
through the production callback. No synthetic device enters a real SCST
queue, and a delayed kobject release is joined before its independent
allocation and module reference are released; incomplete release is
retained. These deterministic ordering tests do not simulate concurrent
teardown or power loss.

Link: https://github.com/SCST-project/scst/issues/394
This commit is contained in:
Gleb Chesnokov
2026-10-14 17:39:14 +03:00
parent 8c4e44fca9
commit edb1a66d64
6 changed files with 256 additions and 3 deletions
@@ -0,0 +1,237 @@
// SPDX-License-Identifier: GPL-2.0-only
#include "../scst_vdisk_fixture.h"
struct sync_device {
struct scst_device dev;
struct completion released;
};
struct sync_fixture {
struct scst_device *dev;
struct scst_sysfs_work_item *pending;
int alloc_error;
int suspend_error;
int backend_result;
unsigned int queues;
unsigned int suspends;
unsigned int resumes;
unsigned int backends;
bool delayed;
};
/* Release can run on another task with CONFIG_DEBUG_KOBJECT_RELEASE. */
static void sync_device_release(struct kobject *kobj)
{
struct scst_device *dev = container_of(kobj, struct scst_device, dev_kobj);
struct sync_device *d = container_of(dev, struct sync_device, dev);
complete(&d->released);
}
static const struct kobj_type sync_device_type = {
.release = sync_device_release,
};
static int sync_suspend(unsigned long timeout)
{
struct kunit *test = kunit_get_current_test();
struct sync_fixture *f = test->priv;
KUNIT_EXPECT_EQ(test, timeout, (unsigned long)SCST_SUSPEND_TIMEOUT_USER);
f->suspends++;
return f->suspend_error;
}
static void sync_resume(void)
{
struct kunit *test = kunit_get_current_test();
struct sync_fixture *f = test->priv;
bool unlocked = mutex_trylock(&scst_mutex);
if (unlocked)
mutex_unlock(&scst_mutex);
KUNIT_EXPECT_TRUE(test, unlocked);
f->resumes++;
}
static int sync_backend(struct scst_device *dev)
{
struct kunit *test = kunit_get_current_test();
struct sync_fixture *f = test->priv;
KUNIT_EXPECT_PTR_EQ(test, dev, f->dev);
KUNIT_EXPECT_EQ(test, f->suspends, 1U);
KUNIT_EXPECT_EQ(test, f->resumes, 0U);
KUNIT_EXPECT_TRUE(test, mutex_is_locked(&scst_mutex));
lockdep_assert_held(&scst_mutex);
lockdep_assert_alua_lock_held();
KUNIT_EXPECT_EQ(test, kref_read(&dev->dev_kobj.kref), 2U);
f->backends++;
/* Backend dispatch has a separate suite with real fake-file operations. */
return f->backend_result;
}
static int sync_alloc(int (*fn)(struct scst_sysfs_work_item *), bool read_only,
struct scst_sysfs_work_item **work)
{
struct kunit *test = kunit_get_current_test();
struct sync_fixture *f = test->priv;
int ret;
KUNIT_EXPECT_FALSE(test, read_only);
if (f->alloc_error)
return f->alloc_error;
kunit_deactivate_static_stub(test, scst_alloc_sysfs_work);
ret = scst_alloc_sysfs_work(fn, read_only, work);
kunit_activate_static_stub(test, scst_alloc_sysfs_work, sync_alloc);
return ret;
}
static int sync_finish(struct sync_fixture *f)
{
struct scst_sysfs_work_item *work = f->pending;
int ret;
f->pending = NULL;
ret = work->sysfs_work_fn(work);
scst_sysfs_work_put(work);
return ret;
}
static int sync_queue(struct scst_sysfs_work_item *work)
{
struct kunit *test = kunit_get_current_test();
struct sync_fixture *f = test->priv;
KUNIT_EXPECT_PTR_EQ(test, work->dev, f->dev);
KUNIT_EXPECT_EQ(test, kref_read(&f->dev->dev_kobj.kref), 2U);
f->queues++;
f->pending = work;
/* Execute in this task so native static stubs never escape their owner. */
return f->delayed ? -EAGAIN : sync_finish(f);
}
static int sync_init(struct kunit *test)
{
struct sync_fixture *f = kunit_kzalloc(test, sizeof(*f), GFP_KERNEL);
struct sync_device *d;
if (!f)
return -ENOMEM;
test->priv = f;
d = kzalloc_obj(*d);
if (!d)
return -ENOMEM;
f->dev = &d->dev;
init_completion(&d->released);
__module_get(THIS_MODULE);
kobject_init(&f->dev->dev_kobj, &sync_device_type);
f->dev->dh_priv = f;
kunit_activate_static_stub(test, scst_suspend_activity, sync_suspend);
kunit_activate_static_stub(test, scst_resume_activity, sync_resume);
kunit_activate_static_stub(test, vdisk_flush_backend, sync_backend);
kunit_activate_static_stub(test, scst_alloc_sysfs_work, sync_alloc);
kunit_activate_static_stub(test, scst_sysfs_queue_wait_work, sync_queue);
return 0;
}
static void sync_exit(struct kunit *test)
{
struct sync_fixture *f = test->priv;
struct sync_device *d;
if (!f || !f->dev)
return;
if (f->pending)
sync_finish(f);
f->dev->dh_priv = NULL;
if (kref_read(&f->dev->dev_kobj.kref) != 1) {
scst_kunit_retain(test, "sync device reference leaked");
return;
}
d = container_of(f->dev, struct sync_device, dev);
kobject_put(&f->dev->dev_kobj);
if (!wait_for_completion_timeout(&d->released, 10 * HZ)) {
scst_kunit_retain(test, "sync device release did not finish");
return;
}
#ifdef CONFIG_DEBUG_KOBJECT_RELEASE
/* Release has signalled; join its return before freeing the work storage. */
flush_delayed_work(&f->dev->dev_kobj.release);
#endif
kfree(d);
module_put(THIS_MODULE);
}
struct sync_case {
const char *name;
int alloc_error;
int suspend_error;
int backend_error;
bool delayed;
bool detached;
};
static const struct sync_case sync_cases[] = {
{ .name = "complete" },
{ .name = "allocation_failed", .alloc_error = -ENOMEM },
{ .name = "suspend_failed", .suspend_error = -EBUSY },
{ .name = "backend_eio", .backend_error = -EIO },
{ .name = "backend_enomem", .backend_error = -ENOMEM },
{ .name = "delayed_completion", .delayed = true },
{ .name = "delayed_error", .delayed = true, .backend_error = -EIO },
{ .name = "handler_removed_before_execution", .delayed = true, .detached = true },
};
static void sync_desc(const struct sync_case *p, char *desc)
{
snprintf(desc, KUNIT_PARAM_DESC_SIZE, "%s", p->name);
}
KUNIT_ARRAY_PARAM(sync, sync_cases, sync_desc);
static void sync_test(struct kunit *test)
{
const struct sync_case *p = test->param_value;
struct sync_fixture *f = test->priv;
int error = p->alloc_error ?: p->suspend_error ?: p->backend_error;
ssize_t ret;
f->alloc_error = p->alloc_error;
f->suspend_error = p->suspend_error;
f->backend_result = p->backend_error;
f->delayed = p->delayed;
ret = vdisk_sysfs_sync_store(&f->dev->dev_kobj, NULL, "1\n", 2);
KUNIT_EXPECT_EQ(test, ret, p->delayed ? (ssize_t)-EAGAIN : error ?: (ssize_t)2);
if (p->delayed) {
KUNIT_EXPECT_EQ(test, f->suspends, 0U);
KUNIT_EXPECT_EQ(test, kref_read(&f->dev->dev_kobj.kref), 2U);
if (p->detached) {
/* Model handler teardown while a timed-out store still owns work. */
f->dev->dh_priv = NULL;
kunit_deactivate_static_stub(test, vdisk_flush_backend);
error = -ENODEV;
}
KUNIT_ASSERT_NOT_NULL(test, f->pending);
KUNIT_EXPECT_EQ(test, sync_finish(f), error);
}
KUNIT_EXPECT_EQ(test, f->queues, p->alloc_error ? 0U : 1U);
KUNIT_EXPECT_EQ(test, f->suspends, p->alloc_error ? 0U : 1U);
KUNIT_EXPECT_EQ(test, f->resumes, p->alloc_error || p->suspend_error ? 0U : 1U);
KUNIT_EXPECT_EQ(test, f->backends,
p->alloc_error || p->suspend_error || p->detached ?
0U : 1U);
KUNIT_EXPECT_EQ(test, kref_read(&f->dev->dev_kobj.kref), 1U);
KUNIT_EXPECT_PTR_EQ(test, f->pending, NULL);
}
static struct kunit_case sync_test_cases[] = {
KUNIT_CASE_PARAM(sync_test, sync_gen_params),
{}
};
scst_kunit_suite(vdisk_sysfs_suite, "VDISK", "-sysfs",
.init = sync_init,
.exit = sync_exit,
.test_cases = sync_test_cases,
);
+5 -2
View File
@@ -7495,6 +7495,8 @@ int vdisk_flush_backend(struct scst_device *dev)
{
struct scst_vdisk_dev *virt_dev = dev->dh_priv;
SCST_KUNIT_REDIRECT(vdisk_flush_backend, dev);
/* Queued work can outlive removal of the handler's sysfs files. */
if (!virt_dev)
return -ENODEV;
@@ -7536,8 +7538,9 @@ out:
return res;
}
static ssize_t vdisk_sysfs_sync_store(struct kobject *kobj, struct kobj_attribute *attr,
const char *buf, size_t count)
SCST_KUNIT_VISIBLE
ssize_t vdisk_sysfs_sync_store(struct kobject *kobj, struct kobj_attribute *attr,
const char *buf, size_t count)
{
struct scst_device *dev = container_of(kobj, struct scst_device, dev_kobj);
struct scst_sysfs_work_item *work;
+2
View File
@@ -20,6 +20,8 @@ enum compl_status_e fileio_exec_write(struct vdisk_cmd_params *p);
enum compl_status_e fileio_exec_read(struct vdisk_cmd_params *p);
bool vdisk_parse_offset(struct vdisk_cmd_params *p, struct scst_cmd *cmd);
enum compl_status_e vdisk_synchronize_cache(struct vdisk_cmd_params *p);
ssize_t vdisk_sysfs_sync_store(struct kobject *kobj, struct kobj_attribute *attr,
const char *buf, size_t count);
int vdisk_flush_backend(struct scst_device *dev);
#endif /* SCST_KUNIT_ENABLED */
+2 -1
View File
@@ -25,4 +25,5 @@ scst-kunit-scst_vdisk := \
kunit/vdisk/suites/scst_vdisk_capacity_test.o \
kunit/vdisk/suites/scst_vdisk_io_test.o \
kunit/vdisk/suites/scst_vdisk_protocol_test.o \
kunit/vdisk/suites/scst_vdisk_sync_test.o
kunit/vdisk/suites/scst_vdisk_sync_test.o \
kunit/vdisk/suites/scst_vdisk_sysfs_test.o
+5
View File
@@ -39,6 +39,7 @@
#include "scst_priv.h"
#include "scst_mem.h"
#include "scst_pres.h"
#include "scst_test.h"
#undef DEFAULT_SYMBOL_NAMESPACE
#define DEFAULT_SYMBOL_NAMESPACE SCST_NAMESPACE
@@ -838,6 +839,8 @@ int scst_suspend_activity(unsigned long timeout)
bool rep = false;
unsigned long cur_time = jiffies, wait_time;
SCST_KUNIT_REDIRECT(scst_suspend_activity, timeout);
TRACE_ENTRY();
rwlock_acquire_read(&scst_suspend_dep_map, 0, 0, _RET_IP_);
@@ -992,6 +995,8 @@ out:
*/
void scst_resume_activity(void)
{
SCST_KUNIT_REDIRECT(scst_resume_activity);
TRACE_ENTRY();
rwlock_release(&scst_suspend_dep_map, _RET_IP_);
+5
View File
@@ -33,6 +33,7 @@
#include "scst_priv.h"
#include "scst_pres.h"
#include "scst_mem.h"
#include "scst_test.h"
#undef DEFAULT_SYMBOL_NAMESPACE
#define DEFAULT_SYMBOL_NAMESPACE SCST_NAMESPACE
@@ -336,6 +337,8 @@ int scst_alloc_sysfs_work(int (*sysfs_work_fn)(struct scst_sysfs_work_item *),
int res = 0;
struct scst_sysfs_work_item *work;
SCST_KUNIT_REDIRECT(scst_alloc_sysfs_work, sysfs_work_fn, read_only_action, res_work);
TRACE_ENTRY();
if (!sysfs_work_fn) {
@@ -502,6 +505,8 @@ int scst_sysfs_queue_wait_work(struct scst_sysfs_work_item *work)
struct task_struct *t;
static atomic_t uid_thread_name = ATOMIC_INIT(0);
SCST_KUNIT_REDIRECT(scst_sysfs_queue_wait_work, work);
TRACE_ENTRY();
spin_lock(&sysfs_work_lock);