From patchwork Thu Mar 7 20:30:06 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bart Van Assche X-Patchwork-Id: 779622 Received: from 008.lax.mailroute.net (008.lax.mailroute.net [199.89.1.11]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8419E23769 for ; Thu, 7 Mar 2024 20:30:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=199.89.1.11 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709843439; cv=none; b=uKZAXwP2Pq9SZrUcRfPoaEBw/xG3mObOeMoRl6W1QeHKOgXWuV5gVI9P/9Bjb9VW41RdJ06ACpD4oN4XFS8PmB9fak5lJyAl0yUm7OynhOumzGrUzbUb2JcKoRbbwH/nyoSC96/8qzFc99YZ1h1/CXq4zTC6AmMr9u6rK7PaC1M= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709843439; c=relaxed/simple; bh=69EgRrLizmuwufflrgKN8hzoWh/Pqn4roEz2bfTSW3M=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=ZlrAIqkhZvK3zHptcc59CjJHncYN9CYO3Bes9P+DoBjmiEwUlAgksHZP4Z3xPloMC9ZRmcYODEnXg+ERlzjgke6IMhgCnfO/wg42Mgx4vPgneaT2Zw89PhgMu18en+PlbIT3SNm68Rp6feJo6Z74CzBJwiSgRDWYRlvEDQsiY3Q= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=acm.org; spf=pass smtp.mailfrom=acm.org; dkim=pass (2048-bit key) header.d=acm.org header.i=@acm.org header.b=zk4Z+SgL; arc=none smtp.client-ip=199.89.1.11 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=acm.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=acm.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=acm.org header.i=@acm.org header.b="zk4Z+SgL" Received: from localhost (localhost [127.0.0.1]) by 008.lax.mailroute.net (Postfix) with ESMTP id 4TrLXX6d7Wz6Cl44w; Thu, 7 Mar 2024 20:30:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=acm.org; h= content-transfer-encoding:mime-version:references:in-reply-to :x-mailer:message-id:date:date:subject:subject:from:from :received:received; s=mr01; t=1709843427; x=1712435428; bh=j42KF b2bzHN8DAUn6WygrBcH8l3UFtNmhf0vJk0cVrc=; b=zk4Z+SgL/Tis1V4UlaS10 bH5fn73lYX2Nrp41Ylersgib7f5dpbEWysUbFqHa9lnOOI6R0pjgN4ZOBkT9VUt9 hIyDRAEDyh6nTFYFe00yFjtIoD89+CdEeBWVq4ROTCCP0kiHZ2xCKEzY0mu7B3ba woL1rC6E62S0kJGrA01btQtDAfsAVuJkVSoox2xNGVzxwPvdebFZKWU9I+JQPYkQ KLTw45RZdWAB/SHthv7F1VMNipRw1YbRblyWwKEXyv8BppcEq2wKZDXjdy7uSZS5 mZOtCiZaqb0CGgW2SmScuACWRYvX6lElFyxnM8JX0xTf5dsBeqCfJAT6FXCMBxcL Q== X-Virus-Scanned: by MailRoute Received: from 008.lax.mailroute.net ([127.0.0.1]) by localhost (008.lax [127.0.0.1]) (mroute_mailscanner, port 10029) with LMTP id VqfpkgHU0-QN; Thu, 7 Mar 2024 20:30:27 +0000 (UTC) Received: from bvanassche-linux.mtv.corp.google.com (unknown [104.132.1.77]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: bvanassche@acm.org) by 008.lax.mailroute.net (Postfix) with ESMTPSA id 4TrLXL2Zvjz6Cl2L2; Thu, 7 Mar 2024 20:30:26 +0000 (UTC) From: Bart Van Assche To: "Martin K . Petersen" Cc: linux-scsi@vger.kernel.org, Bart Van Assche , Douglas Gilbert , John Garry , "James E.J. Bottomley" Subject: [PATCH v3 3/4] scsi: scsi_debug: Simplify command handling Date: Thu, 7 Mar 2024 12:30:06 -0800 Message-ID: <20240307203015.870254-4-bvanassche@acm.org> X-Mailer: git-send-email 2.44.0.278.ge034bb2e1d-goog In-Reply-To: <20240307203015.870254-1-bvanassche@acm.org> References: <20240307203015.870254-1-bvanassche@acm.org> Precedence: bulk X-Mailing-List: linux-scsi@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 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"; v6.4). Cc: Douglas Gilbert Cc: John Garry Signed-off-by: Bart Van Assche --- drivers/scsi/scsi_debug.c | 140 +++++++------------------------------- 1 file changed, 26 insertions(+), 114 deletions(-) diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 7a0b7402b715..1597156cb573 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -253,11 +253,6 @@ static const char *sdebug_version_date = "20210520"; #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, @@ -398,16 +393,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 */ @@ -559,8 +547,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 @@ -5332,10 +5318,9 @@ 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) { @@ -5346,7 +5331,7 @@ 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); @@ -5354,19 +5339,16 @@ static void sdebug_q_cmd_complete(struct sdebug_defer *sd_dp) 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. */ @@ -5648,50 +5630,32 @@ static void scsi_debug_slave_destroy(struct scsi_device *sdp) sdp->hostdata = NULL; } -/* Returns true if the queued command memory should 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); switch (res) { - case 0: /* Not active, it must have already run */ case -1: /* -1 It's executing the CB */ return false; + case 0: /* Not active, it must have already run */ case 1: /* Was active, we've now cancelled */ default: return true; } } else if (defer_t == SDEB_DEFER_WQ) { - /* The caller must free qcmd if cancellation succeeds. */ - return cancel_work(&sd_dp->ew.work); - } else if (defer_t == SDEB_DEFER_POLL) { - return true; + /* Retry aborting later if the completion handler is running. */ + return cancel_work(&sd_dp->ew.work) || + !work_pending(&sd_dp->ew.work); } - 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; } @@ -5706,6 +5670,8 @@ static bool scsi_debug_abort_cmnd(struct scsi_cmnd *cmnd) spin_lock_irqsave(&sdsc->lock, flags); res = scsi_debug_stop_cmnd(cmnd); + if (res) + WRITE_ONCE(sdsc->sd_dp.defer_t, SDEB_DEFER_NONE); spin_unlock_irqrestore(&sdsc->lock, flags); return res; @@ -5783,7 +5749,7 @@ static int scsi_debug_abort(struct scsi_cmnd *SCpnt) return FAILED; } - return SUCCESS; + return ok ? SUCCESS : FAILED; } static bool scsi_debug_stop_all_queued_iter(struct request *rq, void *data) @@ -6056,33 +6022,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 @@ -6099,7 +6038,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; @@ -6131,12 +6069,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) ns_from_boot = ktime_get_boottime_ns(); @@ -6184,7 +6117,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; } @@ -6197,13 +6129,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); /* @@ -6227,13 +6157,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); @@ -7587,12 +7515,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__); @@ -7619,8 +7541,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: @@ -7636,7 +7556,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); @@ -8018,7 +7937,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; @@ -8034,13 +7952,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; @@ -8050,8 +7962,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) { @@ -8060,8 +7970,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; @@ -8376,8 +8284,12 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost, 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; }