From 9b14a97cd47155f0b25d335f3eab569a9e72bdb6 Mon Sep 17 00:00:00 2001 From: Gleb Chesnokov Date: Tue, 20 Oct 2026 16:09:07 +0300 Subject: [PATCH] scst: Use kref for command ownership Coccinelle api/atomic_as_refcounter.cocci reports: scst/src/scst_priv.h:708:5-24: WARNING: atomic_dec_and_test variation before object free at line 709. cmd_ref counts ownership of struct scst_cmd, and its final put invokes scst_free_cmd. Replace the generic atomic counter with kref, using scst_free_cmd as the release callback. Update initialization, get/put, diagnostic reads and the vdisk KUnit fixtures together. Keep the existing barrier after reference acquisition and the deferred out-of-sequence destruction path. Completion and work counters retain their atomic types; only the command lifetime counter changes. Use kref to retain compatibility with the supported old kernels without introducing a separate refcount_t backport. This addresses the ownership counter diagnostic without changing the Coccinelle rule. --- scst/include/scst.h | 3 ++- .../dev_handlers/kunit/vdisk/scst_vdisk_fixture.c | 4 ++-- .../kunit/vdisk/suites/scst_vdisk_protocol_test.c | 2 +- scst/src/scst_lib.c | 9 +++++---- scst/src/scst_priv.h | 15 +++++++-------- scst/src/scst_targ.c | 4 ++-- 6 files changed, 19 insertions(+), 18 deletions(-) diff --git a/scst/include/scst.h b/scst/include/scst.h index e096708f2..6a8a6f1e1 100644 --- a/scst/include/scst.h +++ b/scst/include/scst.h @@ -31,6 +31,7 @@ #define CONFIG_SCST_NO_TOTAL_MEM_CHECKS #include +#include #ifndef INSIDE_KERNEL_TREE #include #endif @@ -2065,7 +2066,7 @@ struct scst_cmd { bool counted; - atomic_t cmd_ref; + struct kref cmd_ref; /* Cmd state, one of SCST_CMD_STATE_* constants */ enum scst_cmd_state state; diff --git a/scst/src/dev_handlers/kunit/vdisk/scst_vdisk_fixture.c b/scst/src/dev_handlers/kunit/vdisk/scst_vdisk_fixture.c index 987db5ef5..d949f4360 100644 --- a/scst/src/dev_handlers/kunit/vdisk/scst_vdisk_fixture.c +++ b/scst/src/dev_handlers/kunit/vdisk/scst_vdisk_fixture.c @@ -66,7 +66,7 @@ static int vdisk_test_fsync(struct file *file, loff_t start, loff_t end, int dat .file = file, .start = start, .end = end, .io_count = f->io_count, .written_len = f->written_len, .completions = f->completions, - .cmd_refs = atomic_read(&f->cmd.cmd_ref), + .cmd_refs = kref_read(&f->cmd.cmd_ref), }; return backend->sync_result; } @@ -146,7 +146,7 @@ int vdisk_test_init(struct kunit *test) vdisk_test_file_init(&f->dif, f); f->vdev.fd = &f->data.file; f->cmd.cdb = f->cdb; - atomic_set(&f->cmd.cmd_ref, 1); + kref_init(&f->cmd.cmd_ref); f->cmd.dev = &f->dev; f->cmd.sess = &f->sess; f->cmd.cmd_gfp_mask = GFP_KERNEL; diff --git a/scst/src/dev_handlers/kunit/vdisk/suites/scst_vdisk_protocol_test.c b/scst/src/dev_handlers/kunit/vdisk/suites/scst_vdisk_protocol_test.c index 17918286a..d502c3c0e 100644 --- a/scst/src/dev_handlers/kunit/vdisk/suites/scst_vdisk_protocol_test.c +++ b/scst/src/dev_handlers/kunit/vdisk/suites/scst_vdisk_protocol_test.c @@ -170,7 +170,7 @@ static void vdisk_cache_test(struct kunit *test) KUNIT_EXPECT_EQ(test, f->flushes[0].completions, p->immed ? 1U : 0U); KUNIT_EXPECT_EQ(test, f->flushes[0].cmd_refs, p->immed ? 2 : 1); } - KUNIT_EXPECT_EQ(test, atomic_read(&f->cmd.cmd_ref), 1); + KUNIT_EXPECT_EQ(test, kref_read(&f->cmd.cmd_ref), 1); KUNIT_EXPECT_EQ(test, f->completions, 1U); KUNIT_EXPECT_TRUE(test, f->completed_before_callback); KUNIT_EXPECT_EQ(test, f->completion_state, SCST_CMD_STATE_DEFAULT); diff --git a/scst/src/scst_lib.c b/scst/src/scst_lib.c index a0a4cdfc0..e661e80bc 100644 --- a/scst/src/scst_lib.c +++ b/scst/src/scst_lib.c @@ -7623,7 +7623,7 @@ int scst_pre_init_cmd(struct scst_cmd *cmd, const uint8_t *cdb, scst_set_cmd_state(cmd, SCST_CMD_STATE_INIT_WAIT); cmd->start_time = jiffies; - atomic_set(&cmd->cmd_ref, 1); + kref_init(&cmd->cmd_ref); cmd->cmd_threads = &scst_main_cmd_threads; cmd->cmd_gfp_mask = GFP_KERNEL; INIT_LIST_HEAD(&cmd->mgmt_cmd_list); @@ -7788,8 +7788,9 @@ out: EXPORT_SYMBOL_GPL(scst_stpg_del_unblock_next); /* No locks supposed to be held */ -void scst_free_cmd(struct scst_cmd *cmd) +void scst_free_cmd(struct kref *ref) { + struct scst_cmd *cmd = container_of(ref, struct scst_cmd, cmd_ref); int destroy = 1; TRACE_ENTRY(); @@ -14248,8 +14249,8 @@ void scst_xmit_process_aborted_cmd(struct scst_cmd *cmd) { TRACE_ENTRY(); - TRACE_MGMT_DBG("Aborted cmd %p done (cmd_ref %d)", cmd, - atomic_read(&cmd->cmd_ref)); + TRACE_MGMT_DBG("Aborted cmd %p done (cmd_ref %u)", cmd, + kref_read(&cmd->cmd_ref)); scst_done_cmd_mgmt(cmd); diff --git a/scst/src/scst_priv.h b/scst/src/scst_priv.h index 0fac9d21b..5fa6aaca5 100644 --- a/scst/src/scst_priv.h +++ b/scst/src/scst_priv.h @@ -689,24 +689,23 @@ static inline void scst_sess_put(struct scst_session *sess) struct scst_cmd *scst_alloc_cmd(const uint8_t *cdb, unsigned int cdb_len, gfp_t gfp_mask); int scst_pre_init_cmd(struct scst_cmd *cmd, const uint8_t *cdb, unsigned int cdb_len, gfp_t gfp_mask); -void scst_free_cmd(struct scst_cmd *cmd); +void scst_free_cmd(struct kref *ref); static inline void __scst_cmd_get(struct scst_cmd *cmd) { - atomic_inc(&cmd->cmd_ref); + kref_get(&cmd->cmd_ref); smp_mb__after_atomic_inc(); - TRACE_DBG("Incrementing cmd %p ref (new value %d)", - cmd, atomic_read(&cmd->cmd_ref)); + TRACE_DBG("Incrementing cmd %p ref (new value %u)", + cmd, kref_read(&cmd->cmd_ref)); } static inline void __scst_cmd_put(struct scst_cmd *cmd) { - TRACE_DBG("Decrementing cmd %p ref (new value %d)", - cmd, atomic_read(&cmd->cmd_ref) - 1); + TRACE_DBG("Decrementing cmd %p ref (new value %u)", + cmd, kref_read(&cmd->cmd_ref) - 1); - if (atomic_dec_and_test(&cmd->cmd_ref)) - scst_free_cmd(cmd); + kref_put(&cmd->cmd_ref, scst_free_cmd); } void scst_throttle_cmd(struct scst_cmd *cmd); diff --git a/scst/src/scst_targ.c b/scst/src/scst_targ.c index 0b2fbdf67..f90de3609 100644 --- a/scst/src/scst_targ.c +++ b/scst/src/scst_targ.c @@ -5074,8 +5074,8 @@ void scst_finish_cmd_mgmt(struct scst_cmd *cmd) TRACE_ENTRY(); - TRACE(TRACE_MGMT, "Aborted cmd %p finished (tag %llu, ref %d)", - cmd, (unsigned long long)cmd->tag, atomic_read(&cmd->cmd_ref)); + TRACE(TRACE_MGMT, "Aborted cmd %p finished (tag %llu, ref %u)", + cmd, (unsigned long long)cmd->tag, kref_read(&cmd->cmd_ref)); spin_lock_irqsave(&scst_mcmd_lock, flags);