[PATCH v3 0/4] scsi_debug improvements

[PATCH v3 0/4] scsi_debug improvements

[Bart Van Assche]

Hi Martin,

This patch series fixes a bug in the scsi_debug driver, simplifies the
driver and also splits it into two source files. Please consider this patch
series for the next merge window.

Thanks,

Bart.

Changes compared to v2:
 - Converted a single patch into a patch series.
 - Addressed John's comments on the patch that splits the source code into two
   files.

Changes compared to v1:
 - Made the patch description more detailed.

Bart Van Assche (4):
  scsi: scsi_debug: Remove a reference to in_use_bm
  scsi: scsi_debug: Do not sleep in atomic sections
  scsi: scsi_debug: Simplify command handling
  scsi: scsi_debug: Make CRC_T10DIF support optional

 drivers/scsi/Kconfig                          |   3 +-
 drivers/scsi/Makefile                         |   2 +
 drivers/scsi/scsi_debug_dif.c                 | 240 +++++++++++
 drivers/scsi/scsi_debug_dif.h                 |  57 +++
 .../scsi/{scsi_debug.c => scsi_debug_main.c}  | 399 ++----------------
 5 files changed, 333 insertions(+), 368 deletions(-)
 create mode 100644 drivers/scsi/scsi_debug_dif.c
 create mode 100644 drivers/scsi/scsi_debug_dif.h
 rename drivers/scsi/{scsi_debug.c => scsi_debug_main.c} (96%)

[PATCH v3 1/4] scsi: scsi_debug: Remove a reference to in_use_bm

[Bart Van Assche]

Commit f1437cd1e535 ("scsi: scsi_debug: Drop sdebug_queue") removed
the 'in_use_bm' struct member. Hence remove a reference to that struct
member from the procfs host info file.

Cc: Douglas Gilbert <dgilbert@interlog.com>
Cc: John Garry <john.g.garry@oracle.com>
Fixes: f1437cd1e535 ("scsi: scsi_debug: Drop sdebug_queue")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/scsi_debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index acf0592d63da..36368c71221b 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -6516,7 +6516,7 @@ static int scsi_debug_show_info(struct seq_file *m, struct Scsi_Host *host)
 		blk_mq_tagset_busy_iter(&host->tag_set, sdebug_submit_queue_iter,
 					&data);
 		if (f >= 0) {
-			seq_printf(m, "    in_use_bm BUSY: %s: %d,%d\n",
+			seq_printf(m, "    BUSY: %s: %d,%d\n",
 				   "first,last bits", f, l);
 		}
 	}

[PATCH v3 2/4] scsi: scsi_debug: Do not sleep in atomic sections

[Bart Van Assche]

stop_qc_helper() is called while a spinlock is held. cancel_work_sync()
may sleep. Sleeping in atomic sections is not allowed. Hence change the
cancel_work_sync() call into a cancel_work() call.

Cc: Douglas Gilbert <dgilbert@interlog.com>
Cc: John Garry <john.g.garry@oracle.com>
Fixes: 1107c7b24ee3 ("scsi: scsi_debug: Dynamically allocate sdebug_queued_cmd")
Cc: stable@vger.kernel.org
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/scsi_debug.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 36368c71221b..7a0b7402b715 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -5648,7 +5648,7 @@ static void scsi_debug_slave_destroy(struct scsi_device *sdp)
 	sdp->hostdata = NULL;
 }
 
-/* Returns true if we require the queued memory to be freed by the caller. */
+/* 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)
 {
@@ -5664,11 +5664,8 @@ static bool stop_qc_helper(struct sdebug_defer *sd_dp,
 			return true;
 		}
 	} else if (defer_t == SDEB_DEFER_WQ) {
-		/* Cancel if pending */
-		if (cancel_work_sync(&sd_dp->ew.work))
-			return true;
-		/* Was not pending, so it must have run */
-		return false;
+		/* The caller must free qcmd if cancellation succeeds. */
+		return cancel_work(&sd_dp->ew.work);
 	} else if (defer_t == SDEB_DEFER_POLL) {
 		return true;
 	}

[PATCH v3 3/4] scsi: scsi_debug: Simplify command handling

[Bart Van Assche]

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 <dgilbert@interlog.com>
Cc: John Garry <john.g.garry@oracle.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 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;
 }

[PATCH v3 4/4] scsi: scsi_debug: Make CRC_T10DIF support optional

[Bart Van Assche]

Not all scsi_debug users need data integrity support. Hence modify the
scsi_debug driver such that it becomes possible to build this driver
without data integrity support. The changes in this patch are as
follows:
- Split the scsi_debug source code into two files without modifying any
  functionality.
- Instead of selecting CRC_T10DIF no matter how the scsi_debug driver is
  built, only select CRC_T10DIF if the scsi_debug driver is built-in to
  the kernel.

Cc: Douglas Gilbert <dgilbert@interlog.com>
Cc: John Garry <john.g.garry@oracle.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/Kconfig                          |   3 +-
 drivers/scsi/Makefile                         |   2 +
 drivers/scsi/scsi_debug_dif.c                 | 240 +++++++++++++++++
 drivers/scsi/scsi_debug_dif.h                 |  57 ++++
 .../scsi/{scsi_debug.c => scsi_debug_main.c}  | 254 +-----------------
 5 files changed, 306 insertions(+), 250 deletions(-)
 create mode 100644 drivers/scsi/scsi_debug_dif.c
 create mode 100644 drivers/scsi/scsi_debug_dif.h
 rename drivers/scsi/{scsi_debug.c => scsi_debug_main.c} (97%)

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 3328052c8715..a283338e0ebd 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -1226,8 +1226,7 @@ config SCSI_WD719X
 
 config SCSI_DEBUG
 	tristate "SCSI debugging host and device simulator"
-	depends on SCSI
-	select CRC_T10DIF
+	depends on SCSI && (m || CRC_T10DIF = y)
 	help
 	  This pseudo driver simulates one or more hosts (SCSI initiators),
 	  each with one or more targets, each with one or more logical units.
diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile
index 1313ddf2fd1a..6287d9d65f04 100644
--- a/drivers/scsi/Makefile
+++ b/drivers/scsi/Makefile
@@ -156,6 +156,8 @@ obj-$(CONFIG_SCSI_HISI_SAS) += hisi_sas/
 
 # This goes last, so that "real" scsi devices probe earlier
 obj-$(CONFIG_SCSI_DEBUG)	+= scsi_debug.o
+scsi_debug-y			+= scsi_debug_main.o
+scsi_debug-$(CONFIG_CRC_T10DIF) += scsi_debug_dif.o
 scsi_mod-y			+= scsi.o hosts.o scsi_ioctl.o \
 				   scsicam.o scsi_error.o scsi_lib.o
 scsi_mod-$(CONFIG_SCSI_CONSTANTS) += constants.o
diff --git a/drivers/scsi/scsi_debug_dif.c b/drivers/scsi/scsi_debug_dif.c
new file mode 100644
index 000000000000..b6cfd64f7a21
--- /dev/null
+++ b/drivers/scsi/scsi_debug_dif.c
@@ -0,0 +1,240 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+#include <linux/crc-t10dif.h>
+#include <linux/t10-pi.h>
+#include <net/checksum.h>
+#include "scsi_debug_dif.h"
+
+#define DEF_DIF 0
+#define DEF_DIX 0
+#define DEF_GUARD 0
+
+int sdebug_dif = DEF_DIF;
+int sdebug_dix = DEF_DIX;
+unsigned int sdebug_guard = DEF_GUARD;
+int dix_writes;
+int dix_reads;
+int dif_errors;
+
+static __be16 dif_compute_csum(const void *buf, int len)
+{
+	__be16 csum;
+
+	if (sdebug_guard)
+		csum = (__force __be16)ip_compute_csum(buf, len);
+	else
+		csum = cpu_to_be16(crc_t10dif(buf, len));
+
+	return csum;
+}
+
+static int dif_verify(struct t10_pi_tuple *sdt, const void *data,
+		      sector_t sector, u32 ei_lba)
+{
+	__be16 csum = dif_compute_csum(data, sdebug_sector_size);
+
+	if (sdt->guard_tag != csum) {
+		pr_err("GUARD check failed on sector %lu rcvd 0x%04x, data 0x%04x\n",
+			(unsigned long)sector,
+			be16_to_cpu(sdt->guard_tag),
+			be16_to_cpu(csum));
+		return 0x01;
+	}
+	if (sdebug_dif == T10_PI_TYPE1_PROTECTION &&
+	    be32_to_cpu(sdt->ref_tag) != (sector & 0xffffffff)) {
+		pr_err("REF check failed on sector %lu\n",
+			(unsigned long)sector);
+		return 0x03;
+	}
+	if (sdebug_dif == T10_PI_TYPE2_PROTECTION &&
+	    be32_to_cpu(sdt->ref_tag) != ei_lba) {
+		pr_err("REF check failed on sector %lu\n",
+			(unsigned long)sector);
+		return 0x03;
+	}
+	return 0;
+}
+
+static struct t10_pi_tuple *dif_store(struct sdeb_store_info *sip,
+				      sector_t sector)
+{
+	sector = sector_div(sector, sdebug_store_sectors);
+
+	return sip->dif_storep + sector;
+}
+
+static void dif_copy_prot(struct scsi_cmnd *scp, sector_t sector,
+			  unsigned int sectors, bool read)
+{
+	size_t resid;
+	void *paddr;
+	struct sdeb_store_info *sip = devip2sip((struct sdebug_dev_info *)
+						scp->device->hostdata, true);
+	struct t10_pi_tuple *dif_storep = sip->dif_storep;
+	const void *dif_store_end = dif_storep + sdebug_store_sectors;
+	struct sg_mapping_iter miter;
+
+	/* Bytes of protection data to copy into sgl */
+	resid = sectors * sizeof(*dif_storep);
+
+	sg_miter_start(&miter, scsi_prot_sglist(scp),
+		       scsi_prot_sg_count(scp), SG_MITER_ATOMIC |
+		       (read ? SG_MITER_TO_SG : SG_MITER_FROM_SG));
+
+	while (sg_miter_next(&miter) && resid > 0) {
+		size_t len = min_t(size_t, miter.length, resid);
+		void *start = dif_store(sip, sector);
+		size_t rest = 0;
+
+		if (dif_store_end < start + len)
+			rest = start + len - dif_store_end;
+
+		paddr = miter.addr;
+
+		if (read)
+			memcpy(paddr, start, len - rest);
+		else
+			memcpy(start, paddr, len - rest);
+
+		if (rest) {
+			if (read)
+				memcpy(paddr + len - rest, dif_storep, rest);
+			else
+				memcpy(dif_storep, paddr + len - rest, rest);
+		}
+
+		sector += len / sizeof(*dif_storep);
+		resid -= len;
+	}
+	sg_miter_stop(&miter);
+}
+
+static void *lba2fake_store(struct sdeb_store_info *sip,
+			    unsigned long long lba)
+{
+	struct sdeb_store_info *lsip = sip;
+
+	lba = do_div(lba, sdebug_store_sectors);
+	if (!sip || !sip->storep) {
+		WARN_ON_ONCE(true);
+		lsip = xa_load(per_store_ap, 0);  /* should never be NULL */
+	}
+	return lsip->storep + lba * sdebug_sector_size;
+}
+
+int prot_verify_read(struct scsi_cmnd *scp, sector_t start_sec,
+		     unsigned int sectors, u32 ei_lba)
+{
+	int ret = 0;
+	unsigned int i;
+	sector_t sector;
+	struct sdeb_store_info *sip = devip2sip((struct sdebug_dev_info *)
+						scp->device->hostdata, true);
+	struct t10_pi_tuple *sdt;
+
+	for (i = 0; i < sectors; i++, ei_lba++) {
+		sector = start_sec + i;
+		sdt = dif_store(sip, sector);
+
+		if (sdt->app_tag == cpu_to_be16(0xffff))
+			continue;
+
+		/*
+		 * Because scsi_debug acts as both initiator and
+		 * target we proceed to verify the PI even if
+		 * RDPROTECT=3. This is done so the "initiator" knows
+		 * which type of error to return. Otherwise we would
+		 * have to iterate over the PI twice.
+		 */
+		if (scp->cmnd[1] >> 5) { /* RDPROTECT */
+			ret = dif_verify(sdt, lba2fake_store(sip, sector),
+					 sector, ei_lba);
+			if (ret) {
+				dif_errors++;
+				break;
+			}
+		}
+	}
+
+	dif_copy_prot(scp, start_sec, sectors, true);
+	dix_reads++;
+
+	return ret;
+}
+
+int prot_verify_write(struct scsi_cmnd *SCpnt, sector_t start_sec,
+		      unsigned int sectors, u32 ei_lba)
+{
+	int ret;
+	struct t10_pi_tuple *sdt;
+	void *daddr;
+	sector_t sector = start_sec;
+	int ppage_offset;
+	int dpage_offset;
+	struct sg_mapping_iter diter;
+	struct sg_mapping_iter piter;
+
+	BUG_ON(scsi_sg_count(SCpnt) == 0);
+	BUG_ON(scsi_prot_sg_count(SCpnt) == 0);
+
+	sg_miter_start(&piter, scsi_prot_sglist(SCpnt),
+			scsi_prot_sg_count(SCpnt),
+			SG_MITER_ATOMIC | SG_MITER_FROM_SG);
+	sg_miter_start(&diter, scsi_sglist(SCpnt), scsi_sg_count(SCpnt),
+			SG_MITER_ATOMIC | SG_MITER_FROM_SG);
+
+	/* For each protection page */
+	while (sg_miter_next(&piter)) {
+		dpage_offset = 0;
+		if (WARN_ON(!sg_miter_next(&diter))) {
+			ret = 0x01;
+			goto out;
+		}
+
+		for (ppage_offset = 0; ppage_offset < piter.length;
+		     ppage_offset += sizeof(struct t10_pi_tuple)) {
+			/* If we're at the end of the current
+			 * data page advance to the next one
+			 */
+			if (dpage_offset >= diter.length) {
+				if (WARN_ON(!sg_miter_next(&diter))) {
+					ret = 0x01;
+					goto out;
+				}
+				dpage_offset = 0;
+			}
+
+			sdt = piter.addr + ppage_offset;
+			daddr = diter.addr + dpage_offset;
+
+			if (SCpnt->cmnd[1] >> 5 != 3) { /* WRPROTECT */
+				ret = dif_verify(sdt, daddr, sector, ei_lba);
+				if (ret)
+					goto out;
+			}
+
+			sector++;
+			ei_lba++;
+			dpage_offset += sdebug_sector_size;
+		}
+		diter.consumed = dpage_offset;
+		sg_miter_stop(&diter);
+	}
+	sg_miter_stop(&piter);
+
+	dif_copy_prot(SCpnt, start_sec, sectors, false);
+	dix_writes++;
+
+	return 0;
+
+out:
+	dif_errors++;
+	sg_miter_stop(&diter);
+	sg_miter_stop(&piter);
+	return ret;
+}
+
+module_param_named(dif, sdebug_dif, int, S_IRUGO);
+module_param_named(dix, sdebug_dix, int, S_IRUGO);
+
+MODULE_PARM_DESC(dif, "data integrity field type: 0-3 (def=0)");
+MODULE_PARM_DESC(dix, "data integrity extensions mask (def=0)");
diff --git a/drivers/scsi/scsi_debug_dif.h b/drivers/scsi/scsi_debug_dif.h
new file mode 100644
index 000000000000..7af633ced854
--- /dev/null
+++ b/drivers/scsi/scsi_debug_dif.h
@@ -0,0 +1,57 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _SCSI_DEBUG_DIF_H
+#define _SCSI_DEBUG_DIF_H
+
+#include <linux/kconfig.h>
+#include <linux/types.h>
+#include <linux/spinlock_types.h>
+#include <scsi/scsi_cmnd.h>
+
+struct sdebug_dev_info;
+struct t10_pi_tuple;
+
+extern int dix_writes;
+extern int dix_reads;
+extern int dif_errors;
+extern struct xarray *const per_store_ap;
+extern int sdebug_dif;
+extern int sdebug_dix;
+extern unsigned int sdebug_guard;
+extern int sdebug_sector_size;
+extern unsigned int sdebug_store_sectors;
+
+/* There is an xarray of pointers to this struct's objects, one per host */
+struct sdeb_store_info {
+	rwlock_t macc_lck;	/* for atomic media access on this store */
+	u8 *storep;		/* user data storage (ram) */
+	struct t10_pi_tuple *dif_storep; /* protection info */
+	void *map_storep;	/* provisioning map */
+};
+
+struct sdeb_store_info *devip2sip(struct sdebug_dev_info *devip,
+				  bool bug_if_fake_rw);
+
+#if IS_ENABLED(CONFIG_CRC_T10DIF)
+
+int prot_verify_read(struct scsi_cmnd *scp, sector_t start_sec,
+		     unsigned int sectors, u32 ei_lba);
+int prot_verify_write(struct scsi_cmnd *SCpnt, sector_t start_sec,
+		      unsigned int sectors, u32 ei_lba);
+
+#else /* CONFIG_CRC_T10DIF */
+
+static inline int prot_verify_read(struct scsi_cmnd *scp, sector_t start_sec,
+				   unsigned int sectors, u32 ei_lba)
+{
+	return 0x01; /* GUARD check failed */
+}
+
+static inline int prot_verify_write(struct scsi_cmnd *SCpnt, sector_t start_sec,
+				    unsigned int sectors, u32 ei_lba)
+{
+	return 0x01; /* GUARD check failed */
+}
+
+#endif /* CONFIG_CRC_T10DIF */
+
+#endif /* _SCSI_DEBUG_DIF_H */
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug_main.c
similarity index 97%
rename from drivers/scsi/scsi_debug.c
rename to drivers/scsi/scsi_debug_main.c
index 1597156cb573..7bc08c55c9e1 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug_main.c
@@ -30,13 +30,11 @@
 #include <linux/moduleparam.h>
 #include <linux/scatterlist.h>
 #include <linux/blkdev.h>
-#include <linux/crc-t10dif.h>
 #include <linux/spinlock.h>
 #include <linux/interrupt.h>
 #include <linux/atomic.h>
 #include <linux/hrtimer.h>
 #include <linux/uuid.h>
-#include <linux/t10-pi.h>
 #include <linux/msdos_partition.h>
 #include <linux/random.h>
 #include <linux/xarray.h>
@@ -45,8 +43,6 @@
 #include <linux/async.h>
 #include <linux/cleanup.h>
 
-#include <net/checksum.h>
-
 #include <asm/unaligned.h>
 
 #include <scsi/scsi.h>
@@ -59,6 +55,7 @@
 #include <scsi/scsi_dbg.h>
 
 #include "sd.h"
+#include "scsi_debug_dif.h"
 #include "scsi_logging.h"
 
 /* make sure inq_product_rev string corresponds to this version */
@@ -120,13 +117,10 @@ static const char *sdebug_version_date = "20210520";
 #define DEF_DEV_SIZE_PRE_INIT   0
 #define DEF_DEV_SIZE_MB   8
 #define DEF_ZBC_DEV_SIZE_MB   128
-#define DEF_DIF 0
-#define DEF_DIX 0
 #define DEF_PER_HOST_STORE false
 #define DEF_D_SENSE   0
 #define DEF_EVERY_NTH   0
 #define DEF_FAKE_RW	0
-#define DEF_GUARD 0
 #define DEF_HOST_LOCK 0
 #define DEF_LBPU 0
 #define DEF_LBPWS 0
@@ -367,14 +361,6 @@ struct sdebug_host_info {
 	struct list_head dev_info_list;
 };
 
-/* There is an xarray of pointers to this struct's objects, one per host */
-struct sdeb_store_info {
-	rwlock_t macc_lck;	/* for atomic media access on this store */
-	u8 *storep;		/* user data storage (ram) */
-	struct t10_pi_tuple *dif_storep; /* protection info */
-	void *map_storep;	/* provisioning map */
-};
-
 #define dev_to_sdebug_host(d)	\
 	container_of(d, struct sdebug_host_info, dev)
 
@@ -785,12 +771,9 @@ static int sdebug_ato = DEF_ATO;
 static int sdebug_cdb_len = DEF_CDB_LEN;
 static int sdebug_jdelay = DEF_JDELAY;	/* if > 0 then unit is jiffies */
 static int sdebug_dev_size_mb = DEF_DEV_SIZE_PRE_INIT;
-static int sdebug_dif = DEF_DIF;
-static int sdebug_dix = DEF_DIX;
 static int sdebug_dsense = DEF_D_SENSE;
 static int sdebug_every_nth = DEF_EVERY_NTH;
 static int sdebug_fake_rw = DEF_FAKE_RW;
-static unsigned int sdebug_guard = DEF_GUARD;
 static int sdebug_host_max_queue;	/* per host */
 static int sdebug_lowest_aligned = DEF_LOWEST_ALIGNED;
 static int sdebug_max_luns = DEF_MAX_LUNS;
@@ -808,7 +791,7 @@ static int sdebug_physblk_exp = DEF_PHYSBLK_EXP;
 static int sdebug_opt_xferlen_exp = DEF_OPT_XFERLEN_EXP;
 static int sdebug_ptype = DEF_PTYPE; /* SCSI peripheral device type */
 static int sdebug_scsi_level = DEF_SCSI_LEVEL;
-static int sdebug_sector_size = DEF_SECTOR_SIZE;
+int sdebug_sector_size = DEF_SECTOR_SIZE;
 static int sdeb_tur_ms_to_ready = DEF_TUR_MS_TO_READY;
 static int sdebug_virtual_gb = DEF_VIRTUAL_GB;
 static int sdebug_vpd_use_hostno = DEF_VPD_USE_HOSTNO;
@@ -850,7 +833,7 @@ enum sam_lun_addr_method {SAM_LUN_AM_PERIPHERAL = 0x0,
 static enum sam_lun_addr_method sdebug_lun_am = SAM_LUN_AM_PERIPHERAL;
 static int sdebug_lun_am_i = (int)SAM_LUN_AM_PERIPHERAL;
 
-static unsigned int sdebug_store_sectors;
+unsigned int sdebug_store_sectors;
 static sector_t sdebug_capacity;	/* in sectors */
 
 /* old BIOS stuff, kernel may get rid of them but some mode sense pages
@@ -863,7 +846,7 @@ static LIST_HEAD(sdebug_host_list);
 static DEFINE_MUTEX(sdebug_host_list_mutex);
 
 static struct xarray per_store_arr;
-static struct xarray *per_store_ap = &per_store_arr;
+struct xarray *const per_store_ap = &per_store_arr;
 static int sdeb_first_idx = -1;		/* invalid index ==> none created */
 static int sdeb_most_recent_idx = -1;
 static DEFINE_RWLOCK(sdeb_fake_rw_lck);	/* need a RW lock when fake_rw=1 */
@@ -874,9 +857,6 @@ static int num_dev_resets;
 static int num_target_resets;
 static int num_bus_resets;
 static int num_host_resets;
-static int dix_writes;
-static int dix_reads;
-static int dif_errors;
 
 /* ZBC global data */
 static bool sdeb_zbc_in_use;	/* true for host-aware and host-managed disks */
@@ -1174,27 +1154,6 @@ static inline bool scsi_debug_lbp(void)
 		(sdebug_lbpu || sdebug_lbpws || sdebug_lbpws10);
 }
 
-static void *lba2fake_store(struct sdeb_store_info *sip,
-			    unsigned long long lba)
-{
-	struct sdeb_store_info *lsip = sip;
-
-	lba = do_div(lba, sdebug_store_sectors);
-	if (!sip || !sip->storep) {
-		WARN_ON_ONCE(true);
-		lsip = xa_load(per_store_ap, 0);  /* should never be NULL */
-	}
-	return lsip->storep + lba * sdebug_sector_size;
-}
-
-static struct t10_pi_tuple *dif_store(struct sdeb_store_info *sip,
-				      sector_t sector)
-{
-	sector = sector_div(sector, sdebug_store_sectors);
-
-	return sip->dif_storep + sector;
-}
-
 static void sdebug_max_tgts_luns(void)
 {
 	struct sdebug_host_info *sdbg_host;
@@ -3353,8 +3312,8 @@ static inline int check_device_access_params
  * that access any of the "stores" in struct sdeb_store_info should call this
  * function with bug_if_fake_rw set to true.
  */
-static inline struct sdeb_store_info *devip2sip(struct sdebug_dev_info *devip,
-						bool bug_if_fake_rw)
+struct sdeb_store_info *devip2sip(struct sdebug_dev_info *devip,
+				  bool bug_if_fake_rw)
 {
 	if (sdebug_fake_rw) {
 		BUG_ON(bug_if_fake_rw);	/* See note above */
@@ -3457,131 +3416,6 @@ static bool comp_write_worker(struct sdeb_store_info *sip, u64 lba, u32 num,
 	return res;
 }
 
-static __be16 dif_compute_csum(const void *buf, int len)
-{
-	__be16 csum;
-
-	if (sdebug_guard)
-		csum = (__force __be16)ip_compute_csum(buf, len);
-	else
-		csum = cpu_to_be16(crc_t10dif(buf, len));
-
-	return csum;
-}
-
-static int dif_verify(struct t10_pi_tuple *sdt, const void *data,
-		      sector_t sector, u32 ei_lba)
-{
-	__be16 csum = dif_compute_csum(data, sdebug_sector_size);
-
-	if (sdt->guard_tag != csum) {
-		pr_err("GUARD check failed on sector %lu rcvd 0x%04x, data 0x%04x\n",
-			(unsigned long)sector,
-			be16_to_cpu(sdt->guard_tag),
-			be16_to_cpu(csum));
-		return 0x01;
-	}
-	if (sdebug_dif == T10_PI_TYPE1_PROTECTION &&
-	    be32_to_cpu(sdt->ref_tag) != (sector & 0xffffffff)) {
-		pr_err("REF check failed on sector %lu\n",
-			(unsigned long)sector);
-		return 0x03;
-	}
-	if (sdebug_dif == T10_PI_TYPE2_PROTECTION &&
-	    be32_to_cpu(sdt->ref_tag) != ei_lba) {
-		pr_err("REF check failed on sector %lu\n",
-			(unsigned long)sector);
-		return 0x03;
-	}
-	return 0;
-}
-
-static void dif_copy_prot(struct scsi_cmnd *scp, sector_t sector,
-			  unsigned int sectors, bool read)
-{
-	size_t resid;
-	void *paddr;
-	struct sdeb_store_info *sip = devip2sip((struct sdebug_dev_info *)
-						scp->device->hostdata, true);
-	struct t10_pi_tuple *dif_storep = sip->dif_storep;
-	const void *dif_store_end = dif_storep + sdebug_store_sectors;
-	struct sg_mapping_iter miter;
-
-	/* Bytes of protection data to copy into sgl */
-	resid = sectors * sizeof(*dif_storep);
-
-	sg_miter_start(&miter, scsi_prot_sglist(scp),
-		       scsi_prot_sg_count(scp), SG_MITER_ATOMIC |
-		       (read ? SG_MITER_TO_SG : SG_MITER_FROM_SG));
-
-	while (sg_miter_next(&miter) && resid > 0) {
-		size_t len = min_t(size_t, miter.length, resid);
-		void *start = dif_store(sip, sector);
-		size_t rest = 0;
-
-		if (dif_store_end < start + len)
-			rest = start + len - dif_store_end;
-
-		paddr = miter.addr;
-
-		if (read)
-			memcpy(paddr, start, len - rest);
-		else
-			memcpy(start, paddr, len - rest);
-
-		if (rest) {
-			if (read)
-				memcpy(paddr + len - rest, dif_storep, rest);
-			else
-				memcpy(dif_storep, paddr + len - rest, rest);
-		}
-
-		sector += len / sizeof(*dif_storep);
-		resid -= len;
-	}
-	sg_miter_stop(&miter);
-}
-
-static int prot_verify_read(struct scsi_cmnd *scp, sector_t start_sec,
-			    unsigned int sectors, u32 ei_lba)
-{
-	int ret = 0;
-	unsigned int i;
-	sector_t sector;
-	struct sdeb_store_info *sip = devip2sip((struct sdebug_dev_info *)
-						scp->device->hostdata, true);
-	struct t10_pi_tuple *sdt;
-
-	for (i = 0; i < sectors; i++, ei_lba++) {
-		sector = start_sec + i;
-		sdt = dif_store(sip, sector);
-
-		if (sdt->app_tag == cpu_to_be16(0xffff))
-			continue;
-
-		/*
-		 * Because scsi_debug acts as both initiator and
-		 * target we proceed to verify the PI even if
-		 * RDPROTECT=3. This is done so the "initiator" knows
-		 * which type of error to return. Otherwise we would
-		 * have to iterate over the PI twice.
-		 */
-		if (scp->cmnd[1] >> 5) { /* RDPROTECT */
-			ret = dif_verify(sdt, lba2fake_store(sip, sector),
-					 sector, ei_lba);
-			if (ret) {
-				dif_errors++;
-				break;
-			}
-		}
-	}
-
-	dif_copy_prot(scp, start_sec, sectors, true);
-	dix_reads++;
-
-	return ret;
-}
-
 static inline void
 sdeb_read_lock(struct sdeb_store_info *sip)
 {
@@ -3789,78 +3623,6 @@ static int resp_read_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 	return 0;
 }
 
-static int prot_verify_write(struct scsi_cmnd *SCpnt, sector_t start_sec,
-			     unsigned int sectors, u32 ei_lba)
-{
-	int ret;
-	struct t10_pi_tuple *sdt;
-	void *daddr;
-	sector_t sector = start_sec;
-	int ppage_offset;
-	int dpage_offset;
-	struct sg_mapping_iter diter;
-	struct sg_mapping_iter piter;
-
-	BUG_ON(scsi_sg_count(SCpnt) == 0);
-	BUG_ON(scsi_prot_sg_count(SCpnt) == 0);
-
-	sg_miter_start(&piter, scsi_prot_sglist(SCpnt),
-			scsi_prot_sg_count(SCpnt),
-			SG_MITER_ATOMIC | SG_MITER_FROM_SG);
-	sg_miter_start(&diter, scsi_sglist(SCpnt), scsi_sg_count(SCpnt),
-			SG_MITER_ATOMIC | SG_MITER_FROM_SG);
-
-	/* For each protection page */
-	while (sg_miter_next(&piter)) {
-		dpage_offset = 0;
-		if (WARN_ON(!sg_miter_next(&diter))) {
-			ret = 0x01;
-			goto out;
-		}
-
-		for (ppage_offset = 0; ppage_offset < piter.length;
-		     ppage_offset += sizeof(struct t10_pi_tuple)) {
-			/* If we're at the end of the current
-			 * data page advance to the next one
-			 */
-			if (dpage_offset >= diter.length) {
-				if (WARN_ON(!sg_miter_next(&diter))) {
-					ret = 0x01;
-					goto out;
-				}
-				dpage_offset = 0;
-			}
-
-			sdt = piter.addr + ppage_offset;
-			daddr = diter.addr + dpage_offset;
-
-			if (SCpnt->cmnd[1] >> 5 != 3) { /* WRPROTECT */
-				ret = dif_verify(sdt, daddr, sector, ei_lba);
-				if (ret)
-					goto out;
-			}
-
-			sector++;
-			ei_lba++;
-			dpage_offset += sdebug_sector_size;
-		}
-		diter.consumed = dpage_offset;
-		sg_miter_stop(&diter);
-	}
-	sg_miter_stop(&piter);
-
-	dif_copy_prot(SCpnt, start_sec, sectors, false);
-	dix_writes++;
-
-	return 0;
-
-out:
-	dif_errors++;
-	sg_miter_stop(&diter);
-	sg_miter_stop(&piter);
-	return ret;
-}
-
 static unsigned long lba_to_map_index(sector_t lba)
 {
 	if (sdebug_unmap_alignment)
@@ -6191,8 +5953,6 @@ module_param_named(cdb_len, sdebug_cdb_len, int, 0644);
 module_param_named(clustering, sdebug_clustering, bool, S_IRUGO | S_IWUSR);
 module_param_named(delay, sdebug_jdelay, int, S_IRUGO | S_IWUSR);
 module_param_named(dev_size_mb, sdebug_dev_size_mb, int, S_IRUGO);
-module_param_named(dif, sdebug_dif, int, S_IRUGO);
-module_param_named(dix, sdebug_dix, int, S_IRUGO);
 module_param_named(dsense, sdebug_dsense, int, S_IRUGO | S_IWUSR);
 module_param_named(every_nth, sdebug_every_nth, int, S_IRUGO | S_IWUSR);
 module_param_named(fake_rw, sdebug_fake_rw, int, S_IRUGO | S_IWUSR);
@@ -6268,8 +6028,6 @@ MODULE_PARM_DESC(cdb_len, "suggest CDB lengths to drivers (def=10)");
 MODULE_PARM_DESC(clustering, "when set enables larger transfers (def=0)");
 MODULE_PARM_DESC(delay, "response delay (def=1 jiffy); 0:imm, -1,-2:tiny");
 MODULE_PARM_DESC(dev_size_mb, "size in MiB of ram shared by devs(def=8)");
-MODULE_PARM_DESC(dif, "data integrity field type: 0-3 (def=0)");
-MODULE_PARM_DESC(dix, "data integrity extensions mask (def=0)");
 MODULE_PARM_DESC(dsense, "use descriptor sense format(def=0 -> fixed)");
 MODULE_PARM_DESC(every_nth, "timeout every nth command(def=0)");
 MODULE_PARM_DESC(fake_rw, "fake reads/writes instead of copying (def=0)");

Re: [PATCH v3 1/4] scsi: scsi_debug: Remove a reference to in_use_bm

[John Garry]

On 07/03/2024 20:30, Bart Van Assche wrote:
> Commit f1437cd1e535 ("scsi: scsi_debug: Drop sdebug_queue") removed
> the 'in_use_bm' struct member. Hence remove a reference to that struct
> member from the procfs host info file.
> 
> Cc: Douglas Gilbert<dgilbert@interlog.com>
> Cc: John Garry<john.g.garry@oracle.com>
> Fixes: f1437cd1e535 ("scsi: scsi_debug: Drop sdebug_queue")
> Signed-off-by: Bart Van Assche<bvanassche@acm.org>

Reviewed-by: John Garry <john.g.garry@oracle.com>

> ---
>   drivers/scsi/scsi_debug.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index acf0592d63da..36368c71221b 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -6516,7 +6516,7 @@ static int scsi_debug_show_info(struct seq_file *m, struct Scsi_Host *host)
>   		blk_mq_tagset_busy_iter(&host->tag_set, sdebug_submit_queue_iter,
>   					&data);
>   		if (f >= 0) {
> -			seq_printf(m, "    in_use_bm BUSY: %s: %d,%d\n",
> +			seq_printf(m, "    BUSY: %s: %d,%d\n",
>   				   "first,last bits", f, l);

maybe this can fit on a single line now without exceeding 80 char (which 
some people still want - I don't mind).

Thanks,
John

Re: [PATCH v3 2/4] scsi: scsi_debug: Do not sleep in atomic sections

[John Garry]

On 07/03/2024 20:30, Bart Van Assche wrote:
> stop_qc_helper() is called while a spinlock is held. cancel_work_sync()
> may sleep. Sleeping in atomic sections is not allowed. Hence change the
> cancel_work_sync() call into a cancel_work() call.
> 
> Cc: Douglas Gilbert <dgilbert@interlog.com>
> Cc: John Garry <john.g.garry@oracle.com>
> Fixes: 1107c7b24ee3 ("scsi: scsi_debug: Dynamically allocate sdebug_queued_cmd")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>   drivers/scsi/scsi_debug.c | 9 +++------
>   1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 36368c71221b..7a0b7402b715 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -5648,7 +5648,7 @@ static void scsi_debug_slave_destroy(struct scsi_device *sdp)
>   	sdp->hostdata = NULL;
>   }
>   
> -/* Returns true if we require the queued memory to be freed by the caller. */
> +/* 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)
>   {
> @@ -5664,11 +5664,8 @@ static bool stop_qc_helper(struct sdebug_defer *sd_dp,
>   			return true;
>   		}
>   	} else if (defer_t == SDEB_DEFER_WQ) {
> -		/* Cancel if pending */
> -		if (cancel_work_sync(&sd_dp->ew.work))
> -			return true;
> -		/* Was not pending, so it must have run */
> -		return false;
> +		/* The caller must free qcmd if cancellation succeeds. */


We were relying on the work CB not running or runnable when we return 
from this function, and that is why there is cancel_work_sync() [which 
is obviously bad under a spinlock]

Otherwise, sdebug_q_cmd_wq_complete() -> sdebug_q_cmd_wq_complete() may 
be running and reference the scsi_cmnd - that should not be done, because...

Checking the comment on scsi_try_to_abort_cmd(), it reads:
" SUCCESS ... indicates that the LLDDs has cleared all references to 
that command"

So, if we change to cancel_work(), we really should ensure 
scsi_debug_abort() -> scsi_debug_abort_cmnd() returns FALSE/FAILED to 
upper layer for when cancel_work() returns false. Effectively the 
(!sqcp) check in scsi_debug_stop_cmnd() checks for already run or not 
even queued. However, we still need to consider when the WQ callback is 
running.

Cheers,
John

Re: [PATCH v3 1/4] scsi: scsi_debug: Remove a reference to in_use_bm

[Bart Van Assche]

On 3/8/24 03:46, John Garry wrote:
> On 07/03/2024 20:30, Bart Van Assche wrote:
>> Commit f1437cd1e535 ("scsi: scsi_debug: Drop sdebug_queue") removed
>> the 'in_use_bm' struct member. Hence remove a reference to that struct
>> member from the procfs host info file.
>>
>> Cc: Douglas Gilbert<dgilbert@interlog.com>
>> Cc: John Garry<john.g.garry@oracle.com>
>> Fixes: f1437cd1e535 ("scsi: scsi_debug: Drop sdebug_queue")
>> Signed-off-by: Bart Van Assche<bvanassche@acm.org>
> 
> Reviewed-by: John Garry <john.g.garry@oracle.com>
> 
>> ---
>>   drivers/scsi/scsi_debug.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
>> index acf0592d63da..36368c71221b 100644
>> --- a/drivers/scsi/scsi_debug.c
>> +++ b/drivers/scsi/scsi_debug.c
>> @@ -6516,7 +6516,7 @@ static int scsi_debug_show_info(struct seq_file 
>> *m, struct Scsi_Host *host)
>>           blk_mq_tagset_busy_iter(&host->tag_set, 
>> sdebug_submit_queue_iter,
>>                       &data);
>>           if (f >= 0) {
>> -            seq_printf(m, "    in_use_bm BUSY: %s: %d,%d\n",
>> +            seq_printf(m, "    BUSY: %s: %d,%d\n",
>>                      "first,last bits", f, l);
> 
> maybe this can fit on a single line now without exceeding 80 char (which 
> some people still want - I don't mind).

The most compact version of this patch I can come up with is the
following:

@@ -6384,8 +6384,8 @@ static int scsi_debug_show_info(struct seq_file 
*m, struct Scsi_Host *host)
  		blk_mq_tagset_busy_iter(&host->tag_set, sdebug_submit_queue_iter,
  					&data);
  		if (f >= 0) {
-			seq_printf(m, "    in_use_bm BUSY: %s: %d,%d\n",
-				   "first,last bits", f, l);
+			seq_printf(m, "    BUSY: first,last bits: %d,%d\n", f,
+				   l);
  		}
  	}

Thanks,

Bart.