[RFC 0/6] scsi_debug: random doublestore verify

[RFC 0/6] scsi_debug: random doublestore verify

[Douglas Gilbert]

This patchset contains various measures to improve the speed and
usefulness of this driver. It has been used to test the rewrite
of the SCSI generic (sg) driver which is still underway.

Disk to disk copies are the test of choice by the author. Some
testing has been done using real hard disks and SSDs but the
bulk of the testing has been done using this driver as both the
source and destination of the copy. SSDs have two shortcomings:
they are not as fast as the manufacturers would like users to
believe with an average latency to READ at around 100
microseconds; the second problem is "endurance". Endurance is
a wear-out factor based on the number of WRITEs to the SSD.
One would hope both these measures will improve in the future.

The author found that precise command duration timing gave a
false impression of how "bulletproof" the sg driver state
machines and locking was. The first patch involving randomizing
the command durations and it did expose various issues in the
driver under test (sg). The next issue was the correctness of
the bulk copies being done. The doublestore and verify patches
allow the copies to be verified and it demonstrated at least
one area of concern for the sg driver.

Since all scsi_debug memory stores accesses are done in the
context of queuecommand() call, the *_irqsave() and
*_irqrestore() variants of the associated locks have been
removed.  That could be a problem if queuecommand() can ever
be called form an interrupt or related context.

Finally to address the discrepancy between command duration
times seen by the sg driver compared to what was set with
this driver's ndelay option, this driver's timekeeping for
short durations was made more accurate.

This patchset is against Martin Petersen's git repository
and its 5.6/scsi-queue branch.

---

One shortcoming of the doublestore approach is that both
stores share all the other module settings that are
held at file scope in the driver. With a little extra hacking
it is possible to have multiple scsi_debug (like) drivers
in a single kernel. That may be one way around this
shortcoming.

Douglas Gilbert (6):
  scsi_debug: randomize command completion time
  scsi_debug: add doublestore option
  scsi_debug: implement verify(10), add verify(16)
  scsi_debug: weaken rwlock around ramdisk access
  scsi_debug: improve command duration calculation
  scsi_debug: bump to version 1.89

 drivers/scsi/scsi_debug.c | 444 +++++++++++++++++++++++++++++---------
 1 file changed, 343 insertions(+), 101 deletions(-)

-- 
2.24.1

[RFC 1/6] scsi_debug: randomize command completion time

[Douglas Gilbert]

Add a new command line option (e.g. random=1) and sysfs attribute
that causes subsequent command completion times to be between the
current command delay setting and 0. A unformily distributed 32
bit, kernel provided integer is used for this purpose.

Since the existing 'delay' whose units are jiffies (typically
milliseconds) and 'ndelay' (units: nanoseconds) options (and sysfs
attributes) span a range greater than 32 bits, some scaling is
required.

The purpose of this patch is to widen the range of testing cases
that are visited in long running tests. Put simply: rarely struct
race conditions are more likely to be found when this facility is
used.

The default is the previous case in which all command completions
were roughly equal to (if not, slightly longer) than the value
given by the 'delay' or 'ndelay' settings (or their defaults).
This option's default is equivalent to setting 'random=0' .

Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
---
 drivers/scsi/scsi_debug.c | 42 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 39 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 44cb054d5e66..18c07e9ace15 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -38,6 +38,7 @@
 #include <linux/hrtimer.h>
 #include <linux/uuid.h>
 #include <linux/t10-pi.h>
+#include <linux/random.h>
 
 #include <net/checksum.h>
 
@@ -125,6 +126,7 @@ static const char *sdebug_version_date = "20190125";
 #define DEF_PHYSBLK_EXP 0
 #define DEF_OPT_XFERLEN_EXP 0
 #define DEF_PTYPE   TYPE_DISK
+#define DEF_RANDOM false
 #define DEF_REMOVABLE false
 #define DEF_SCSI_LEVEL   7    /* INQUIRY, byte2 [6->SPC-4; 7->SPC-5] */
 #define DEF_SECTOR_SIZE 512
@@ -655,6 +657,7 @@ static unsigned int sdebug_unmap_max_blocks = DEF_UNMAP_MAX_BLOCKS;
 static unsigned int sdebug_unmap_max_desc = DEF_UNMAP_MAX_DESC;
 static unsigned int sdebug_write_same_length = DEF_WRITESAME_LENGTH;
 static int sdebug_uuid_ctl = DEF_UUID_CTL;
+static bool sdebug_random = DEF_RANDOM;
 static bool sdebug_removable = DEF_REMOVABLE;
 static bool sdebug_clustering;
 static bool sdebug_host_lock = DEF_HOST_LOCK;
@@ -4354,9 +4357,21 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
 		ktime_t kt;
 
 		if (delta_jiff > 0) {
-			kt = ns_to_ktime((u64)delta_jiff * (NSEC_PER_SEC / HZ));
-		} else
-			kt = ndelay;
+			u64 ns = jiffies_to_nsecs(delta_jiff);
+
+			if (sdebug_random && ns < U32_MAX) {
+				ns = prandom_u32_max((u32)ns);
+			} else if (sdebug_random) {
+				ns >>= 12;	/* scale to 4 usec precision */
+				if (ns < U32_MAX)	/* over 4 hours max */
+					ns = prandom_u32_max((u32)ns);
+				ns <<= 12;
+			}
+			kt = ns_to_ktime(ns);
+		} else {	/* ndelay has a 4.2 second max */
+			kt = sdebug_random ? prandom_u32_max((u32)ndelay) :
+					     (u32)ndelay;
+		}
 		if (!sd_dp->init_hrt) {
 			sd_dp->init_hrt = true;
 			sqcp->sd_dp = sd_dp;
@@ -4451,6 +4466,7 @@ module_param_named(opts, sdebug_opts, int, S_IRUGO | S_IWUSR);
 module_param_named(physblk_exp, sdebug_physblk_exp, int, S_IRUGO);
 module_param_named(opt_xferlen_exp, sdebug_opt_xferlen_exp, int, S_IRUGO);
 module_param_named(ptype, sdebug_ptype, int, S_IRUGO | S_IWUSR);
+module_param_named(random, sdebug_random, bool, S_IRUGO | S_IWUSR);
 module_param_named(removable, sdebug_removable, bool, S_IRUGO | S_IWUSR);
 module_param_named(scsi_level, sdebug_scsi_level, int, S_IRUGO);
 module_param_named(sector_size, sdebug_sector_size, int, S_IRUGO);
@@ -4511,6 +4527,7 @@ MODULE_PARM_DESC(opts, "1->noise, 2->medium_err, 4->timeout, 8->recovered_err...
 MODULE_PARM_DESC(physblk_exp, "physical block exponent (def=0)");
 MODULE_PARM_DESC(opt_xferlen_exp, "optimal transfer length granularity exponent (def=physblk_exp)");
 MODULE_PARM_DESC(ptype, "SCSI peripheral type(def=0[disk])");
+MODULE_PARM_DESC(random, "If set, uniformly randomize command duration between 0 and delay_in_ns");
 MODULE_PARM_DESC(removable, "claim to have removable media (def=0)");
 MODULE_PARM_DESC(scsi_level, "SCSI level to simulate(def=7[SPC-5])");
 MODULE_PARM_DESC(sector_size, "logical block size in bytes (def=512)");
@@ -5101,6 +5118,24 @@ static ssize_t map_show(struct device_driver *ddp, char *buf)
 }
 static DRIVER_ATTR_RO(map);
 
+static ssize_t random_show(struct device_driver *ddp, char *buf)
+{
+	return scnprintf(buf, PAGE_SIZE, "%d\n", (int)sdebug_random);
+}
+
+static ssize_t random_store(struct device_driver *ddp, const char *buf,
+			    size_t count)
+{
+	int n;
+
+	if (count > 0 && kstrtoint(buf, 10, &n) == 0 && n >= 0) {
+		sdebug_random = (n > 0);
+		return count;
+	}
+	return -EINVAL;
+}
+static DRIVER_ATTR_RW(random);
+
 static ssize_t removable_show(struct device_driver *ddp, char *buf)
 {
 	return scnprintf(buf, PAGE_SIZE, "%d\n", sdebug_removable ? 1 : 0);
@@ -5211,6 +5246,7 @@ static struct attribute *sdebug_drv_attrs[] = {
 	&driver_attr_guard.attr,
 	&driver_attr_ato.attr,
 	&driver_attr_map.attr,
+	&driver_attr_random.attr,
 	&driver_attr_removable.attr,
 	&driver_attr_host_lock.attr,
 	&driver_attr_ndelay.attr,
-- 
2.24.1

[RFC 2/6] scsi_debug: add doublestore option

[Douglas Gilbert]

The scsi_debug driver has always been restricted to using one
(or none) ramdisk image for its storage. This means that thousands
of scsi_debug devices can be created without exhausting the host
machine's RAM. The downside is that all scsi_debug devices share
the same ramdisk image. This option doubles the amount of ramdisk
storage space with the first, third, fifth (etc) created
scsi_debug devices using the first ramdisk image while the second,
fourth, sixth (etc) created scsi_debug devices using the second
ramdisk image.

The reason for doing this is to check that (partial) disk to disk
copies based on scsi_debug devices have actually worked properly.
As an example: assume /dev/sdb and /dev/sg1 are the same
scsi_debug device, while /dev/sdc and /dev/sg2 are also the
same scsi_debug device. With doublestore=1 they will have
different ramdisk images. Then the following pseudocode could
be executed to check the if sgh_dd copy worked:
    dd if=/dev/urandom of=/dev/sdb
    sgh_dd if=/dev/sg1 of=/dev/sg2 [plus option(s) to test]
    cmp /dev/sdb /dev/sdc

If the cmp fails then the copy has failed (or some other
mechanism wrote to /dev/sdb or /dev/sdc in the interim).

Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
---
 drivers/scsi/scsi_debug.c | 250 ++++++++++++++++++++++++++------------
 1 file changed, 174 insertions(+), 76 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 18c07e9ace15..45934dae8617 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -108,6 +108,7 @@ static const char *sdebug_version_date = "20190125";
 #define DEF_DEV_SIZE_MB   8
 #define DEF_DIF 0
 #define DEF_DIX 0
+#define DEF_DOUBLESTORE false
 #define DEF_D_SENSE   0
 #define DEF_EVERY_NTH   0
 #define DEF_FAKE_RW	0
@@ -255,6 +256,7 @@ struct sdebug_dev_info {
 	unsigned long uas_bm[1];
 	atomic_t num_in_q;
 	atomic_t stopped;
+	int sdg_devnum;
 	bool used;
 };
 
@@ -633,6 +635,7 @@ static int sdebug_max_queue = SDEBUG_CANQUEUE;	/* per submit queue */
 static unsigned int sdebug_medium_error_start = OPT_MEDIUM_ERR_ADDR;
 static int sdebug_medium_error_count = OPT_MEDIUM_ERR_NUM;
 static atomic_t retired_max_queue;	/* if > 0 then was prior max_queue */
+static atomic_t a_sdg_devnum;
 static int sdebug_ndelay = DEF_NDELAY;	/* if > 0 then unit is nanoseconds */
 static int sdebug_no_lun_0 = DEF_NO_LUN_0;
 static int sdebug_no_uld;
@@ -658,6 +661,7 @@ static unsigned int sdebug_unmap_max_desc = DEF_UNMAP_MAX_DESC;
 static unsigned int sdebug_write_same_length = DEF_WRITESAME_LENGTH;
 static int sdebug_uuid_ctl = DEF_UUID_CTL;
 static bool sdebug_random = DEF_RANDOM;
+static bool sdebug_doublestore = DEF_DOUBLESTORE;
 static bool sdebug_removable = DEF_REMOVABLE;
 static bool sdebug_clustering;
 static bool sdebug_host_lock = DEF_HOST_LOCK;
@@ -681,7 +685,7 @@ static int sdebug_sectors_per;		/* sectors per cylinder */
 static LIST_HEAD(sdebug_host_list);
 static DEFINE_SPINLOCK(sdebug_host_list_lock);
 
-static unsigned char *fake_storep;	/* ramdisk storage */
+static u8 *fake_store_a[2];		/* ramdisk storage */
 static struct t10_pi_tuple *dif_storep;	/* protection info */
 static void *map_storep;		/* provisioning map */
 
@@ -699,6 +703,9 @@ static int submit_queues = DEF_SUBMIT_QUEUES;  /* > 1 for multi-queue (mq) */
 static struct sdebug_queue *sdebug_q_arr;  /* ptr to array of submit queues */
 
 static DEFINE_RWLOCK(atomic_rw);
+static DEFINE_RWLOCK(atomic_rw2);
+
+static rwlock_t *ramdisk_lck_a[2];
 
 static char sdebug_proc_name[] = MY_NAME;
 static const char *my_name = MY_NAME;
@@ -730,11 +737,11 @@ static inline bool scsi_debug_lbp(void)
 		(sdebug_lbpu || sdebug_lbpws || sdebug_lbpws10);
 }
 
-static void *lba2fake_store(unsigned long long lba)
+static void *lba2fake_store(unsigned long long lba, int acc_num)
 {
 	lba = do_div(lba, sdebug_store_sectors);
 
-	return fake_storep + lba * sdebug_sector_size;
+	return fake_store_a[acc_num % 2] + lba * sdebug_sector_size;
 }
 
 static struct t10_pi_tuple *dif_store(sector_t sector)
@@ -1043,7 +1050,7 @@ static int p_fill_from_dev_buffer(struct scsi_cmnd *scp, const void *arr,
 		 __func__, off_dst, scsi_bufflen(scp), act_len,
 		 scsi_get_resid(scp));
 	n = scsi_bufflen(scp) - (off_dst + act_len);
-	scsi_set_resid(scp, min(scsi_get_resid(scp), n));
+	scsi_set_resid(scp, min_t(int, scsi_get_resid(scp), n));
 	return 0;
 }
 
@@ -1535,7 +1542,7 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 	}
 	put_unaligned_be16(0x2100, arr + n);	/* SPL-4 no version claimed */
 	ret = fill_from_dev_buffer(scp, arr,
-			    min(alloc_len, SDEBUG_LONG_INQ_SZ));
+			    min_t(int, alloc_len, SDEBUG_LONG_INQ_SZ));
 	kfree(arr);
 	return ret;
 }
@@ -1690,7 +1697,7 @@ static int resp_readcap16(struct scsi_cmnd *scp,
 	}
 
 	return fill_from_dev_buffer(scp, arr,
-				    min(alloc_len, SDEBUG_READCAP16_ARR_SZ));
+			    min_t(int, alloc_len, SDEBUG_READCAP16_ARR_SZ));
 }
 
 #define SDEBUG_MAX_TGTPGS_ARR_SZ 1412
@@ -1764,9 +1771,9 @@ static int resp_report_tgtpgs(struct scsi_cmnd *scp,
 	 * - The constructed command length
 	 * - The maximum array size
 	 */
-	rlen = min(alen,n);
+	rlen = min_t(int, alen,n);
 	ret = fill_from_dev_buffer(scp, arr,
-				   min(rlen, SDEBUG_MAX_TGTPGS_ARR_SZ));
+			   min_t(int, rlen, SDEBUG_MAX_TGTPGS_ARR_SZ));
 	kfree(arr);
 	return ret;
 }
@@ -2268,7 +2275,7 @@ static int resp_mode_sense(struct scsi_cmnd *scp,
 		arr[0] = offset - 1;
 	else
 		put_unaligned_be16((offset - 2), arr + 0);
-	return fill_from_dev_buffer(scp, arr, min(alloc_len, offset));
+	return fill_from_dev_buffer(scp, arr, min_t(int, alloc_len, offset));
 }
 
 #define SDEBUG_MAX_MSELECT_SZ 512
@@ -2453,9 +2460,9 @@ static int resp_log_sense(struct scsi_cmnd *scp,
 		mk_sense_invalid_fld(scp, SDEB_IN_CDB, 3, -1);
 		return check_condition_result;
 	}
-	len = min(get_unaligned_be16(arr + 2) + 4, alloc_len);
+	len = min_t(int, get_unaligned_be16(arr + 2) + 4, alloc_len);
 	return fill_from_dev_buffer(scp, arr,
-		    min(len, SDEBUG_MAX_INQ_ARR_SZ));
+		    min_t(int, len, SDEBUG_MAX_INQ_ARR_SZ));
 }
 
 static inline int check_device_access_params(struct scsi_cmnd *scp,
@@ -2478,6 +2485,18 @@ static inline int check_device_access_params(struct scsi_cmnd *scp,
 	return 0;
 }
 
+static int scp2acc_num(struct scsi_cmnd *scp)
+{
+	if (sdebug_doublestore) {
+		struct scsi_device *sdp = scp->device;
+		struct sdebug_dev_info *devip =
+				(struct sdebug_dev_info *)sdp->hostdata;
+
+		return devip->sdg_devnum;
+	}
+	return 0;
+}
+
 /* Returns number of bytes copied or -1 if error. */
 static int do_device_access(struct scsi_cmnd *scmd, u32 sg_skip, u64 lba,
 			    u32 num, bool do_write)
@@ -2486,6 +2505,7 @@ static int do_device_access(struct scsi_cmnd *scmd, u32 sg_skip, u64 lba,
 	u64 block, rest = 0;
 	struct scsi_data_buffer *sdb = &scmd->sdb;
 	enum dma_data_direction dir;
+	u8 *fsp;
 
 	if (do_write) {
 		dir = DMA_TO_DEVICE;
@@ -2498,20 +2518,21 @@ static int do_device_access(struct scsi_cmnd *scmd, u32 sg_skip, u64 lba,
 		return 0;
 	if (scmd->sc_data_direction != dir)
 		return -1;
+	fsp = fake_store_a[scp2acc_num(scmd) % 2];
 
 	block = do_div(lba, sdebug_store_sectors);
 	if (block + num > sdebug_store_sectors)
 		rest = block + num - sdebug_store_sectors;
 
 	ret = sg_copy_buffer(sdb->table.sgl, sdb->table.nents,
-		   fake_storep + (block * sdebug_sector_size),
+		   fsp + (block * sdebug_sector_size),
 		   (num - rest) * sdebug_sector_size, sg_skip, do_write);
 	if (ret != (num - rest) * sdebug_sector_size)
 		return ret;
 
 	if (rest) {
 		ret += sg_copy_buffer(sdb->table.sgl, sdb->table.nents,
-			    fake_storep, rest * sdebug_sector_size,
+			    fsp, rest * sdebug_sector_size,
 			    sg_skip + ((num - rest) * sdebug_sector_size),
 			    do_write);
 	}
@@ -2522,31 +2543,32 @@ static int do_device_access(struct scsi_cmnd *scmd, u32 sg_skip, u64 lba,
 /* If lba2fake_store(lba,num) compares equal to arr(num), then copy top half of
  * arr into lba2fake_store(lba,num) and return true. If comparison fails then
  * return false. */
-static bool comp_write_worker(u64 lba, u32 num, const u8 *arr)
+static bool comp_write_worker(u64 lba, u32 num, const u8 *arr, int acc_num)
 {
 	bool res;
 	u64 block, rest = 0;
 	u32 store_blks = sdebug_store_sectors;
 	u32 lb_size = sdebug_sector_size;
+	u8 *fsp;
 
 	block = do_div(lba, store_blks);
 	if (block + num > store_blks)
 		rest = block + num - store_blks;
 
-	res = !memcmp(fake_storep + (block * lb_size), arr,
-		      (num - rest) * lb_size);
+	fsp = fake_store_a[acc_num % 2];
+
+	res = !memcmp(fsp + (block * lb_size), arr, (num - rest) * lb_size);
 	if (!res)
 		return res;
 	if (rest)
-		res = memcmp(fake_storep, arr + ((num - rest) * lb_size),
+		res = memcmp(fsp, arr + ((num - rest) * lb_size),
 			     rest * lb_size);
 	if (!res)
 		return res;
 	arr += num * lb_size;
-	memcpy(fake_storep + (block * lb_size), arr, (num - rest) * lb_size);
+	memcpy(fsp + (block * lb_size), arr, (num - rest) * lb_size);
 	if (rest)
-		memcpy(fake_storep, arr + ((num - rest) * lb_size),
-		       rest * lb_size);
+		memcpy(fsp, arr + ((num - rest) * lb_size), rest * lb_size);
 	return res;
 }
 
@@ -2605,7 +2627,7 @@ static void dif_copy_prot(struct scsi_cmnd *SCpnt, sector_t sector,
 			(read ? SG_MITER_TO_SG : SG_MITER_FROM_SG));
 
 	while (sg_miter_next(&miter) && resid > 0) {
-		size_t len = min(miter.length, resid);
+		size_t len = min_t(size_t, miter.length, resid);
 		void *start = dif_store(sector);
 		size_t rest = 0;
 
@@ -2632,12 +2654,12 @@ static void dif_copy_prot(struct scsi_cmnd *SCpnt, sector_t sector,
 	sg_miter_stop(&miter);
 }
 
-static int prot_verify_read(struct scsi_cmnd *SCpnt, sector_t start_sec,
+static int prot_verify_read(struct scsi_cmnd *scp, sector_t start_sec,
 			    unsigned int sectors, u32 ei_lba)
 {
 	unsigned int i;
-	struct t10_pi_tuple *sdt;
 	sector_t sector;
+	struct t10_pi_tuple *sdt;
 
 	for (i = 0; i < sectors; i++, ei_lba++) {
 		int ret;
@@ -2648,14 +2670,15 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, sector_t start_sec,
 		if (sdt->app_tag == cpu_to_be16(0xffff))
 			continue;
 
-		ret = dif_verify(sdt, lba2fake_store(sector), sector, ei_lba);
+		ret = dif_verify(sdt, lba2fake_store(sector, scp2acc_num(scp)),
+				 sector, ei_lba);
 		if (ret) {
 			dif_errors++;
 			return ret;
 		}
 	}
 
-	dif_copy_prot(SCpnt, start_sec, sectors, true);
+	dif_copy_prot(scp, start_sec, sectors, true);
 	dix_reads++;
 
 	return 0;
@@ -2665,10 +2688,11 @@ static int resp_read_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 {
 	u8 *cmd = scp->cmnd;
 	struct sdebug_queued_cmd *sqcp;
-	u64 lba;
 	u32 num;
 	u32 ei_lba;
+	int acc_num = scp2acc_num(scp);
 	unsigned long iflags;
+	u64 lba;
 	int ret;
 	bool check_prot;
 
@@ -2752,21 +2776,22 @@ static int resp_read_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 		return check_condition_result;
 	}
 
-	read_lock_irqsave(&atomic_rw, iflags);
+	read_lock_irqsave(ramdisk_lck_a[acc_num % 2], iflags);
 
 	/* DIX + T10 DIF */
 	if (unlikely(sdebug_dix && scsi_prot_sg_count(scp))) {
 		int prot_ret = prot_verify_read(scp, lba, num, ei_lba);
 
 		if (prot_ret) {
-			read_unlock_irqrestore(&atomic_rw, iflags);
+			read_unlock_irqrestore(ramdisk_lck_a[acc_num % 2],
+					       iflags);
 			mk_sense_buffer(scp, ABORTED_COMMAND, 0x10, prot_ret);
 			return illegal_condition_result;
 		}
 	}
 
 	ret = do_device_access(scp, 0, lba, num, false);
-	read_unlock_irqrestore(&atomic_rw, iflags);
+	read_unlock_irqrestore(ramdisk_lck_a[acc_num % 2], iflags);
 	if (unlikely(ret == -1))
 		return DID_ERROR << 16;
 
@@ -2938,9 +2963,10 @@ static void map_region(sector_t lba, unsigned int len)
 	}
 }
 
-static void unmap_region(sector_t lba, unsigned int len)
+static void unmap_region(sector_t lba, unsigned int len, int acc_num)
 {
 	sector_t end = lba + len;
+	u8 *fsp = fake_store_a[acc_num % 2];
 
 	while (lba < end) {
 		unsigned long index = lba_to_map_index(lba);
@@ -2950,8 +2976,7 @@ static void unmap_region(sector_t lba, unsigned int len)
 		    index < map_size) {
 			clear_bit(index, map_storep);
 			if (sdebug_lbprz) {  /* for LBPRZ=2 return 0xff_s */
-				memset(fake_storep +
-				       lba * sdebug_sector_size,
+				memset(fsp + lba * sdebug_sector_size,
 				       (sdebug_lbprz & 1) ? 0 : 0xff,
 				       sdebug_sector_size *
 				       sdebug_unmap_granularity);
@@ -2968,13 +2993,14 @@ static void unmap_region(sector_t lba, unsigned int len)
 
 static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 {
-	u8 *cmd = scp->cmnd;
-	u64 lba;
+	bool check_prot;
 	u32 num;
 	u32 ei_lba;
-	unsigned long iflags;
+	int acc_num = scp2acc_num(scp);
 	int ret;
-	bool check_prot;
+	unsigned long iflags;
+	u64 lba;
+	u8 *cmd = scp->cmnd;
 
 	switch (cmd[0]) {
 	case WRITE_16:
@@ -3030,14 +3056,15 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 	ret = check_device_access_params(scp, lba, num, true);
 	if (ret)
 		return ret;
-	write_lock_irqsave(&atomic_rw, iflags);
+	write_lock_irqsave(ramdisk_lck_a[acc_num % 2], iflags);
 
 	/* DIX + T10 DIF */
 	if (unlikely(sdebug_dix && scsi_prot_sg_count(scp))) {
 		int prot_ret = prot_verify_write(scp, lba, num, ei_lba);
 
 		if (prot_ret) {
-			write_unlock_irqrestore(&atomic_rw, iflags);
+			write_unlock_irqrestore(ramdisk_lck_a[acc_num % 2],
+						iflags);
 			mk_sense_buffer(scp, ILLEGAL_REQUEST, 0x10, prot_ret);
 			return illegal_condition_result;
 		}
@@ -3046,7 +3073,7 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 	ret = do_device_access(scp, 0, lba, num, true);
 	if (unlikely(scsi_debug_lbp()))
 		map_region(lba, num);
-	write_unlock_irqrestore(&atomic_rw, iflags);
+	write_unlock_irqrestore(ramdisk_lck_a[acc_num % 2], iflags);
 	if (unlikely(-1 == ret))
 		return DID_ERROR << 16;
 	else if (unlikely(sdebug_verbose &&
@@ -3092,6 +3119,7 @@ static int resp_write_scat(struct scsi_cmnd *scp,
 	u32 num, num_by, bt_len, lbdof_blen, sg_off, cum_lb;
 	u32 lb_size = sdebug_sector_size;
 	u32 ei_lba;
+	int acc_num = scp2acc_num(scp);
 	u64 lba;
 	unsigned long iflags;
 	int ret, res;
@@ -3155,7 +3183,7 @@ static int resp_write_scat(struct scsi_cmnd *scp,
 		goto err_out;
 	}
 
-	write_lock_irqsave(&atomic_rw, iflags);
+	write_lock_irqsave(ramdisk_lck_a[acc_num % 2], iflags);
 	sg_off = lbdof_blen;
 	/* Spec says Buffer xfer Length field in number of LBs in dout */
 	cum_lb = 0;
@@ -3238,7 +3266,7 @@ static int resp_write_scat(struct scsi_cmnd *scp,
 	}
 	ret = 0;
 err_out_unlock:
-	write_unlock_irqrestore(&atomic_rw, iflags);
+	write_unlock_irqrestore(ramdisk_lck_a[acc_num % 2], iflags);
 err_out:
 	kfree(lrdp);
 	return ret;
@@ -3247,27 +3275,30 @@ static int resp_write_scat(struct scsi_cmnd *scp,
 static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num,
 			   u32 ei_lba, bool unmap, bool ndob)
 {
-	int ret;
 	unsigned long iflags;
 	unsigned long long i;
-	u32 lb_size = sdebug_sector_size;
 	u64 block, lbaa;
+	u32 lb_size = sdebug_sector_size;
+	int ret;
+	int acc_num = scp2acc_num(scp);
 	u8 *fs1p;
+	u8 *fsp;
 
 	ret = check_device_access_params(scp, lba, num, true);
 	if (ret)
 		return ret;
 
-	write_lock_irqsave(&atomic_rw, iflags);
+	write_lock_irqsave(ramdisk_lck_a[acc_num % 2], iflags);
 
 	if (unmap && scsi_debug_lbp()) {
-		unmap_region(lba, num);
+		unmap_region(lba, num, acc_num);
 		goto out;
 	}
 	lbaa = lba;
 	block = do_div(lbaa, sdebug_store_sectors);
 	/* if ndob then zero 1 logical block, else fetch 1 logical block */
-	fs1p = fake_storep + (block * lb_size);
+	fsp = fake_store_a[acc_num % 2];
+	fs1p = fsp + (block * lb_size);
 	if (ndob) {
 		memset(fs1p, 0, lb_size);
 		ret = 0;
@@ -3275,7 +3306,7 @@ static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num,
 		ret = fetch_to_dev_buffer(scp, fs1p, lb_size);
 
 	if (-1 == ret) {
-		write_unlock_irqrestore(&atomic_rw, iflags);
+		write_unlock_irqrestore(ramdisk_lck_a[acc_num % 2], iflags);
 		return DID_ERROR << 16;
 	} else if (sdebug_verbose && !ndob && (ret < lb_size))
 		sdev_printk(KERN_INFO, scp->device,
@@ -3286,12 +3317,12 @@ static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num,
 	for (i = 1 ; i < num ; i++) {
 		lbaa = lba + i;
 		block = do_div(lbaa, sdebug_store_sectors);
-		memmove(fake_storep + (block * lb_size), fs1p, lb_size);
+		memmove(fsp + (block * lb_size), fs1p, lb_size);
 	}
 	if (scsi_debug_lbp())
 		map_region(lba, num);
 out:
-	write_unlock_irqrestore(&atomic_rw, iflags);
+	write_unlock_irqrestore(ramdisk_lck_a[acc_num % 2], iflags);
 
 	return 0;
 }
@@ -3403,7 +3434,8 @@ static int resp_comp_write(struct scsi_cmnd *scp,
 {
 	u8 *cmd = scp->cmnd;
 	u8 *arr;
-	u8 *fake_storep_hold;
+	u8 **fspp;
+	u8 *fsp_hold;
 	u64 lba;
 	u32 dnum;
 	u32 lb_size = sdebug_sector_size;
@@ -3411,6 +3443,7 @@ static int resp_comp_write(struct scsi_cmnd *scp,
 	unsigned long iflags;
 	int ret;
 	int retval = 0;
+	int acc_num = scp2acc_num(scp);
 
 	lba = get_unaligned_be64(cmd + 2);
 	num = cmd[13];		/* 1 to a maximum of 255 logical blocks */
@@ -3437,14 +3470,15 @@ static int resp_comp_write(struct scsi_cmnd *scp,
 		return check_condition_result;
 	}
 
-	write_lock_irqsave(&atomic_rw, iflags);
+	write_lock_irqsave(ramdisk_lck_a[acc_num % 2], iflags);
 
 	/* trick do_device_access() to fetch both compare and write buffers
 	 * from data-in into arr. Safe (atomic) since write_lock held. */
-	fake_storep_hold = fake_storep;
-	fake_storep = arr;
+	fspp = &fake_store_a[scp2acc_num(scp) % 2];
+	fsp_hold = *fspp;
+	*fspp = arr;
 	ret = do_device_access(scp, 0, 0, dnum, true);
-	fake_storep = fake_storep_hold;
+	*fspp = fsp_hold;
 	if (ret == -1) {
 		retval = DID_ERROR << 16;
 		goto cleanup;
@@ -3452,7 +3486,7 @@ static int resp_comp_write(struct scsi_cmnd *scp,
 		sdev_printk(KERN_INFO, scp->device, "%s: compare_write: cdb "
 			    "indicated=%u, IO sent=%d bytes\n", my_name,
 			    dnum * lb_size, ret);
-	if (!comp_write_worker(lba, num, arr)) {
+	if (!comp_write_worker(lba, num, arr, scp2acc_num(scp))) {
 		mk_sense_buffer(scp, MISCOMPARE, MISCOMPARE_VERIFY_ASC, 0);
 		retval = check_condition_result;
 		goto cleanup;
@@ -3460,7 +3494,7 @@ static int resp_comp_write(struct scsi_cmnd *scp,
 	if (scsi_debug_lbp())
 		map_region(lba, num);
 cleanup:
-	write_unlock_irqrestore(&atomic_rw, iflags);
+	write_unlock_irqrestore(ramdisk_lck_a[acc_num % 2], iflags);
 	kfree(arr);
 	return retval;
 }
@@ -3477,6 +3511,7 @@ static int resp_unmap(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 	struct unmap_block_desc *desc;
 	unsigned int i, payload_len, descriptors;
 	int ret;
+	int acc_num = scp2acc_num(scp);
 	unsigned long iflags;
 
 
@@ -3505,7 +3540,7 @@ static int resp_unmap(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 
 	desc = (void *)&buf[8];
 
-	write_lock_irqsave(&atomic_rw, iflags);
+	write_lock_irqsave(ramdisk_lck_a[acc_num % 2], iflags);
 
 	for (i = 0 ; i < descriptors ; i++) {
 		unsigned long long lba = get_unaligned_be64(&desc[i].lba);
@@ -3515,13 +3550,13 @@ static int resp_unmap(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 		if (ret)
 			goto out;
 
-		unmap_region(lba, num);
+		unmap_region(lba, num, scp2acc_num(scp));
 	}
 
 	ret = 0;
 
 out:
-	write_unlock_irqrestore(&atomic_rw, iflags);
+	write_unlock_irqrestore(ramdisk_lck_a[acc_num % 2], iflags);
 	kfree(buf);
 
 	return ret;
@@ -3891,6 +3926,7 @@ static int scsi_debug_slave_configure(struct scsi_device *sdp)
 	if (sdebug_no_uld)
 		sdp->no_uld_attach = 1;
 	config_cdb_len(sdp);
+	devip->sdg_devnum = atomic_inc_return(&a_sdg_devnum);
 	return 0;
 }
 
@@ -4146,8 +4182,7 @@ static int scsi_debug_host_reset(struct scsi_cmnd *SCpnt)
 	return SUCCESS;
 }
 
-static void __init sdebug_build_parts(unsigned char *ramp,
-				      unsigned long store_size)
+static void sdebug_build_parts(unsigned char *ramp, unsigned long store_size)
 {
 	struct partition *pp;
 	int starts[SDEBUG_MAX_PARTS + 2];
@@ -4436,6 +4471,7 @@ 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(doublestore, sdebug_doublestore, bool, S_IRUGO | S_IWUSR);
 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);
@@ -4499,6 +4535,7 @@ 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(doublestore, "If set, 2 data buffers allocated, devices alternate between the two");
 MODULE_PARM_DESC(every_nth, "timeout every nth command(def=0)");
 MODULE_PARM_DESC(fake_rw, "fake reads/writes instead of copying (def=0)");
 MODULE_PARM_DESC(guard, "protection checksum: 0=crc, 1=ip (def=0)");
@@ -4788,16 +4825,22 @@ static ssize_t fake_rw_store(struct device_driver *ddp, const char *buf,
 		n = (n > 0);
 		sdebug_fake_rw = (sdebug_fake_rw > 0);
 		if (sdebug_fake_rw != n) {
-			if ((0 == n) && (NULL == fake_storep)) {
-				unsigned long sz =
-					(unsigned long)sdebug_dev_size_mb *
-					1048576;
-
-				fake_storep = vzalloc(sz);
-				if (NULL == fake_storep) {
-					pr_err("out of memory, 9\n");
+			unsigned long sz = (unsigned long)sdebug_dev_size_mb *
+					   1048576;
+
+			if (n == 0 && !fake_store_a[0]) {
+				fake_store_a[0] = vzalloc(sz);
+				if (!fake_store_a[0])
 					return -ENOMEM;
-				}
+				if (sdebug_num_parts > 0)
+					sdebug_build_parts(fake_store_a[0], sz);
+			}
+			if (sdebug_doublestore && n == 0 && !fake_store_a[1]) {
+				fake_store_a[1] = vzalloc(sz);
+				if (!fake_store_a[1])
+					return -ENOMEM;
+				if (sdebug_num_parts > 0)
+					sdebug_build_parts(fake_store_a[1], sz);
 			}
 			sdebug_fake_rw = n;
 		}
@@ -4848,6 +4891,47 @@ static ssize_t dev_size_mb_show(struct device_driver *ddp, char *buf)
 }
 static DRIVER_ATTR_RO(dev_size_mb);
 
+static ssize_t doublestore_show(struct device_driver *ddp, char *buf)
+{
+	return scnprintf(buf, PAGE_SIZE, "%d\n", (int)sdebug_doublestore);
+}
+
+static ssize_t doublestore_store(struct device_driver *ddp, const char *buf,
+				 size_t count)
+{
+	int n;
+
+	if (count > 0 && kstrtoint(buf, 10, &n) == 0 && n >= 0) {
+		unsigned long iflags;
+
+		if (sdebug_doublestore == (n > 0))
+			return count;	/* no state change */
+		if (n <= 0) {
+			write_lock_irqsave(ramdisk_lck_a[1], iflags);
+			sdebug_doublestore = false;
+			vfree(fake_store_a[1]);
+			fake_store_a[1] = NULL;
+			write_unlock_irqrestore(ramdisk_lck_a[1], iflags);
+		} else {
+			unsigned long sz = (unsigned long)sdebug_dev_size_mb *
+					   1048576;
+			u8 *fsp = vzalloc(sz);
+
+			if (!fsp)
+				return -ENOMEM;
+			if (sdebug_num_parts > 0)
+				sdebug_build_parts(fsp, sz);
+			write_lock_irqsave(ramdisk_lck_a[1], iflags);
+			fake_store_a[1] = fsp;
+			sdebug_doublestore = true;
+			write_unlock_irqrestore(ramdisk_lck_a[1], iflags);
+		}
+		return count;
+	}
+	return -EINVAL;
+}
+static DRIVER_ATTR_RW(doublestore);
+
 static ssize_t num_parts_show(struct device_driver *ddp, char *buf)
 {
 	return scnprintf(buf, PAGE_SIZE, "%d\n", sdebug_num_parts);
@@ -5225,6 +5309,7 @@ static struct attribute *sdebug_drv_attrs[] = {
 	&driver_attr_opts.attr,
 	&driver_attr_ptype.attr,
 	&driver_attr_dsense.attr,
+	&driver_attr_doublestore.attr,
 	&driver_attr_fake_rw.attr,
 	&driver_attr_no_lun_0.attr,
 	&driver_attr_num_tgts.attr,
@@ -5266,7 +5351,10 @@ static int __init scsi_debug_init(void)
 	int k;
 	int ret;
 
+	ramdisk_lck_a[0] = &atomic_rw;
+	ramdisk_lck_a[1] = &atomic_rw2;
 	atomic_set(&retired_max_queue, 0);
+	atomic_set(&a_sdg_devnum, 0);
 
 	if (sdebug_ndelay >= 1000 * 1000 * 1000) {
 		pr_warn("ndelay must be less than 1 second, ignored\n");
@@ -5363,14 +5451,22 @@ static int __init scsi_debug_init(void)
 	}
 
 	if (sdebug_fake_rw == 0) {
-		fake_storep = vzalloc(sz);
-		if (NULL == fake_storep) {
-			pr_err("out of memory, 1\n");
+		fake_store_a[0] = vzalloc(sz);
+		if (!fake_store_a[0]) {
 			ret = -ENOMEM;
 			goto free_q_arr;
 		}
 		if (sdebug_num_parts > 0)
-			sdebug_build_parts(fake_storep, sz);
+			sdebug_build_parts(fake_store_a[0], sz);
+		if (sdebug_doublestore) {
+			fake_store_a[1] = vzalloc(sz);
+			if (!fake_store_a[1]) {
+				ret = -ENOMEM;
+				goto free_q_arr;
+			}
+			if (sdebug_num_parts > 0)
+				sdebug_build_parts(fake_store_a[1], sz);
+		}
 	}
 
 	if (sdebug_dix) {
@@ -5467,7 +5563,8 @@ static int __init scsi_debug_init(void)
 free_vm:
 	vfree(map_storep);
 	vfree(dif_storep);
-	vfree(fake_storep);
+	vfree(fake_store_a[1]);
+	vfree(fake_store_a[0]);
 free_q_arr:
 	kfree(sdebug_q_arr);
 	return ret;
@@ -5487,7 +5584,8 @@ static void __exit scsi_debug_exit(void)
 
 	vfree(map_storep);
 	vfree(dif_storep);
-	vfree(fake_storep);
+	vfree(fake_store_a[1]);
+	vfree(fake_store_a[0]);
 	kfree(sdebug_q_arr);
 }
 
-- 
2.24.1

[RFC 3/6] scsi_debug: implement verify(10), add verify(16)

[Douglas Gilbert]

With the addition of the doublestore option, the ability to check
whether the two different ramdisk images are the same or not
becomes useful. Prior to this patch VERIFY(10) always returned
true (i.e. the SCSI GOOD status) without checking. This option
adds support for BYTCHK equal to 1 and 3 . If the comparison
fails then a sense key of MISCOMPARE is returned as per the
T10 standards. Add support for the VERIFY(16).

Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
---
 drivers/scsi/scsi_debug.c | 118 ++++++++++++++++++++++++++++++++++----
 1 file changed, 107 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 45934dae8617..5d9dc9bdd1a7 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -342,7 +342,7 @@ enum sdeb_opcode_index {
 	SDEB_I_SERV_ACT_OUT_16 = 13,	/* add ...SERV_ACT_OUT_12 if needed */
 	SDEB_I_MAINT_IN = 14,
 	SDEB_I_MAINT_OUT = 15,
-	SDEB_I_VERIFY = 16,		/* 10 only */
+	SDEB_I_VERIFY = 16,		/* VERIFY(10), VERIFY(16) */
 	SDEB_I_VARIABLE_LEN = 17,	/* READ(32), WRITE(32), WR_SCAT(32) */
 	SDEB_I_RESERVE = 18,		/* 6, 10 */
 	SDEB_I_RELEASE = 19,		/* 6, 10 */
@@ -385,7 +385,8 @@ static const unsigned char opcode_ind_arr[256] = {
 	0, SDEB_I_VARIABLE_LEN,
 /* 0x80; 0x80->0x9f: 16 byte cdbs */
 	0, 0, 0, 0, 0, SDEB_I_ATA_PT, 0, 0,
-	SDEB_I_READ, SDEB_I_COMP_WRITE, SDEB_I_WRITE, 0, 0, 0, 0, 0,
+	SDEB_I_READ, SDEB_I_COMP_WRITE, SDEB_I_WRITE, 0,
+	0, 0, 0, SDEB_I_VERIFY,
 	0, SDEB_I_SYNC_CACHE, 0, SDEB_I_WRITE_SAME, 0, 0, 0, 0,
 	0, 0, 0, 0, 0, 0, SDEB_I_SERV_ACT_IN_16, SDEB_I_SERV_ACT_OUT_16,
 /* 0xa0; 0xa0->0xbf: 12 byte cdbs */
@@ -427,6 +428,7 @@ static int resp_report_tgtpgs(struct scsi_cmnd *, struct sdebug_dev_info *);
 static int resp_unmap(struct scsi_cmnd *, struct sdebug_dev_info *);
 static int resp_rsup_opcodes(struct scsi_cmnd *, struct sdebug_dev_info *);
 static int resp_rsup_tmfs(struct scsi_cmnd *, struct sdebug_dev_info *);
+static int resp_verify(struct scsi_cmnd *, struct sdebug_dev_info *);
 static int resp_write_same_10(struct scsi_cmnd *, struct sdebug_dev_info *);
 static int resp_write_same_16(struct scsi_cmnd *, struct sdebug_dev_info *);
 static int resp_comp_write(struct scsi_cmnd *, struct sdebug_dev_info *);
@@ -471,6 +473,12 @@ static const struct opcode_info_t write_iarr[] = {
 		   0xbf, 0xc7, 0, 0, 0, 0} },
 };
 
+static const struct opcode_info_t verify_iarr[] = {
+	{0, 0x2f, 0, F_D_OUT_MAYBE | FF_MEDIA_IO, resp_verify,/* VERIFY(10) */
+	    NULL, {10,  0xf7, 0xff, 0xff, 0xff, 0xff, 0xbf, 0xff, 0xff, 0xc7,
+		   0, 0, 0, 0, 0, 0} },
+};
+
 static const struct opcode_info_t sa_in_16_iarr[] = {
 	{0, 0x9e, 0x12, F_SA_LOW | F_D_IN, resp_get_lba_status, NULL,
 	    {16,  0x12, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
@@ -571,9 +579,10 @@ static const struct opcode_info_t opcode_info_arr[SDEB_I_LAST_ELEMENT + 1] = {
 /* 15 */
 	{0, 0, 0, F_INV_OP | FF_RESPOND, NULL, NULL, /* MAINT OUT */
 	    {0,  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} },
-	{0, 0x2f, 0, F_D_OUT_MAYBE | FF_MEDIA_IO, NULL, NULL, /* VERIFY(10) */
-	    {10,  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xc7,
-	     0, 0, 0, 0, 0, 0} },
+	{ARRAY_SIZE(verify_iarr), 0x8f, 0,
+	    F_D_OUT_MAYBE | FF_MEDIA_IO, resp_verify,	/* VERIFY(16) */
+	    verify_iarr, {16,  0xf6, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+			  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x3f, 0xc7} },
 	{ARRAY_SIZE(vl_iarr), 0x7f, 0x9, F_SA_HIGH | F_D_IN | FF_MEDIA_IO,
 	    resp_read_dt0, vl_iarr,	/* VARIABLE LENGTH, READ(32) */
 	    {32,  0xc7, 0, 0, 0, 0, 0x3f, 0x18, 0x0, 0x9, 0xfe, 0, 0xff, 0xff,
@@ -2543,7 +2552,8 @@ static int do_device_access(struct scsi_cmnd *scmd, u32 sg_skip, u64 lba,
 /* If lba2fake_store(lba,num) compares equal to arr(num), then copy top half of
  * arr into lba2fake_store(lba,num) and return true. If comparison fails then
  * return false. */
-static bool comp_write_worker(u64 lba, u32 num, const u8 *arr, int acc_num)
+static bool comp_write_worker(u64 lba, u32 num, const u8 *arr, int acc_num,
+			      bool compare_only)
 {
 	bool res;
 	u64 block, rest = 0;
@@ -2565,6 +2575,8 @@ static bool comp_write_worker(u64 lba, u32 num, const u8 *arr, int acc_num)
 			     rest * lb_size);
 	if (!res)
 		return res;
+	if (compare_only)
+		return true;
 	arr += num * lb_size;
 	memcpy(fsp + (block * lb_size), arr, (num - rest) * lb_size);
 	if (rest)
@@ -3472,9 +3484,11 @@ static int resp_comp_write(struct scsi_cmnd *scp,
 
 	write_lock_irqsave(ramdisk_lck_a[acc_num % 2], iflags);
 
-	/* trick do_device_access() to fetch both compare and write buffers
-	 * from data-in into arr. Safe (atomic) since write_lock held. */
-	fspp = &fake_store_a[scp2acc_num(scp) % 2];
+	/*
+	 * Trick do_device_access() to fetch both compare and write buffers
+	 * from data-out into arr. Safe (atomic) since write_lock held.
+	 */
+	fspp = &fake_store_a[acc_num % 2];
 	fsp_hold = *fspp;
 	*fspp = arr;
 	ret = do_device_access(scp, 0, 0, dnum, true);
@@ -3486,7 +3500,7 @@ static int resp_comp_write(struct scsi_cmnd *scp,
 		sdev_printk(KERN_INFO, scp->device, "%s: compare_write: cdb "
 			    "indicated=%u, IO sent=%d bytes\n", my_name,
 			    dnum * lb_size, ret);
-	if (!comp_write_worker(lba, num, arr, scp2acc_num(scp))) {
+	if (!comp_write_worker(lba, num, arr, acc_num, false)) {
 		mk_sense_buffer(scp, MISCOMPARE, MISCOMPARE_VERIFY_ASC, 0);
 		retval = check_condition_result;
 		goto cleanup;
@@ -3550,7 +3564,7 @@ static int resp_unmap(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 		if (ret)
 			goto out;
 
-		unmap_region(lba, num, scp2acc_num(scp));
+		unmap_region(lba, num, acc_num);
 	}
 
 	ret = 0;
@@ -3731,6 +3745,88 @@ static int resp_report_luns(struct scsi_cmnd *scp,
 	return res;
 }
 
+static int resp_verify(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
+{
+	bool is_bytchk3 = false;
+	u8 bytchk;
+	int ret, j;
+	int acc_num = scp2acc_num(scp);
+	u32 vnum, a_num, off;
+	const u32 lb_size = sdebug_sector_size;
+	unsigned long iflags;
+	u64 lba;
+	u8 *arr;
+	u8 **fspp;
+	u8 *fsp_hold;
+	u8 *cmd = scp->cmnd;
+
+	bytchk = (cmd[1] >> 1) & 0x3;
+	if (bytchk == 0) {
+		return 0;	/* always claim internal verify okay */
+	} else if (bytchk == 2) {
+		mk_sense_invalid_fld(scp, SDEB_IN_CDB, 2, 2);
+		return check_condition_result;
+	} else if (bytchk == 3) {
+		is_bytchk3 = true;	/* 1 block sent, compared repeatedly */
+	}
+	switch (cmd[0]) {
+	case VERIFY_16:
+		lba = get_unaligned_be64(cmd + 2);
+		vnum = get_unaligned_be32(cmd + 10);
+		break;
+	case VERIFY:		/* is VERIFY(10) */
+		lba = get_unaligned_be32(cmd + 2);
+		vnum = get_unaligned_be16(cmd + 7);
+		break;
+	default:
+		mk_sense_invalid_opcode(scp);
+		return check_condition_result;
+	}
+	a_num = is_bytchk3 ? 1 : vnum;
+	/* Treat following check like one for read (i.e. no write) access */
+	ret = check_device_access_params(scp, lba, a_num, false);
+	if (ret)
+		return ret;
+
+	arr = kcalloc(lb_size, vnum, GFP_ATOMIC);
+	if (!arr) {
+		mk_sense_buffer(scp, ILLEGAL_REQUEST, INSUFF_RES_ASC,
+				INSUFF_RES_ASCQ);
+		return check_condition_result;
+	}
+	/* Not changing store, so only need read access */
+	read_lock_irqsave(ramdisk_lck_a[acc_num % 2], iflags);
+
+	/* trick do_device_access() to fetch data-out into arr. */
+	fspp = &fake_store_a[acc_num % 2];
+	fsp_hold = *fspp;
+	*fspp = arr;
+	ret = do_device_access(scp, 0, 0, a_num, true);
+	*fspp = fsp_hold;
+	if (ret == -1) {
+		ret = DID_ERROR << 16;
+		goto cleanup;
+	} else if (sdebug_verbose && (ret < (a_num * lb_size))) {
+		sdev_printk(KERN_INFO, scp->device,
+			    "%s: %s: cdb indicated=%u, IO sent=%d bytes\n",
+			    my_name, __func__, a_num * lb_size, ret);
+	}
+	if (is_bytchk3) {
+		for (j = 1, off = lb_size; j < vnum; ++j, off += lb_size)
+			memcpy(arr + off, arr, lb_size);
+	}
+	ret = 0;
+	if (!comp_write_worker(lba, vnum, arr, acc_num, true)) {
+		mk_sense_buffer(scp, MISCOMPARE, MISCOMPARE_VERIFY_ASC, 0);
+		ret = check_condition_result;
+		goto cleanup;
+	}
+cleanup:
+	read_unlock_irqrestore(ramdisk_lck_a[acc_num % 2], iflags);
+	kfree(arr);
+	return ret;
+}
+
 static struct sdebug_queue *get_queue(struct scsi_cmnd *cmnd)
 {
 	u32 tag = blk_mq_unique_tag(cmnd->request);
-- 
2.24.1

[RFC 4/6] scsi_debug: weaken rwlock around ramdisk access

[Douglas Gilbert]

The design of this driver is to do any ramdisk access on the
same thread that invoked the queuecommand() call. That is
assumed to be user space context. The command
duration is implemented by setting the delay with a high
resolution timer. The hr timer's callback may well be in
interrupt context, but it doesn't touch the ramdisk. So try
removing the _irqsave()/_irqrestore() portion on the read-
write lock that protects ramdisk access.

Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
---
 drivers/scsi/scsi_debug.c | 59 +++++++++++++++------------------------
 1 file changed, 22 insertions(+), 37 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 5d9dc9bdd1a7..a3cb0bc29f81 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -2703,7 +2703,6 @@ static int resp_read_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 	u32 num;
 	u32 ei_lba;
 	int acc_num = scp2acc_num(scp);
-	unsigned long iflags;
 	u64 lba;
 	int ret;
 	bool check_prot;
@@ -2788,22 +2787,21 @@ static int resp_read_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 		return check_condition_result;
 	}
 
-	read_lock_irqsave(ramdisk_lck_a[acc_num % 2], iflags);
+	read_lock(ramdisk_lck_a[acc_num % 2]);
 
 	/* DIX + T10 DIF */
 	if (unlikely(sdebug_dix && scsi_prot_sg_count(scp))) {
 		int prot_ret = prot_verify_read(scp, lba, num, ei_lba);
 
 		if (prot_ret) {
-			read_unlock_irqrestore(ramdisk_lck_a[acc_num % 2],
-					       iflags);
+			read_unlock(ramdisk_lck_a[acc_num % 2]);
 			mk_sense_buffer(scp, ABORTED_COMMAND, 0x10, prot_ret);
 			return illegal_condition_result;
 		}
 	}
 
 	ret = do_device_access(scp, 0, lba, num, false);
-	read_unlock_irqrestore(ramdisk_lck_a[acc_num % 2], iflags);
+	read_unlock(ramdisk_lck_a[acc_num % 2]);
 	if (unlikely(ret == -1))
 		return DID_ERROR << 16;
 
@@ -3010,7 +3008,6 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 	u32 ei_lba;
 	int acc_num = scp2acc_num(scp);
 	int ret;
-	unsigned long iflags;
 	u64 lba;
 	u8 *cmd = scp->cmnd;
 
@@ -3068,15 +3065,14 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 	ret = check_device_access_params(scp, lba, num, true);
 	if (ret)
 		return ret;
-	write_lock_irqsave(ramdisk_lck_a[acc_num % 2], iflags);
+	write_lock(ramdisk_lck_a[acc_num % 2]);
 
 	/* DIX + T10 DIF */
 	if (unlikely(sdebug_dix && scsi_prot_sg_count(scp))) {
 		int prot_ret = prot_verify_write(scp, lba, num, ei_lba);
 
 		if (prot_ret) {
-			write_unlock_irqrestore(ramdisk_lck_a[acc_num % 2],
-						iflags);
+			write_unlock(ramdisk_lck_a[acc_num % 2]);
 			mk_sense_buffer(scp, ILLEGAL_REQUEST, 0x10, prot_ret);
 			return illegal_condition_result;
 		}
@@ -3085,7 +3081,7 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 	ret = do_device_access(scp, 0, lba, num, true);
 	if (unlikely(scsi_debug_lbp()))
 		map_region(lba, num);
-	write_unlock_irqrestore(ramdisk_lck_a[acc_num % 2], iflags);
+	write_unlock(ramdisk_lck_a[acc_num % 2]);
 	if (unlikely(-1 == ret))
 		return DID_ERROR << 16;
 	else if (unlikely(sdebug_verbose &&
@@ -3133,7 +3129,6 @@ static int resp_write_scat(struct scsi_cmnd *scp,
 	u32 ei_lba;
 	int acc_num = scp2acc_num(scp);
 	u64 lba;
-	unsigned long iflags;
 	int ret, res;
 	bool is_16;
 	static const u32 lrd_size = 32; /* + parameter list header size */
@@ -3195,7 +3190,7 @@ static int resp_write_scat(struct scsi_cmnd *scp,
 		goto err_out;
 	}
 
-	write_lock_irqsave(ramdisk_lck_a[acc_num % 2], iflags);
+	write_lock(ramdisk_lck_a[acc_num % 2]);
 	sg_off = lbdof_blen;
 	/* Spec says Buffer xfer Length field in number of LBs in dout */
 	cum_lb = 0;
@@ -3278,7 +3273,7 @@ static int resp_write_scat(struct scsi_cmnd *scp,
 	}
 	ret = 0;
 err_out_unlock:
-	write_unlock_irqrestore(ramdisk_lck_a[acc_num % 2], iflags);
+	write_unlock(ramdisk_lck_a[acc_num % 2]);
 err_out:
 	kfree(lrdp);
 	return ret;
@@ -3287,7 +3282,6 @@ static int resp_write_scat(struct scsi_cmnd *scp,
 static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num,
 			   u32 ei_lba, bool unmap, bool ndob)
 {
-	unsigned long iflags;
 	unsigned long long i;
 	u64 block, lbaa;
 	u32 lb_size = sdebug_sector_size;
@@ -3300,7 +3294,7 @@ static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num,
 	if (ret)
 		return ret;
 
-	write_lock_irqsave(ramdisk_lck_a[acc_num % 2], iflags);
+	write_lock(ramdisk_lck_a[acc_num % 2]);
 
 	if (unmap && scsi_debug_lbp()) {
 		unmap_region(lba, num, acc_num);
@@ -3318,7 +3312,7 @@ static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num,
 		ret = fetch_to_dev_buffer(scp, fs1p, lb_size);
 
 	if (-1 == ret) {
-		write_unlock_irqrestore(ramdisk_lck_a[acc_num % 2], iflags);
+		write_unlock(ramdisk_lck_a[acc_num % 2]);
 		return DID_ERROR << 16;
 	} else if (sdebug_verbose && !ndob && (ret < lb_size))
 		sdev_printk(KERN_INFO, scp->device,
@@ -3334,7 +3328,7 @@ static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num,
 	if (scsi_debug_lbp())
 		map_region(lba, num);
 out:
-	write_unlock_irqrestore(ramdisk_lck_a[acc_num % 2], iflags);
+	write_unlock(ramdisk_lck_a[acc_num % 2]);
 
 	return 0;
 }
@@ -3452,7 +3446,6 @@ static int resp_comp_write(struct scsi_cmnd *scp,
 	u32 dnum;
 	u32 lb_size = sdebug_sector_size;
 	u8 num;
-	unsigned long iflags;
 	int ret;
 	int retval = 0;
 	int acc_num = scp2acc_num(scp);
@@ -3482,7 +3475,7 @@ static int resp_comp_write(struct scsi_cmnd *scp,
 		return check_condition_result;
 	}
 
-	write_lock_irqsave(ramdisk_lck_a[acc_num % 2], iflags);
+	write_lock(ramdisk_lck_a[acc_num % 2]);
 
 	/*
 	 * Trick do_device_access() to fetch both compare and write buffers
@@ -3508,7 +3501,7 @@ static int resp_comp_write(struct scsi_cmnd *scp,
 	if (scsi_debug_lbp())
 		map_region(lba, num);
 cleanup:
-	write_unlock_irqrestore(ramdisk_lck_a[acc_num % 2], iflags);
+	write_unlock(ramdisk_lck_a[acc_num % 2]);
 	kfree(arr);
 	return retval;
 }
@@ -3526,8 +3519,6 @@ static int resp_unmap(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 	unsigned int i, payload_len, descriptors;
 	int ret;
 	int acc_num = scp2acc_num(scp);
-	unsigned long iflags;
-
 
 	if (!scsi_debug_lbp())
 		return 0;	/* fib and say its done */
@@ -3554,7 +3545,7 @@ static int resp_unmap(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 
 	desc = (void *)&buf[8];
 
-	write_lock_irqsave(ramdisk_lck_a[acc_num % 2], iflags);
+	write_lock(ramdisk_lck_a[acc_num % 2]);
 
 	for (i = 0 ; i < descriptors ; i++) {
 		unsigned long long lba = get_unaligned_be64(&desc[i].lba);
@@ -3570,7 +3561,7 @@ static int resp_unmap(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 	ret = 0;
 
 out:
-	write_unlock_irqrestore(ramdisk_lck_a[acc_num % 2], iflags);
+	write_unlock(ramdisk_lck_a[acc_num % 2]);
 	kfree(buf);
 
 	return ret;
@@ -3753,7 +3744,6 @@ static int resp_verify(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 	int acc_num = scp2acc_num(scp);
 	u32 vnum, a_num, off;
 	const u32 lb_size = sdebug_sector_size;
-	unsigned long iflags;
 	u64 lba;
 	u8 *arr;
 	u8 **fspp;
@@ -3795,7 +3785,7 @@ static int resp_verify(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 		return check_condition_result;
 	}
 	/* Not changing store, so only need read access */
-	read_lock_irqsave(ramdisk_lck_a[acc_num % 2], iflags);
+	read_lock(ramdisk_lck_a[acc_num % 2]);
 
 	/* trick do_device_access() to fetch data-out into arr. */
 	fspp = &fake_store_a[acc_num % 2];
@@ -3822,7 +3812,7 @@ static int resp_verify(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 		goto cleanup;
 	}
 cleanup:
-	read_unlock_irqrestore(ramdisk_lck_a[acc_num % 2], iflags);
+	read_unlock(ramdisk_lck_a[acc_num % 2]);
 	kfree(arr);
 	return ret;
 }
@@ -4471,9 +4461,6 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
 
 	cmnd->result = pfp != NULL ? pfp(cmnd, devip) : 0;
 	if (cmnd->result & SDEG_RES_IMMED_MASK) {
-		/*
-		 * This is the F_DELAY_OVERR case. No delay.
-		 */
 		cmnd->result &= ~SDEG_RES_IMMED_MASK;
 		delta_jiff = ndelay = 0;
 	}
@@ -4998,16 +4985,14 @@ static ssize_t doublestore_store(struct device_driver *ddp, const char *buf,
 	int n;
 
 	if (count > 0 && kstrtoint(buf, 10, &n) == 0 && n >= 0) {
-		unsigned long iflags;
-
 		if (sdebug_doublestore == (n > 0))
 			return count;	/* no state change */
 		if (n <= 0) {
-			write_lock_irqsave(ramdisk_lck_a[1], iflags);
+			write_lock(ramdisk_lck_a[1]);
 			sdebug_doublestore = false;
 			vfree(fake_store_a[1]);
 			fake_store_a[1] = NULL;
-			write_unlock_irqrestore(ramdisk_lck_a[1], iflags);
+			write_unlock(ramdisk_lck_a[1]);
 		} else {
 			unsigned long sz = (unsigned long)sdebug_dev_size_mb *
 					   1048576;
@@ -5017,10 +5002,10 @@ static ssize_t doublestore_store(struct device_driver *ddp, const char *buf,
 				return -ENOMEM;
 			if (sdebug_num_parts > 0)
 				sdebug_build_parts(fsp, sz);
-			write_lock_irqsave(ramdisk_lck_a[1], iflags);
+			write_lock(ramdisk_lck_a[1]);
 			fake_store_a[1] = fsp;
 			sdebug_doublestore = true;
-			write_unlock_irqrestore(ramdisk_lck_a[1], iflags);
+			write_unlock(ramdisk_lck_a[1]);
 		}
 		return count;
 	}
@@ -5953,7 +5938,7 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,
 		pfp = r_pfp;    /* if leaf function ptr NULL, try the root's */
 
 fini:
-	if (F_DELAY_OVERR & flags)
+	if (F_DELAY_OVERR & flags)	/* cmds like INQUIRY respond asap */
 		return schedule_resp(scp, devip, errsts, pfp, 0, 0);
 	else if ((flags & F_LONG_DELAY) && (sdebug_jdelay > 0 ||
 					    sdebug_ndelay > 10000)) {
-- 
2.24.1

[RFC 5/6] scsi_debug: improve command duration calculation

[Douglas Gilbert]

Previously the code did the work implied by the given SCSI
command and after that it waited for a timer based on the user
specified command duration to be exhausted before informing
the mid-level that the command was complete. For short command
durations the time to complete the work implied by the SCSI
command could be significant compared to the user specified
command duration.

For example a WRITE of 128 blocks (say 512 bytes each) on a
machine that can copy from main memory to main memory at a rate
of 10 GB/sec will take around 6.4 microseconds to do that copy.
If the user specified a command duration of 5 microseconds
(ndelay=5000) should the driver do a further delay of 5
microseconds after the copy or return immediately because
6.4 > 5 ?

The action prior to this patch was to always do the timer based
delay. After this patch, for ndelay values less than 1
millisecond, this driver will complete the command immediately.
And in the case where the user specified delay was 7
microseconds, a timer delay of 600 nanoseconds will be set
((7 - 6.4) * 1000).

Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
---
 drivers/scsi/scsi_debug.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index a3cb0bc29f81..f60decb06f59 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -4370,6 +4370,8 @@ static void setup_inject(struct sdebug_queue *sqp,
 	sqcp->inj_cmd_abort = !!(SDEBUG_OPT_CMD_ABORT & sdebug_opts);
 }
 
+#define INCLUSIVE_TIMING_MAX_NS 1000000		/* 1 millisecond */
+
 /* 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
@@ -4381,8 +4383,10 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
 				    struct sdebug_dev_info *),
 			 int delta_jiff, int ndelay)
 {
-	unsigned long iflags;
+	bool new_sd_dp;
 	int k, num_in_q, qdepth, inject;
+	unsigned long iflags;
+	u64 ns_from_boot = 0;
 	struct sdebug_queue *sqp;
 	struct sdebug_queued_cmd *sqcp;
 	struct scsi_device *sdp;
@@ -4398,7 +4402,6 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
 	if (delta_jiff == 0)
 		goto respond_in_thread;
 
-	/* schedule the response at a later time if resources permit */
 	sqp = get_queue(cmnd);
 	spin_lock_irqsave(&sqp->qc_lock, iflags);
 	if (unlikely(atomic_read(&sqp->blocked))) {
@@ -4457,8 +4460,15 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
 		sd_dp = kzalloc(sizeof(*sd_dp), GFP_ATOMIC);
 		if (sd_dp == NULL)
 			return SCSI_MLQUEUE_HOST_BUSY;
+		new_sd_dp = true;
+	} else {
+		new_sd_dp = false;
 	}
 
+	if (ndelay > 0 && ndelay < INCLUSIVE_TIMING_MAX_NS)
+		ns_from_boot = ktime_get_boottime_ns();
+
+	/* one of the resp_*() response functions is called here */
 	cmnd->result = pfp != NULL ? pfp(cmnd, devip) : 0;
 	if (cmnd->result & SDEG_RES_IMMED_MASK) {
 		cmnd->result &= ~SDEG_RES_IMMED_MASK;
@@ -4489,6 +4499,22 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
 		} else {	/* ndelay has a 4.2 second max */
 			kt = sdebug_random ? prandom_u32_max((u32)ndelay) :
 					     (u32)ndelay;
+			if (ndelay < INCLUSIVE_TIMING_MAX_NS) {
+				u64 d = ktime_get_boottime_ns() - ns_from_boot;
+
+				if (kt <= d) {	/* elapsed duration >= kt */
+					sqcp->a_cmnd = NULL;
+					atomic_dec(&devip->num_in_q);
+					clear_bit(k, sqp->in_use_bm);
+					if (new_sd_dp)
+						kfree(sd_dp);
+					/* call scsi_done() from this thread */
+					cmnd->scsi_done(cmnd);
+					return 0;
+				}
+				/* otherwise reduce kt by elapsed time */
+				kt -= d;
+			}
 		}
 		if (!sd_dp->init_hrt) {
 			sd_dp->init_hrt = true;
@@ -4502,6 +4528,7 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
 		if (sdebug_statistics)
 			sd_dp->issuing_cpu = raw_smp_processor_id();
 		sd_dp->defer_t = SDEB_DEFER_HRT;
+		/* schedule the invocation of scsi_done() for a later time */
 		hrtimer_start(&sd_dp->hrt, kt, HRTIMER_MODE_REL_PINNED);
 	} else {	/* jdelay < 0, use work queue */
 		if (!sd_dp->init_wq) {
-- 
2.24.1

[RFC 6/6] scsi_debug: bump to version 1.89

[Douglas Gilbert]

The scsi_debug driver version is visible in:
   /sys/modules/scsi_debug/version
and can thus be used by user space programs to alter the
features they try to use. Since the doublestore option is
a significant addition, bump the version number which
will appear as 0189 when the above file is read.

Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
---
 drivers/scsi/scsi_debug.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index f60decb06f59..032edef3abd4 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -7,7 +7,7 @@
  *  anything out of the ordinary is seen.
  * ^^^^^^^^^^^^^^^^^^^^^^^ Original ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  *
- * Copyright (C) 2001 - 2018 Douglas Gilbert
+ * Copyright (C) 2001 - 2019 Douglas Gilbert
  *
  *  For documentation see http://sg.danny.cz/sg/sdebug26.html
  */
@@ -57,8 +57,8 @@
 #include "scsi_logging.h"
 
 /* make sure inq_product_rev string corresponds to this version */
-#define SDEBUG_VERSION "0188"	/* format to fit INQUIRY revision field */
-static const char *sdebug_version_date = "20190125";
+#define SDEBUG_VERSION "0189"	/* format to fit INQUIRY revision field */
+static const char *sdebug_version_date = "20191221";
 
 #define MY_NAME "scsi_debug"
 
-- 
2.24.1

Re: [RFC 3/6] scsi_debug: implement verify(10), add verify(16)

[Douglas Gilbert]

See below, near the end of the patch.

On 2019-12-21 10:59 p.m., Douglas Gilbert wrote:
> With the addition of the doublestore option, the ability to check
> whether the two different ramdisk images are the same or not
> becomes useful. Prior to this patch VERIFY(10) always returned
> true (i.e. the SCSI GOOD status) without checking. This option
> adds support for BYTCHK equal to 1 and 3 . If the comparison
> fails then a sense key of MISCOMPARE is returned as per the
> T10 standards. Add support for the VERIFY(16).
> 
> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
> ---
>   drivers/scsi/scsi_debug.c | 118 ++++++++++++++++++++++++++++++++++----
>   1 file changed, 107 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 45934dae8617..5d9dc9bdd1a7 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -342,7 +342,7 @@ enum sdeb_opcode_index {
>   	SDEB_I_SERV_ACT_OUT_16 = 13,	/* add ...SERV_ACT_OUT_12 if needed */
>   	SDEB_I_MAINT_IN = 14,
>   	SDEB_I_MAINT_OUT = 15,
> -	SDEB_I_VERIFY = 16,		/* 10 only */
> +	SDEB_I_VERIFY = 16,		/* VERIFY(10), VERIFY(16) */
>   	SDEB_I_VARIABLE_LEN = 17,	/* READ(32), WRITE(32), WR_SCAT(32) */
>   	SDEB_I_RESERVE = 18,		/* 6, 10 */
>   	SDEB_I_RELEASE = 19,		/* 6, 10 */
> @@ -385,7 +385,8 @@ static const unsigned char opcode_ind_arr[256] = {
>   	0, SDEB_I_VARIABLE_LEN,
>   /* 0x80; 0x80->0x9f: 16 byte cdbs */
>   	0, 0, 0, 0, 0, SDEB_I_ATA_PT, 0, 0,
> -	SDEB_I_READ, SDEB_I_COMP_WRITE, SDEB_I_WRITE, 0, 0, 0, 0, 0,
> +	SDEB_I_READ, SDEB_I_COMP_WRITE, SDEB_I_WRITE, 0,
> +	0, 0, 0, SDEB_I_VERIFY,
>   	0, SDEB_I_SYNC_CACHE, 0, SDEB_I_WRITE_SAME, 0, 0, 0, 0,
>   	0, 0, 0, 0, 0, 0, SDEB_I_SERV_ACT_IN_16, SDEB_I_SERV_ACT_OUT_16,
>   /* 0xa0; 0xa0->0xbf: 12 byte cdbs */
> @@ -427,6 +428,7 @@ static int resp_report_tgtpgs(struct scsi_cmnd *, struct sdebug_dev_info *);
>   static int resp_unmap(struct scsi_cmnd *, struct sdebug_dev_info *);
>   static int resp_rsup_opcodes(struct scsi_cmnd *, struct sdebug_dev_info *);
>   static int resp_rsup_tmfs(struct scsi_cmnd *, struct sdebug_dev_info *);
> +static int resp_verify(struct scsi_cmnd *, struct sdebug_dev_info *);
>   static int resp_write_same_10(struct scsi_cmnd *, struct sdebug_dev_info *);
>   static int resp_write_same_16(struct scsi_cmnd *, struct sdebug_dev_info *);
>   static int resp_comp_write(struct scsi_cmnd *, struct sdebug_dev_info *);
> @@ -471,6 +473,12 @@ static const struct opcode_info_t write_iarr[] = {
>   		   0xbf, 0xc7, 0, 0, 0, 0} },
>   };
>   
> +static const struct opcode_info_t verify_iarr[] = {
> +	{0, 0x2f, 0, F_D_OUT_MAYBE | FF_MEDIA_IO, resp_verify,/* VERIFY(10) */
> +	    NULL, {10,  0xf7, 0xff, 0xff, 0xff, 0xff, 0xbf, 0xff, 0xff, 0xc7,
> +		   0, 0, 0, 0, 0, 0} },
> +};
> +
>   static const struct opcode_info_t sa_in_16_iarr[] = {
>   	{0, 0x9e, 0x12, F_SA_LOW | F_D_IN, resp_get_lba_status, NULL,
>   	    {16,  0x12, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> @@ -571,9 +579,10 @@ static const struct opcode_info_t opcode_info_arr[SDEB_I_LAST_ELEMENT + 1] = {
>   /* 15 */
>   	{0, 0, 0, F_INV_OP | FF_RESPOND, NULL, NULL, /* MAINT OUT */
>   	    {0,  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} },
> -	{0, 0x2f, 0, F_D_OUT_MAYBE | FF_MEDIA_IO, NULL, NULL, /* VERIFY(10) */
> -	    {10,  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xc7,
> -	     0, 0, 0, 0, 0, 0} },
> +	{ARRAY_SIZE(verify_iarr), 0x8f, 0,
> +	    F_D_OUT_MAYBE | FF_MEDIA_IO, resp_verify,	/* VERIFY(16) */
> +	    verify_iarr, {16,  0xf6, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> +			  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x3f, 0xc7} },
>   	{ARRAY_SIZE(vl_iarr), 0x7f, 0x9, F_SA_HIGH | F_D_IN | FF_MEDIA_IO,
>   	    resp_read_dt0, vl_iarr,	/* VARIABLE LENGTH, READ(32) */
>   	    {32,  0xc7, 0, 0, 0, 0, 0x3f, 0x18, 0x0, 0x9, 0xfe, 0, 0xff, 0xff,
> @@ -2543,7 +2552,8 @@ static int do_device_access(struct scsi_cmnd *scmd, u32 sg_skip, u64 lba,
>   /* If lba2fake_store(lba,num) compares equal to arr(num), then copy top half of
>    * arr into lba2fake_store(lba,num) and return true. If comparison fails then
>    * return false. */
> -static bool comp_write_worker(u64 lba, u32 num, const u8 *arr, int acc_num)
> +static bool comp_write_worker(u64 lba, u32 num, const u8 *arr, int acc_num,
> +			      bool compare_only)
>   {
>   	bool res;
>   	u64 block, rest = 0;
> @@ -2565,6 +2575,8 @@ static bool comp_write_worker(u64 lba, u32 num, const u8 *arr, int acc_num)
>   			     rest * lb_size);
>   	if (!res)
>   		return res;
> +	if (compare_only)
> +		return true;
>   	arr += num * lb_size;
>   	memcpy(fsp + (block * lb_size), arr, (num - rest) * lb_size);
>   	if (rest)
> @@ -3472,9 +3484,11 @@ static int resp_comp_write(struct scsi_cmnd *scp,
>   
>   	write_lock_irqsave(ramdisk_lck_a[acc_num % 2], iflags);
>   
> -	/* trick do_device_access() to fetch both compare and write buffers
> -	 * from data-in into arr. Safe (atomic) since write_lock held. */
> -	fspp = &fake_store_a[scp2acc_num(scp) % 2];
> +	/*
> +	 * Trick do_device_access() to fetch both compare and write buffers
> +	 * from data-out into arr. Safe (atomic) since write_lock held.
> +	 */
> +	fspp = &fake_store_a[acc_num % 2];
>   	fsp_hold = *fspp;
>   	*fspp = arr;
>   	ret = do_device_access(scp, 0, 0, dnum, true);
> @@ -3486,7 +3500,7 @@ static int resp_comp_write(struct scsi_cmnd *scp,
>   		sdev_printk(KERN_INFO, scp->device, "%s: compare_write: cdb "
>   			    "indicated=%u, IO sent=%d bytes\n", my_name,
>   			    dnum * lb_size, ret);
> -	if (!comp_write_worker(lba, num, arr, scp2acc_num(scp))) {
> +	if (!comp_write_worker(lba, num, arr, acc_num, false)) {
>   		mk_sense_buffer(scp, MISCOMPARE, MISCOMPARE_VERIFY_ASC, 0);
>   		retval = check_condition_result;
>   		goto cleanup;
> @@ -3550,7 +3564,7 @@ static int resp_unmap(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
>   		if (ret)
>   			goto out;
>   
> -		unmap_region(lba, num, scp2acc_num(scp));
> +		unmap_region(lba, num, acc_num);
>   	}
>   
>   	ret = 0;
> @@ -3731,6 +3745,88 @@ static int resp_report_luns(struct scsi_cmnd *scp,
>   	return res;
>   }
>   
> +static int resp_verify(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
> +{
> +	bool is_bytchk3 = false;
> +	u8 bytchk;
> +	int ret, j;
> +	int acc_num = scp2acc_num(scp);
> +	u32 vnum, a_num, off;
> +	const u32 lb_size = sdebug_sector_size;
> +	unsigned long iflags;
> +	u64 lba;
> +	u8 *arr;
> +	u8 **fspp;
> +	u8 *fsp_hold;
> +	u8 *cmd = scp->cmnd;
> +
> +	bytchk = (cmd[1] >> 1) & 0x3;
> +	if (bytchk == 0) {
> +		return 0;	/* always claim internal verify okay */
> +	} else if (bytchk == 2) {
> +		mk_sense_invalid_fld(scp, SDEB_IN_CDB, 2, 2);
> +		return check_condition_result;
> +	} else if (bytchk == 3) {
> +		is_bytchk3 = true;	/* 1 block sent, compared repeatedly */
> +	}
> +	switch (cmd[0]) {
> +	case VERIFY_16:
> +		lba = get_unaligned_be64(cmd + 2);
> +		vnum = get_unaligned_be32(cmd + 10);
> +		break;
> +	case VERIFY:		/* is VERIFY(10) */
> +		lba = get_unaligned_be32(cmd + 2);
> +		vnum = get_unaligned_be16(cmd + 7);
> +		break;
> +	default:
> +		mk_sense_invalid_opcode(scp);
> +		return check_condition_result;
> +	}
> +	a_num = is_bytchk3 ? 1 : vnum;
> +	/* Treat following check like one for read (i.e. no write) access */
> +	ret = check_device_access_params(scp, lba, a_num, false);
> +	if (ret)
> +		return ret;
> +
> +	arr = kcalloc(lb_size, vnum, GFP_ATOMIC);
> +	if (!arr) {
> +		mk_sense_buffer(scp, ILLEGAL_REQUEST, INSUFF_RES_ASC,
> +				INSUFF_RES_ASCQ);
> +		return check_condition_result;
> +	}
> +	/* Not changing store, so only need read access */
> +	read_lock_irqsave(ramdisk_lck_a[acc_num % 2], iflags);
> +
> +	/* trick do_device_access() to fetch data-out into arr. */
> +	fspp = &fake_store_a[acc_num % 2];
> +	fsp_hold = *fspp;
> +	*fspp = arr;
> +	ret = do_device_access(scp, 0, 0, a_num, true);
> +	*fspp = fsp_hold;

The above hack is copied from the compare_and_write response code where it
is safe (just) due to the write_lock. That is weakened to a read_lock here
and that leads to very ugly crashes when multiple threads are visiting
this code.

In version 2 of this code, introduce a do_dout_fetch() function in both
compare_and_write and verify responses. That way the hack disappears and so
does the ugly crash. Testing ongoing.

> +	if (ret == -1) {
> +		ret = DID_ERROR << 16;
> +		goto cleanup;
> +	} else if (sdebug_verbose && (ret < (a_num * lb_size))) {
> +		sdev_printk(KERN_INFO, scp->device,
> +			    "%s: %s: cdb indicated=%u, IO sent=%d bytes\n",
> +			    my_name, __func__, a_num * lb_size, ret);
> +	}
> +	if (is_bytchk3) {
> +		for (j = 1, off = lb_size; j < vnum; ++j, off += lb_size)
> +			memcpy(arr + off, arr, lb_size);
> +	}
> +	ret = 0;
> +	if (!comp_write_worker(lba, vnum, arr, acc_num, true)) {
> +		mk_sense_buffer(scp, MISCOMPARE, MISCOMPARE_VERIFY_ASC, 0);
> +		ret = check_condition_result;
> +		goto cleanup;
> +	}
> +cleanup:
> +	read_unlock_irqrestore(ramdisk_lck_a[acc_num % 2], iflags);
> +	kfree(arr);
> +	return ret;
> +}
> +
>   static struct sdebug_queue *get_queue(struct scsi_cmnd *cmnd)
>   {
>   	u32 tag = blk_mq_unique_tag(cmnd->request);
>