]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
scsi: scsi_debug: Simplify command handling
authorBart Van Assche <bvanassche@acm.org>
Mon, 24 Feb 2025 11:55:16 +0000 (11:55 +0000)
committerMartin K. Petersen <martin.petersen@oracle.com>
Tue, 25 Feb 2025 01:53:07 +0000 (20:53 -0500)
Simplify command handling by moving struct sdebug_defer into the private
SCSI command data instead of allocating it separately. The only functional
change is that aborting a SCSI command now fails and is retried at a later
time if the completion handler can't be cancelled.

See also commit 1107c7b24ee3 ("scsi: scsi_debug: Dynamically allocate
sdebug_queued_cmd").

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: John Garry <john.g.garry@oracle.com>
Link: https://lore.kernel.org/r/20250224115517.495899-4-john.g.garry@oracle.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
drivers/scsi/scsi_debug.c

index 7631c2c465949a7823a83da328bc8e4df3b436c9..c1a217b5aac2682e5953758bb7235a1f3cd5dd7f 100644 (file)
@@ -300,11 +300,6 @@ struct tape_block {
 
 #define SDEB_XA_NOT_IN_USE XA_MARK_1
 
-static struct kmem_cache *queued_cmd_cache;
-
-#define TO_QUEUED_CMD(scmd)  ((void *)(scmd)->host_scribble)
-#define ASSIGN_QUEUED_CMD(scmnd, qc) { (scmnd)->host_scribble = (void *) qc; }
-
 /* Zone types (zbcr05 table 25) */
 enum sdebug_z_type {
        ZBC_ZTYPE_CNV   = 0x1,
@@ -460,17 +455,9 @@ struct sdebug_defer {
        enum sdeb_defer_type defer_t;
 };
 
-
-struct sdebug_queued_cmd {
-       /* corresponding bit set in in_use_bm[] in owning struct sdebug_queue
-        * instance indicates this slot is in use.
-        */
-       struct sdebug_defer sd_dp;
-       struct scsi_cmnd *scmd;
-};
-
 struct sdebug_scsi_cmd {
        spinlock_t   lock;
+       struct sdebug_defer sd_dp;
 };
 
 static atomic_t sdebug_cmnd_count;   /* number of incoming commands */
@@ -636,8 +623,6 @@ static int sdebug_add_store(void);
 static void sdebug_erase_store(int idx, struct sdeb_store_info *sip);
 static void sdebug_erase_all_stores(bool apart_from_first);
 
-static void sdebug_free_queued_cmd(struct sdebug_queued_cmd *sqcp);
-
 /*
  * The following are overflow arrays for cdbs that "hit" the same index in
  * the opcode_info_arr array. The most time sensitive (or commonly used) cdb
@@ -6333,10 +6318,10 @@ static u32 get_tag(struct scsi_cmnd *cmnd)
 /* Queued (deferred) command completions converge here. */
 static void sdebug_q_cmd_complete(struct sdebug_defer *sd_dp)
 {
-       struct sdebug_queued_cmd *sqcp = container_of(sd_dp, struct sdebug_queued_cmd, sd_dp);
+       struct sdebug_scsi_cmd *sdsc = container_of(sd_dp,
+                                       typeof(*sdsc), sd_dp);
+       struct scsi_cmnd *scp = (struct scsi_cmnd *)sdsc - 1;
        unsigned long flags;
-       struct scsi_cmnd *scp = sqcp->scmd;
-       struct sdebug_scsi_cmd *sdsc;
        bool aborted;
 
        if (sdebug_statistics) {
@@ -6347,27 +6332,23 @@ static void sdebug_q_cmd_complete(struct sdebug_defer *sd_dp)
 
        if (!scp) {
                pr_err("scmd=NULL\n");
-               goto out;
+               return;
        }
 
-       sdsc = scsi_cmd_priv(scp);
        spin_lock_irqsave(&sdsc->lock, flags);
        aborted = sd_dp->aborted;
        if (unlikely(aborted))
                sd_dp->aborted = false;
-       ASSIGN_QUEUED_CMD(scp, NULL);
 
        spin_unlock_irqrestore(&sdsc->lock, flags);
 
        if (aborted) {
                pr_info("bypassing scsi_done() due to aborted cmd, kicking-off EH\n");
                blk_abort_request(scsi_cmd_to_rq(scp));
-               goto out;
+               return;
        }
 
        scsi_done(scp); /* callback to mid level */
-out:
-       sdebug_free_queued_cmd(sqcp);
 }
 
 /* When high resolution timer goes off this function is called. */
@@ -6674,10 +6655,15 @@ static void scsi_debug_sdev_destroy(struct scsi_device *sdp)
        sdp->hostdata = NULL;
 }
 
-/* Returns true if we require the queued memory to be freed by the caller. */
-static bool stop_qc_helper(struct sdebug_defer *sd_dp,
-                          enum sdeb_defer_type defer_t)
+/* Returns true if it is safe to complete @cmnd. */
+static bool scsi_debug_stop_cmnd(struct scsi_cmnd *cmnd)
 {
+       struct sdebug_scsi_cmd *sdsc = scsi_cmd_priv(cmnd);
+       struct sdebug_defer *sd_dp = &sdsc->sd_dp;
+       enum sdeb_defer_type defer_t = READ_ONCE(sd_dp->defer_t);
+
+       lockdep_assert_held(&sdsc->lock);
+
        if (defer_t == SDEB_DEFER_HRT) {
                int res = hrtimer_try_to_cancel(&sd_dp->hrt);
 
@@ -6702,28 +6688,6 @@ static bool stop_qc_helper(struct sdebug_defer *sd_dp,
        return false;
 }
 
-
-static bool scsi_debug_stop_cmnd(struct scsi_cmnd *cmnd)
-{
-       enum sdeb_defer_type l_defer_t;
-       struct sdebug_defer *sd_dp;
-       struct sdebug_scsi_cmd *sdsc = scsi_cmd_priv(cmnd);
-       struct sdebug_queued_cmd *sqcp = TO_QUEUED_CMD(cmnd);
-
-       lockdep_assert_held(&sdsc->lock);
-
-       if (!sqcp)
-               return false;
-       sd_dp = &sqcp->sd_dp;
-       l_defer_t = READ_ONCE(sd_dp->defer_t);
-       ASSIGN_QUEUED_CMD(cmnd, NULL);
-
-       if (stop_qc_helper(sd_dp, l_defer_t))
-               sdebug_free_queued_cmd(sqcp);
-
-       return true;
-}
-
 /*
  * Called from scsi_debug_abort() only, which is for timed-out cmd.
  */
@@ -7106,33 +7070,6 @@ static bool inject_on_this_cmd(void)
 
 #define INCLUSIVE_TIMING_MAX_NS 1000000                /* 1 millisecond */
 
-
-void sdebug_free_queued_cmd(struct sdebug_queued_cmd *sqcp)
-{
-       if (sqcp)
-               kmem_cache_free(queued_cmd_cache, sqcp);
-}
-
-static struct sdebug_queued_cmd *sdebug_alloc_queued_cmd(struct scsi_cmnd *scmd)
-{
-       struct sdebug_queued_cmd *sqcp;
-       struct sdebug_defer *sd_dp;
-
-       sqcp = kmem_cache_zalloc(queued_cmd_cache, GFP_ATOMIC);
-       if (!sqcp)
-               return NULL;
-
-       sd_dp = &sqcp->sd_dp;
-
-       hrtimer_init(&sd_dp->hrt, CLOCK_MONOTONIC, HRTIMER_MODE_REL_PINNED);
-       sd_dp->hrt.function = sdebug_q_cmd_hrt_complete;
-       INIT_WORK(&sd_dp->ew.work, sdebug_q_cmd_wq_complete);
-
-       sqcp->scmd = scmd;
-
-       return sqcp;
-}
-
 /* Complete the processing of the thread that queued a SCSI command to this
  * driver. It either completes the command by calling cmnd_done() or
  * schedules a hr timer or work queue then returns 0. Returns
@@ -7149,7 +7086,6 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
        struct sdebug_scsi_cmd *sdsc = scsi_cmd_priv(cmnd);
        unsigned long flags;
        u64 ns_from_boot = 0;
-       struct sdebug_queued_cmd *sqcp;
        struct scsi_device *sdp;
        struct sdebug_defer *sd_dp;
 
@@ -7181,12 +7117,7 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
                }
        }
 
-       sqcp = sdebug_alloc_queued_cmd(cmnd);
-       if (!sqcp) {
-               pr_err("%s no alloc\n", __func__);
-               return SCSI_MLQUEUE_HOST_BUSY;
-       }
-       sd_dp = &sqcp->sd_dp;
+       sd_dp = &sdsc->sd_dp;
 
        if (polled || (ndelay > 0 && ndelay < INCLUSIVE_TIMING_MAX_NS))
                ns_from_boot = ktime_get_boottime_ns();
@@ -7234,7 +7165,6 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
 
                                if (kt <= d) {  /* elapsed duration >= kt */
                                        /* call scsi_done() from this thread */
-                                       sdebug_free_queued_cmd(sqcp);
                                        scsi_done(cmnd);
                                        return 0;
                                }
@@ -7247,13 +7177,11 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
                if (polled) {
                        spin_lock_irqsave(&sdsc->lock, flags);
                        sd_dp->cmpl_ts = ktime_add(ns_to_ktime(ns_from_boot), kt);
-                       ASSIGN_QUEUED_CMD(cmnd, sqcp);
                        WRITE_ONCE(sd_dp->defer_t, SDEB_DEFER_POLL);
                        spin_unlock_irqrestore(&sdsc->lock, flags);
                } else {
                        /* schedule the invocation of scsi_done() for a later time */
                        spin_lock_irqsave(&sdsc->lock, flags);
-                       ASSIGN_QUEUED_CMD(cmnd, sqcp);
                        WRITE_ONCE(sd_dp->defer_t, SDEB_DEFER_HRT);
                        hrtimer_start(&sd_dp->hrt, kt, HRTIMER_MODE_REL_PINNED);
                        /*
@@ -7277,13 +7205,11 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
                        sd_dp->issuing_cpu = raw_smp_processor_id();
                if (polled) {
                        spin_lock_irqsave(&sdsc->lock, flags);
-                       ASSIGN_QUEUED_CMD(cmnd, sqcp);
                        sd_dp->cmpl_ts = ns_to_ktime(ns_from_boot);
                        WRITE_ONCE(sd_dp->defer_t, SDEB_DEFER_POLL);
                        spin_unlock_irqrestore(&sdsc->lock, flags);
                } else {
                        spin_lock_irqsave(&sdsc->lock, flags);
-                       ASSIGN_QUEUED_CMD(cmnd, sqcp);
                        WRITE_ONCE(sd_dp->defer_t, SDEB_DEFER_WQ);
                        schedule_work(&sd_dp->ew.work);
                        spin_unlock_irqrestore(&sdsc->lock, flags);
@@ -8650,12 +8576,6 @@ static int __init scsi_debug_init(void)
        hosts_to_add = sdebug_add_host;
        sdebug_add_host = 0;
 
-       queued_cmd_cache = KMEM_CACHE(sdebug_queued_cmd, SLAB_HWCACHE_ALIGN);
-       if (!queued_cmd_cache) {
-               ret = -ENOMEM;
-               goto driver_unreg;
-       }
-
        sdebug_debugfs_root = debugfs_create_dir("scsi_debug", NULL);
        if (IS_ERR_OR_NULL(sdebug_debugfs_root))
                pr_info("%s: failed to create initial debugfs directory\n", __func__);
@@ -8682,8 +8602,6 @@ static int __init scsi_debug_init(void)
 
        return 0;
 
-driver_unreg:
-       driver_unregister(&sdebug_driverfs_driver);
 bus_unreg:
        bus_unregister(&pseudo_lld_bus);
 dev_unreg:
@@ -8699,7 +8617,6 @@ static void __exit scsi_debug_exit(void)
 
        for (; k; k--)
                sdebug_do_remove_host(true);
-       kmem_cache_destroy(queued_cmd_cache);
        driver_unregister(&sdebug_driverfs_driver);
        bus_unregister(&pseudo_lld_bus);
        root_device_unregister(pseudo_primary);
@@ -9083,7 +9000,6 @@ static bool sdebug_blk_mq_poll_iter(struct request *rq, void *opaque)
        struct sdebug_defer *sd_dp;
        u32 unique_tag = blk_mq_unique_tag(rq);
        u16 hwq = blk_mq_unique_tag_to_hwq(unique_tag);
-       struct sdebug_queued_cmd *sqcp;
        unsigned long flags;
        int queue_num = data->queue_num;
        ktime_t time;
@@ -9099,13 +9015,7 @@ static bool sdebug_blk_mq_poll_iter(struct request *rq, void *opaque)
        time = ktime_get_boottime();
 
        spin_lock_irqsave(&sdsc->lock, flags);
-       sqcp = TO_QUEUED_CMD(cmd);
-       if (!sqcp) {
-               spin_unlock_irqrestore(&sdsc->lock, flags);
-               return true;
-       }
-
-       sd_dp = &sqcp->sd_dp;
+       sd_dp = &sdsc->sd_dp;
        if (READ_ONCE(sd_dp->defer_t) != SDEB_DEFER_POLL) {
                spin_unlock_irqrestore(&sdsc->lock, flags);
                return true;
@@ -9115,8 +9025,6 @@ static bool sdebug_blk_mq_poll_iter(struct request *rq, void *opaque)
                spin_unlock_irqrestore(&sdsc->lock, flags);
                return true;
        }
-
-       ASSIGN_QUEUED_CMD(cmd, NULL);
        spin_unlock_irqrestore(&sdsc->lock, flags);
 
        if (sdebug_statistics) {
@@ -9125,8 +9033,6 @@ static bool sdebug_blk_mq_poll_iter(struct request *rq, void *opaque)
                        atomic_inc(&sdebug_miss_cpus);
        }
 
-       sdebug_free_queued_cmd(sqcp);
-
        scsi_done(cmd); /* callback to mid level */
        (*data->num_entries)++;
        return true;
@@ -9441,8 +9347,12 @@ err_out:
 static int sdebug_init_cmd_priv(struct Scsi_Host *shost, struct scsi_cmnd *cmd)
 {
        struct sdebug_scsi_cmd *sdsc = scsi_cmd_priv(cmd);
+       struct sdebug_defer *sd_dp = &sdsc->sd_dp;
 
        spin_lock_init(&sdsc->lock);
+       hrtimer_init(&sd_dp->hrt, CLOCK_MONOTONIC, HRTIMER_MODE_REL_PINNED);
+       sd_dp->hrt.function = sdebug_q_cmd_hrt_complete;
+       INIT_WORK(&sd_dp->ew.work, sdebug_q_cmd_wq_complete);
 
        return 0;
 }