* [PATCH v2.5 0/6] scsi_debug: rebase second half of series
@ 2016-05-01 2:44 Douglas Gilbert
2016-05-01 2:44 ` [PATCH v2.5 1/6] scsi_debug: use pdt constants Douglas Gilbert
` (5 more replies)
0 siblings, 6 replies; 24+ messages in thread
From: Douglas Gilbert @ 2016-05-01 2:44 UTC (permalink / raw)
To: linux-scsi; +Cc: martin.petersen, tomas.winkler, emilne, bart.vanassche
Changes since original version:
- reduce resp_report_luns to reporting 256 LUNs (0 to 255)
using address_method=0 which is single level peripheral
device addressing method. Reviewer would like further
address_methods support which will be presented as a
separate patch
Primary reason for this patch series is to add multi queue support
modelled on the null_blk driver. Incorporate REPORT LUNS patch from
Tomas Winkler sent in Febrary 2015; trim to a maximum of 256 LUNs
(0 to 255) per target. Add parameter that permits LU names to use
UUIDs (spc5r08.pdf).
The first half of this series (1 through 7) have already been applied
to the maintainers' trees. That differed slightly from my v2 series
(after 7/12 was applied). So 1/6 of this series is a resync and the
next 5 patches in this series correspond to v2 8/12 through 12/12
(give or take a little fuzz).
Douglas Gilbert (6):
scsi_debug: use pdt constants
scsi_debug: rework resp_report_luns
scsi_debug: add multiple queue support
scsi_debug: vpd and mode page work
scsi_debug: uuid for lu name
scsi_debug: use locally assigned naa
drivers/scsi/scsi_debug.c | 1134 ++++++++++++++++++++++++++++-----------------
1 file changed, 710 insertions(+), 424 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2.5 1/6] scsi_debug: use pdt constants
2016-05-01 2:44 [PATCH v2.5 0/6] scsi_debug: rebase second half of series Douglas Gilbert
@ 2016-05-01 2:44 ` Douglas Gilbert
2016-05-02 8:25 ` Hannes Reinecke
2016-05-03 23:56 ` Bart Van Assche
2016-05-01 2:44 ` [PATCH v2.5 2/6] scsi_debug: rework resp_report_luns Douglas Gilbert
` (4 subsequent siblings)
5 siblings, 2 replies; 24+ messages in thread
From: Douglas Gilbert @ 2016-05-01 2:44 UTC (permalink / raw)
To: linux-scsi; +Cc: martin.petersen, tomas.winkler, emilne, bart.vanassche
Use TYPE_* constants for SCSI peripheral device types instead
of numbers. Further cleanups requested by checkpatch.pl .
Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
---
drivers/scsi/scsi_debug.c | 86 +++++++++++++++++++++++++----------------------
1 file changed, 45 insertions(+), 41 deletions(-)
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 6b2d006..fc0246c 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -59,8 +59,8 @@
#include "scsi_logging.h"
/* make sure inq_product_rev string corresponds to this version */
-#define SCSI_DEBUG_VERSION "1.86"
-static const char *sdebug_version_date = "20160422";
+#define SDEBUG_VERSION "1.86"
+static const char *sdebug_version_date = "20160430";
#define MY_NAME "scsi_debug"
@@ -123,7 +123,7 @@ static const char *sdebug_version_date = "20160422";
#define DEF_OPTS 0
#define DEF_OPT_BLKS 1024
#define DEF_PHYSBLK_EXP 0
-#define DEF_PTYPE 0
+#define DEF_PTYPE TYPE_DISK
#define DEF_REMOVABLE false
#define DEF_SCSI_LEVEL 6 /* INQUIRY, byte2 [6->SPC-4] */
#define DEF_SECTOR_SIZE 512
@@ -137,6 +137,8 @@ static const char *sdebug_version_date = "20160422";
#define DEF_STRICT 0
#define JDELAY_OVERRIDDEN -9999
+#define SDEBUG_LUN_0_VAL 0
+
/* bit mask values for sdebug_opts */
#define SDEBUG_OPT_NOISE 1
#define SDEBUG_OPT_MEDIUM_ERR 2
@@ -232,7 +234,7 @@ static const char *sdebug_version_date = "20160422";
#define SDEBUG_MAX_PARTS 4
-#define SCSI_DEBUG_MAX_CMD_LEN 32
+#define SDEBUG_MAX_CMD_LEN 32
struct sdebug_dev_info {
@@ -278,8 +280,8 @@ struct sdebug_scmd_extra_t {
};
struct opcode_info_t {
- u8 num_attached; /* 0 if this is it (i.e. a leaf); use 0xff
- * for terminating element */
+ u8 num_attached; /* 0 if this is it (i.e. a leaf); use 0xff */
+ /* for terminating element */
u8 opcode; /* if num_attached > 0, preferred */
u16 sa; /* service action */
u32 flags; /* OR-ed set of SDEB_F_* */
@@ -571,7 +573,7 @@ static int sdebug_num_tgts = DEF_NUM_TGTS; /* targets per host */
static int sdebug_opt_blks = DEF_OPT_BLKS;
static int sdebug_opts = DEF_OPTS;
static int sdebug_physblk_exp = DEF_PHYSBLK_EXP;
-static int sdebug_ptype = DEF_PTYPE; /* SCSI peripheral type (0==disk) */
+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;
static int sdebug_virtual_gb = DEF_VIRTUAL_GB;
@@ -649,7 +651,7 @@ static const int device_qfull_result =
(DID_OK << 16) | (COMMAND_COMPLETE << 8) | SAM_STAT_TASK_SET_FULL;
-static unsigned int scsi_debug_lbp(void)
+static inline unsigned int scsi_debug_lbp(void)
{
return 0 == sdebug_fake_rw &&
(sdebug_lbpu || sdebug_lbpws || sdebug_lbpws10);
@@ -825,7 +827,8 @@ static int make_ua(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
break;
case SDEBUG_UA_MICROCODE_CHANGED:
mk_sense_buffer(scp, UNIT_ATTENTION,
- TARGET_CHANGED_ASC, MICROCODE_CHANGED_ASCQ);
+ TARGET_CHANGED_ASC,
+ MICROCODE_CHANGED_ASCQ);
if (sdebug_verbose)
cp = "microcode has been changed";
break;
@@ -1225,11 +1228,11 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
arr = kzalloc(SDEBUG_MAX_INQ_ARR_SZ, GFP_ATOMIC);
if (! arr)
return DID_REQUEUE << 16;
- have_wlun = (scp->device->lun == SCSI_W_LUN_REPORT_LUNS);
+ have_wlun = scsi_is_wlun(scp->device->lun);
if (have_wlun)
- pq_pdt = 0x1e; /* present, wlun */
- else if (sdebug_no_lun_0 && (0 == devip->lun))
- pq_pdt = 0x7f; /* not present, no device type */
+ pq_pdt = TYPE_WLUN; /* present, wlun */
+ else if (sdebug_no_lun_0 && (devip->lun == SDEBUG_LUN_0_VAL))
+ pq_pdt = 0x7f; /* not present, PQ=3, PDT=0x1f */
else
pq_pdt = (sdebug_ptype & 0x1f);
arr[0] = pq_pdt;
@@ -1244,7 +1247,7 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
port_group_id = (((host_no + 1) & 0x7f) << 8) +
(devip->channel & 0x7f);
- if (0 == sdebug_vpd_use_hostno)
+ if (sdebug_vpd_use_hostno == 0)
host_no = 0;
lu_id_num = have_wlun ? -1 : (((host_no + 1) * 2000) +
(devip->target * 1000) + devip->lun);
@@ -1333,7 +1336,7 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
arr[3] = 2; /* response_data_format==2 */
arr[4] = SDEBUG_LONG_INQ_SZ - 5;
arr[5] = (int)have_dif_prot; /* PROTECT bit */
- if (0 == sdebug_vpd_use_hostno)
+ if (sdebug_vpd_use_hostno == 0)
arr[5] = 0x10; /* claim: implicit TGPS */
arr[6] = 0x10; /* claim: MultiP */
/* arr[6] |= 0x40; ... claim: EncServ (enclosure services) */
@@ -1345,9 +1348,9 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
arr[58] = 0x0; arr[59] = 0xa2; /* SAM-5 rev 4 */
arr[60] = 0x4; arr[61] = 0x68; /* SPC-4 rev 37 */
n = 62;
- if (sdebug_ptype == 0) {
+ if (sdebug_ptype == TYPE_DISK) {
arr[n++] = 0x4; arr[n++] = 0xc5; /* SBC-4 rev 36 */
- } else if (sdebug_ptype == 1) {
+ } else if (sdebug_ptype == TYPE_TAPE) {
arr[n++] = 0x5; arr[n++] = 0x25; /* SSC-4 rev 3 */
}
arr[n++] = 0x20; arr[n++] = 0xe6; /* SPL-3 rev 7 */
@@ -1534,7 +1537,7 @@ static int resp_report_tgtpgs(struct scsi_cmnd * scp,
* The asymmetric access state is cycled according to the host_id.
*/
n = 4;
- if (0 == sdebug_vpd_use_hostno) {
+ if (sdebug_vpd_use_hostno == 0) {
arr[n++] = host_no % 3; /* Asymm access state */
arr[n++] = 0x0F; /* claim: all states are supported */
} else {
@@ -1938,7 +1941,7 @@ static int resp_mode_sense(struct scsi_cmnd *scp,
subpcode = cmd[3];
msense_6 = (MODE_SENSE == cmd[0]);
llbaa = msense_6 ? 0 : !!(cmd[1] & 0x10);
- if ((0 == sdebug_ptype) && (0 == dbd))
+ if ((sdebug_ptype == TYPE_DISK) && (dbd == 0))
bd_len = llbaa ? 16 : 8;
else
bd_len = 0;
@@ -1950,9 +1953,9 @@ static int resp_mode_sense(struct scsi_cmnd *scp,
}
target_dev_id = ((devip->sdbg_host->shost->host_no + 1) * 2000) +
(devip->target * 1000) - 3;
- /* set DPOFUA bit for disks */
- if (0 == sdebug_ptype)
- dev_spec = 0x10; /* would be 0x90 if read-only */
+ /* for disks set DPOFUA bit and clear write protect (WP) bit */
+ if (sdebug_ptype == TYPE_DISK)
+ dev_spec = 0x10; /* =0x90 if WP=1 implies read-only */
else
dev_spec = 0x0;
if (msense_6) {
@@ -3345,7 +3348,7 @@ static void sdebug_q_cmd_complete(struct sdebug_defer *sd_dp)
spin_lock_irqsave(&queued_arr_lock, iflags);
sqcp = &queued_arr[qa_indx];
scp = sqcp->a_cmnd;
- if (unlikely(NULL == scp)) {
+ if (unlikely(scp == NULL)) {
spin_unlock_irqrestore(&queued_arr_lock, iflags);
pr_err("scp is NULL\n");
return;
@@ -3470,11 +3473,11 @@ static int scsi_debug_slave_configure(struct scsi_device *sdp)
if (sdebug_verbose)
pr_info("slave_configure <%u %u %u %llu>\n",
sdp->host->host_no, sdp->channel, sdp->id, sdp->lun);
- if (sdp->host->max_cmd_len != SCSI_DEBUG_MAX_CMD_LEN)
- sdp->host->max_cmd_len = SCSI_DEBUG_MAX_CMD_LEN;
- if (NULL == devip) {
+ if (sdp->host->max_cmd_len != SDEBUG_MAX_CMD_LEN)
+ sdp->host->max_cmd_len = SDEBUG_MAX_CMD_LEN;
+ if (devip == NULL) {
devip = find_build_dev_info(sdp);
- if (NULL == devip)
+ if (devip == NULL)
return 1; /* no resources, will be marked offline */
}
sdp->hostdata = devip;
@@ -3528,7 +3531,7 @@ static bool stop_queued_cmnd(struct scsi_cmnd *cmnd)
sd_dp = sqcp->sd_dp;
spin_unlock_irqrestore(&queued_arr_lock,
iflags);
- if ((sdebug_jdelay > 0) || (sdebug_ndelay > 0)) {
+ if (sdebug_jdelay > 0 || sdebug_ndelay > 0) {
if (sd_dp)
hrtimer_cancel(&sd_dp->hrt);
} else if (sdebug_jdelay < 0) {
@@ -3565,7 +3568,7 @@ static void stop_all_queued(void)
sqcp->a_cmnd = NULL;
sd_dp = sqcp->sd_dp;
spin_unlock_irqrestore(&queued_arr_lock, iflags);
- if ((sdebug_jdelay > 0) || (sdebug_ndelay > 0)) {
+ if (sdebug_jdelay > 0 || sdebug_ndelay > 0) {
if (sd_dp)
hrtimer_cancel(&sd_dp->hrt);
} else if (sdebug_jdelay < 0) {
@@ -3779,8 +3782,8 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
if (unlikely(WARN_ON(!cmnd)))
return SCSI_MLQUEUE_HOST_BUSY;
- if (unlikely(NULL == devip)) {
- if (0 == scsi_result)
+ if (unlikely(devip == NULL)) {
+ if (scsi_result == 0)
scsi_result = DID_NO_CONNECT << 16;
goto respond_in_thread;
}
@@ -3841,7 +3844,7 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
cmnd->result = scsi_result;
spin_unlock_irqrestore(&queued_arr_lock, iflags);
sd_dp = sqcp->sd_dp;
- if ((delta_jiff > 0) || (sdebug_ndelay > 0)) {
+ if (delta_jiff > 0 || sdebug_ndelay > 0) {
ktime_t kt;
if (delta_jiff > 0) {
@@ -3938,7 +3941,7 @@ module_param_named(write_same_length, sdebug_write_same_length, int,
MODULE_AUTHOR("Eric Youngdale + Douglas Gilbert");
MODULE_DESCRIPTION("SCSI debug adapter driver");
MODULE_LICENSE("GPL");
-MODULE_VERSION(SCSI_DEBUG_VERSION);
+MODULE_VERSION(SDEBUG_VERSION);
MODULE_PARM_DESC(add_host, "0..127 hosts allowed(def=1)");
MODULE_PARM_DESC(ato, "application tag ownership: 0=disk 1=host (def=1)");
@@ -3984,9 +3987,10 @@ static char sdebug_info[256];
static const char * scsi_debug_info(struct Scsi_Host * shp)
{
- sprintf(sdebug_info, "scsi_debug, version %s [%s], "
- "dev_size_mb=%d, opts=0x%x", SCSI_DEBUG_VERSION,
- sdebug_version_date, sdebug_dev_size_mb, sdebug_opts);
+ sprintf(sdebug_info,
+ "scsi_debug, version %s [%s], dev_size_mb=%d, opts=0x%x",
+ SDEBUG_VERSION, sdebug_version_date, sdebug_dev_size_mb,
+ sdebug_opts);
return sdebug_info;
}
@@ -4036,7 +4040,7 @@ static int scsi_debug_show_info(struct seq_file *m, struct Scsi_Host *host)
"command aborts=%d; RESETs: device=%d, target=%d, bus=%d, "
"host=%d\ndix_reads=%d dix_writes=%d dif_errors=%d "
"usec_in_jiffy=%lu\n",
- SCSI_DEBUG_VERSION, sdebug_version_date,
+ SDEBUG_VERSION, sdebug_version_date,
sdebug_num_tgts, sdebug_dev_size_mb, sdebug_opts,
sdebug_every_nth, b, sdebug_jdelay, sdebug_ndelay,
sdebug_max_luns, atomic_read(&sdebug_completions),
@@ -4064,7 +4068,7 @@ static ssize_t delay_store(struct device_driver *ddp, const char *buf,
{
int jdelay, res;
- if ((count > 0) && (1 == sscanf(buf, "%d", &jdelay))) {
+ if (count > 0 && sscanf(buf, "%d", &jdelay) == 1) {
res = count;
if (sdebug_jdelay != jdelay) {
unsigned long iflags;
@@ -4672,7 +4676,7 @@ static int __init scsi_debug_init(void)
(sdebug_sectors_per * sdebug_heads);
}
- if (0 == sdebug_fake_rw) {
+ if (sdebug_fake_rw == 0) {
fake_storep = vmalloc(sz);
if (NULL == fake_storep) {
pr_err("out of memory, 1\n");
@@ -5044,8 +5048,8 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,
}
}
if (unlikely(!(F_SKIP_UA & flags) &&
- SDEBUG_NUM_UAS != find_first_bit(devip->uas_bm,
- SDEBUG_NUM_UAS))) {
+ find_first_bit(devip->uas_bm,
+ SDEBUG_NUM_UAS) != SDEBUG_NUM_UAS)) {
errsts = make_ua(scp, devip);
if (errsts)
goto check_cond;
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2.5 2/6] scsi_debug: rework resp_report_luns
2016-05-01 2:44 [PATCH v2.5 0/6] scsi_debug: rebase second half of series Douglas Gilbert
2016-05-01 2:44 ` [PATCH v2.5 1/6] scsi_debug: use pdt constants Douglas Gilbert
@ 2016-05-01 2:44 ` Douglas Gilbert
2016-05-02 8:25 ` Hannes Reinecke
2016-05-03 23:58 ` Bart Van Assche
2016-05-01 2:44 ` [PATCH v2.5 3/6] scsi_debug: add multiple queue support Douglas Gilbert
` (3 subsequent siblings)
5 siblings, 2 replies; 24+ messages in thread
From: Douglas Gilbert @ 2016-05-01 2:44 UTC (permalink / raw)
To: linux-scsi; +Cc: martin.petersen, tomas.winkler, emilne, bart.vanassche
Based on "[PATH V2] scsi_debug: rework resp_report_luns" patch
sent by Tomas Winkler on Thursday, 26 Feb 2015. His notes:
1. Remove duplicated boundary checks which simplify the fill-in
loop
2. Use more of scsi generic API
Replace fixed length response array a with heap allocation
allowing up to 256 normal LUNs per target.
Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
---
drivers/scsi/scsi_debug.c | 135 +++++++++++++++++++++++++++++-----------------
1 file changed, 87 insertions(+), 48 deletions(-)
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index fc0246c..e97ddf0 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -3208,63 +3208,94 @@ static int resp_get_lba_status(struct scsi_cmnd *scp,
return fill_from_dev_buffer(scp, arr, SDEBUG_GET_LBA_STATUS_LEN);
}
-#define SDEBUG_RLUN_ARR_SZ 256
-
-static int resp_report_luns(struct scsi_cmnd * scp,
- struct sdebug_dev_info * devip)
+/* Even though each pseudo target has a REPORT LUNS "well known logical unit"
+ * (W-LUN), the normal Linux scanning logic does not associate it with a
+ * device (e.g. /dev/sg7). The following magic will make that association:
+ * "cd /sys/class/scsi_host/host<n> ; echo '- - 49409' > scan"
+ * where <n> is a host number. If there are multiple targets in a host then
+ * the above will associate a W-LUN to each target. To only get a W-LUN
+ * for target 2, then use "echo '- 2 49409' > scan" .
+ */
+static int resp_report_luns(struct scsi_cmnd *scp,
+ struct sdebug_dev_info *devip)
{
+ unsigned char *cmd = scp->cmnd;
unsigned int alloc_len;
- int lun_cnt, i, upper, num, n, want_wlun, shortish;
+ unsigned char select_report;
u64 lun;
- unsigned char *cmd = scp->cmnd;
- int select_report = (int)cmd[2];
- struct scsi_lun *one_lun;
- unsigned char arr[SDEBUG_RLUN_ARR_SZ];
- unsigned char * max_addr;
+ struct scsi_lun *lun_p;
+ u8 *arr;
+ unsigned int lun_cnt; /* normal LUN count (max: 256) */
+ unsigned int wlun_cnt; /* report luns W-LUN count */
+ unsigned int tlun_cnt; /* total LUN count */
+ unsigned int rlen; /* response length (in bytes) */
+ int i, res;
clear_luns_changed_on_target(devip);
- alloc_len = cmd[9] + (cmd[8] << 8) + (cmd[7] << 16) + (cmd[6] << 24);
- shortish = (alloc_len < 4);
- if (shortish || (select_report > 2)) {
- mk_sense_invalid_fld(scp, SDEB_IN_CDB, shortish ? 6 : 2, -1);
+
+ select_report = cmd[2];
+ alloc_len = get_unaligned_be32(cmd + 6);
+
+ if (alloc_len < 4) {
+ pr_err("alloc len too small %d\n", alloc_len);
+ mk_sense_invalid_fld(scp, SDEB_IN_CDB, 6, -1);
return check_condition_result;
}
- /* can produce response with up to 16k luns (lun 0 to lun 16383) */
- memset(arr, 0, SDEBUG_RLUN_ARR_SZ);
- lun_cnt = sdebug_max_luns;
- if (1 == select_report)
+
+ switch (select_report) {
+ case 0: /* all LUNs apart from W-LUNs */
+ lun_cnt = sdebug_max_luns;
+ wlun_cnt = 0;
+ break;
+ case 1: /* only W-LUNs */
lun_cnt = 0;
- else if (sdebug_no_lun_0 && (lun_cnt > 0))
+ wlun_cnt = 1;
+ break;
+ case 2: /* all LUNs */
+ lun_cnt = sdebug_max_luns;
+ wlun_cnt = 1;
+ break;
+ case 0x10: /* only administrative LUs */
+ case 0x11: /* see SPC-5 */
+ case 0x12: /* only subsiduary LUs owned by referenced LU */
+ default:
+ pr_debug("select report invalid %d\n", select_report);
+ mk_sense_invalid_fld(scp, SDEB_IN_CDB, 2, -1);
+ return check_condition_result;
+ }
+
+ if (sdebug_no_lun_0 && (lun_cnt > 0))
--lun_cnt;
- want_wlun = (select_report > 0) ? 1 : 0;
- num = lun_cnt + want_wlun;
- arr[2] = ((sizeof(struct scsi_lun) * num) >> 8) & 0xff;
- arr[3] = (sizeof(struct scsi_lun) * num) & 0xff;
- n = min((int)((SDEBUG_RLUN_ARR_SZ - 8) /
- sizeof(struct scsi_lun)), num);
- if (n < num) {
- want_wlun = 0;
- lun_cnt = n;
- }
- one_lun = (struct scsi_lun *) &arr[8];
- max_addr = arr + SDEBUG_RLUN_ARR_SZ;
- for (i = 0, lun = (sdebug_no_lun_0 ? 1 : 0);
- ((i < lun_cnt) && ((unsigned char *)(one_lun + i) < max_addr));
- i++, lun++) {
- upper = (lun >> 8) & 0x3f;
- if (upper)
- one_lun[i].scsi_lun[0] =
- (upper | (SAM2_LUN_ADDRESS_METHOD << 6));
- one_lun[i].scsi_lun[1] = lun & 0xff;
- }
- if (want_wlun) {
- one_lun[i].scsi_lun[0] = (SCSI_W_LUN_REPORT_LUNS >> 8) & 0xff;
- one_lun[i].scsi_lun[1] = SCSI_W_LUN_REPORT_LUNS & 0xff;
- i++;
- }
- alloc_len = (unsigned char *)(one_lun + i) - arr;
- return fill_from_dev_buffer(scp, arr,
- min((int)alloc_len, SDEBUG_RLUN_ARR_SZ));
+
+ tlun_cnt = lun_cnt + wlun_cnt;
+
+ rlen = (tlun_cnt * sizeof(struct scsi_lun)) + 8;
+ arr = vmalloc(rlen);
+ if (!arr) {
+ mk_sense_buffer(scp, ILLEGAL_REQUEST, INSUFF_RES_ASC,
+ INSUFF_RES_ASCQ);
+ return check_condition_result;
+ }
+ memset(arr, 0, rlen);
+ pr_debug("select_report %d luns = %d wluns = %d no_lun0 %d\n",
+ select_report, lun_cnt, wlun_cnt, sdebug_no_lun_0);
+
+ /* luns start at byte 8 in response following the header */
+ lun_p = (struct scsi_lun *)&arr[8];
+
+ /* LUNs use single level peripheral device addressing method */
+ lun = sdebug_no_lun_0 ? 1 : 0;
+ for (i = 0; i < lun_cnt; i++)
+ int_to_scsilun(lun++, lun_p++);
+
+ if (wlun_cnt)
+ int_to_scsilun(SCSI_W_LUN_REPORT_LUNS, lun_p++);
+
+ put_unaligned_be32(rlen - 8, &arr[0]);
+
+ res = fill_from_dev_buffer(scp, arr, rlen);
+ vfree(arr);
+ return res;
}
static int resp_xdwriteread(struct scsi_cmnd *scp, unsigned long long lba,
@@ -4303,6 +4334,10 @@ static ssize_t max_luns_store(struct device_driver *ddp, const char *buf,
bool changed;
if ((count > 0) && (1 == sscanf(buf, "%d", &n)) && (n >= 0)) {
+ if (n > 256) {
+ pr_warn("max_luns can be no more than 256\n");
+ return -EINVAL;
+ }
changed = (sdebug_max_luns != n);
sdebug_max_luns = n;
sdebug_max_tgts_luns();
@@ -4647,6 +4682,10 @@ static int __init scsi_debug_init(void)
pr_err("invalid physblk_exp %u\n", sdebug_physblk_exp);
return -EINVAL;
}
+ if (sdebug_max_luns > 256) {
+ pr_warn("max_luns can be no more than 256, use default\n");
+ sdebug_max_luns = DEF_MAX_LUNS;
+ }
if (sdebug_lowest_aligned > 0x3fff) {
pr_err("lowest_aligned too big: %u\n", sdebug_lowest_aligned);
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2.5 3/6] scsi_debug: add multiple queue support
2016-05-01 2:44 [PATCH v2.5 0/6] scsi_debug: rebase second half of series Douglas Gilbert
2016-05-01 2:44 ` [PATCH v2.5 1/6] scsi_debug: use pdt constants Douglas Gilbert
2016-05-01 2:44 ` [PATCH v2.5 2/6] scsi_debug: rework resp_report_luns Douglas Gilbert
@ 2016-05-01 2:44 ` Douglas Gilbert
2016-05-02 8:35 ` Hannes Reinecke
2016-05-04 22:32 ` Bart Van Assche
2016-05-01 2:44 ` [PATCH v2.5 4/6] scsi_debug: vpd and mode page work Douglas Gilbert
` (2 subsequent siblings)
5 siblings, 2 replies; 24+ messages in thread
From: Douglas Gilbert @ 2016-05-01 2:44 UTC (permalink / raw)
To: linux-scsi; +Cc: martin.petersen, tomas.winkler, emilne, bart.vanassche
Add submit_queue parameter (minimum and default: 1; maximum:
nr_cpu_ids) that controls how many queues are built, each with
their own lock and in_use bit vector. Add statistics parameter
which is default on.
Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
---
drivers/scsi/scsi_debug.c | 680 +++++++++++++++++++++++++++++-----------------
1 file changed, 426 insertions(+), 254 deletions(-)
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index e97ddf0..0932111 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -135,6 +135,8 @@ static const char *sdebug_version_date = "20160430";
#define DEF_VPD_USE_HOSTNO 1
#define DEF_WRITESAME_LENGTH 0xFFFF
#define DEF_STRICT 0
+#define DEF_STATISTICS true
+#define DEF_SUBMIT_QUEUES 1
#define JDELAY_OVERRIDDEN -9999
#define SDEBUG_LUN_0_VAL 0
@@ -201,20 +203,17 @@ static const char *sdebug_version_date = "20160430";
* or "peripheral device" addressing (value 0) */
#define SAM2_LUN_ADDRESS_METHOD 0
-/* SCSI_DEBUG_CANQUEUE is the maximum number of commands that can be queued
- * (for response) at one time. Can be reduced by max_queue option. Command
- * responses are not queued when jdelay=0 and ndelay=0. The per-device
- * DEF_CMD_PER_LUN can be changed via sysfs:
- * /sys/class/scsi_device/<h:c:t:l>/device/queue_depth but cannot exceed
- * SCSI_DEBUG_CANQUEUE. */
-#define SCSI_DEBUG_CANQUEUE_WORDS 9 /* a WORD is bits in a long */
-#define SCSI_DEBUG_CANQUEUE (SCSI_DEBUG_CANQUEUE_WORDS * BITS_PER_LONG)
+/* SDEBUG_CANQUEUE is the maximum number of commands that can be queued
+ * (for response) per submit queue at one time. Can be reduced by max_queue
+ * option. Command responses are not queued when jdelay=0 and ndelay=0. The
+ * per-device DEF_CMD_PER_LUN can be changed via sysfs:
+ * /sys/class/scsi_device/<h:c:t:l>/device/queue_depth
+ * but cannot exceed SDEBUG_CANQUEUE .
+ */
+#define SDEBUG_CANQUEUE_WORDS 3 /* a WORD is bits in a long */
+#define SDEBUG_CANQUEUE (SDEBUG_CANQUEUE_WORDS * BITS_PER_LONG)
#define DEF_CMD_PER_LUN 255
-#if DEF_CMD_PER_LUN > SCSI_DEBUG_CANQUEUE
-#warning "Expect DEF_CMD_PER_LUN <= SCSI_DEBUG_CANQUEUE"
-#endif
-
#define F_D_IN 1
#define F_D_OUT 2
#define F_D_OUT_MAYBE 4 /* WRITE SAME, NDOB bit */
@@ -245,7 +244,7 @@ struct sdebug_dev_info {
struct sdebug_host_info *sdbg_host;
unsigned long uas_bm[1];
atomic_t num_in_q;
- char stopped; /* TODO: should be atomic */
+ atomic_t stopped;
bool used;
};
@@ -262,21 +261,30 @@ struct sdebug_host_info {
struct sdebug_defer {
struct hrtimer hrt;
struct execute_work ew;
- int qa_indx;
+ int qc_indx;
+ int sq_indx;
};
struct sdebug_queued_cmd {
- /* in_use flagged by a bit in queued_in_use_bm[] */
+ /* a bit in in_use_bm[] in struct sdebug_queue flags if in use */
struct sdebug_defer *sd_dp;
struct scsi_cmnd *a_cmnd;
+ unsigned int inj_recovered:1;
+ unsigned int inj_transport:1;
+ unsigned int inj_dif:1;
+ unsigned int inj_dix:1;
+ unsigned int inj_short:1;
};
-struct sdebug_scmd_extra_t {
- bool inj_recovered;
- bool inj_transport;
- bool inj_dif;
- bool inj_dix;
- bool inj_short;
+struct sdebug_queue {
+ struct sdebug_queued_cmd qc_arr[SDEBUG_CANQUEUE];
+ unsigned long in_use_bm[SDEBUG_CANQUEUE_WORDS];
+ spinlock_t qc_lock;
+ atomic_t blocked; /* to temporarily stop more being queued */
+ atomic_t cmnd_count; /* number of incoming commands */
+ atomic_t completions; /* only command with deferred responses */
+ atomic_t misqueues; /* completion q different from submission q */
+ atomic_t a_tsf; /* 'almost task set full' event counter */
};
struct opcode_info_t {
@@ -326,6 +334,7 @@ enum sdeb_opcode_index {
SDEB_I_LAST_ELEMENT = 30, /* keep this last */
};
+
static const unsigned char opcode_ind_arr[256] = {
/* 0x0; 0x0->0x1f: 6 byte cdbs */
SDEB_I_TEST_UNIT_READY, SDEB_I_REZERO_UNIT, 0, SDEB_I_REQUEST_SENSE,
@@ -563,7 +572,7 @@ static int sdebug_fake_rw = DEF_FAKE_RW;
static unsigned int sdebug_guard = DEF_GUARD;
static int sdebug_lowest_aligned = DEF_LOWEST_ALIGNED;
static int sdebug_max_luns = DEF_MAX_LUNS;
-static int sdebug_max_queue = SCSI_DEBUG_CANQUEUE;
+static int sdebug_max_queue = SDEBUG_CANQUEUE; /* per submit queue */
static atomic_t retired_max_queue; /* if > 0 then was prior max_queue */
static int sdebug_ndelay = DEF_NDELAY; /* if > 0 then unit is nanoseconds */
static int sdebug_no_lun_0 = DEF_NO_LUN_0;
@@ -594,10 +603,8 @@ static bool sdebug_strict = DEF_STRICT;
static bool sdebug_any_injecting_opt;
static bool sdebug_verbose;
static bool have_dif_prot;
-
-static atomic_t sdebug_cmnd_count;
-static atomic_t sdebug_completions;
-static atomic_t sdebug_a_tsf; /* counter of 'almost' TSFs */
+static bool sdebug_statistics = DEF_STATISTICS;
+static bool sdebug_mq_available;
static unsigned int sdebug_store_sectors;
static sector_t sdebug_capacity; /* in sectors */
@@ -625,10 +632,9 @@ static int dix_writes;
static int dix_reads;
static int dif_errors;
-static struct sdebug_queued_cmd queued_arr[SCSI_DEBUG_CANQUEUE];
-static unsigned long queued_in_use_bm[SCSI_DEBUG_CANQUEUE_WORDS];
+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_SPINLOCK(queued_arr_lock);
static DEFINE_RWLOCK(atomic_rw);
static char sdebug_proc_name[] = MY_NAME;
@@ -1428,16 +1434,15 @@ static int resp_start_stop(struct scsi_cmnd * scp,
struct sdebug_dev_info * devip)
{
unsigned char *cmd = scp->cmnd;
- int power_cond, start;
+ int power_cond, stop;
power_cond = (cmd[4] & 0xf0) >> 4;
if (power_cond) {
mk_sense_invalid_fld(scp, SDEB_IN_CDB, 4, 7);
return check_condition_result;
}
- start = cmd[4] & 1;
- if (start == devip->stopped)
- devip->stopped = !start;
+ stop = !(cmd[4] & 1);
+ atomic_xchg(&devip->stopped, stop);
return 0;
}
@@ -2450,6 +2455,7 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, sector_t start_sec,
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;
@@ -2509,11 +2515,14 @@ static int resp_read_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
"to DIF device\n");
}
if (unlikely(sdebug_any_injecting_opt)) {
- struct sdebug_scmd_extra_t *ep = scsi_cmd_priv(scp);
+ sqcp = (struct sdebug_queued_cmd *)scp->host_scribble;
- if (ep->inj_short)
- num /= 2;
- }
+ if (sqcp) {
+ if (sqcp->inj_short)
+ num /= 2;
+ }
+ } else
+ sqcp = NULL;
/* inline check_device_access_params() */
if (unlikely(lba + num > sdebug_capacity)) {
@@ -2563,22 +2572,20 @@ static int resp_read_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
scsi_in(scp)->resid = scsi_bufflen(scp) - ret;
- if (unlikely(sdebug_any_injecting_opt)) {
- struct sdebug_scmd_extra_t *ep = scsi_cmd_priv(scp);
-
- if (ep->inj_recovered) {
+ if (unlikely(sqcp)) {
+ if (sqcp->inj_recovered) {
mk_sense_buffer(scp, RECOVERED_ERROR,
THRESHOLD_EXCEEDED, 0);
return check_condition_result;
- } else if (ep->inj_transport) {
+ } else if (sqcp->inj_transport) {
mk_sense_buffer(scp, ABORTED_COMMAND,
TRANSPORT_PROBLEM, ACK_NAK_TO);
return check_condition_result;
- } else if (ep->inj_dif) {
+ } else if (sqcp->inj_dif) {
/* Logical block guard check failed */
mk_sense_buffer(scp, ABORTED_COMMAND, 0x10, 1);
return illegal_condition_result;
- } else if (ep->inj_dix) {
+ } else if (sqcp->inj_dix) {
mk_sense_buffer(scp, ILLEGAL_REQUEST, 0x10, 1);
return illegal_condition_result;
}
@@ -2851,25 +2858,29 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
write_unlock_irqrestore(&atomic_rw, iflags);
if (unlikely(-1 == ret))
return DID_ERROR << 16;
- else if (sdebug_verbose && (ret < (num * sdebug_sector_size)))
+ else if (unlikely(sdebug_verbose &&
+ (ret < (num * sdebug_sector_size))))
sdev_printk(KERN_INFO, scp->device,
"%s: write: cdb indicated=%u, IO sent=%d bytes\n",
my_name, num * sdebug_sector_size, ret);
if (unlikely(sdebug_any_injecting_opt)) {
- struct sdebug_scmd_extra_t *ep = scsi_cmd_priv(scp);
+ struct sdebug_queued_cmd *sqcp =
+ (struct sdebug_queued_cmd *)scp->host_scribble;
- if (ep->inj_recovered) {
- mk_sense_buffer(scp, RECOVERED_ERROR,
- THRESHOLD_EXCEEDED, 0);
- return check_condition_result;
- } else if (ep->inj_dif) {
- /* Logical block guard check failed */
- mk_sense_buffer(scp, ABORTED_COMMAND, 0x10, 1);
- return illegal_condition_result;
- } else if (ep->inj_dix) {
- mk_sense_buffer(scp, ILLEGAL_REQUEST, 0x10, 1);
- return illegal_condition_result;
+ if (sqcp) {
+ if (sqcp->inj_recovered) {
+ mk_sense_buffer(scp, RECOVERED_ERROR,
+ THRESHOLD_EXCEEDED, 0);
+ return check_condition_result;
+ } else if (sqcp->inj_dif) {
+ /* Logical block guard check failed */
+ mk_sense_buffer(scp, ABORTED_COMMAND, 0x10, 1);
+ return illegal_condition_result;
+ } else if (sqcp->inj_dix) {
+ mk_sense_buffer(scp, ILLEGAL_REQUEST, 0x10, 1);
+ return illegal_condition_result;
+ }
}
}
return 0;
@@ -3360,27 +3371,40 @@ static int resp_xdwriteread_10(struct scsi_cmnd *scp,
return resp_xdwriteread(scp, lba, num, devip);
}
+static struct sdebug_queue *get_queue(void)
+{
+ struct sdebug_queue *sqp = sdebug_q_arr;
+
+ return sqp + (raw_smp_processor_id() % submit_queues);
+}
+
/* Queued command completions converge here. */
static void sdebug_q_cmd_complete(struct sdebug_defer *sd_dp)
{
- int qa_indx;
+ int qc_indx;
int retiring = 0;
unsigned long iflags;
+ struct sdebug_queue *sqp;
struct sdebug_queued_cmd *sqcp;
struct scsi_cmnd *scp;
struct sdebug_dev_info *devip;
- atomic_inc(&sdebug_completions);
- qa_indx = sd_dp->qa_indx;
- if (unlikely((qa_indx < 0) || (qa_indx >= SCSI_DEBUG_CANQUEUE))) {
- pr_err("wild qa_indx=%d\n", qa_indx);
+ sqp = sdebug_q_arr + sd_dp->sq_indx;
+ if (sdebug_statistics) {
+ atomic_inc(&sqp->completions);
+ if (get_queue() != sqp)
+ atomic_inc(&sqp->misqueues);
+ }
+ qc_indx = sd_dp->qc_indx;
+ if (unlikely((qc_indx < 0) || (qc_indx >= SDEBUG_CANQUEUE))) {
+ pr_err("wild qc_indx=%d\n", qc_indx);
return;
}
- spin_lock_irqsave(&queued_arr_lock, iflags);
- sqcp = &queued_arr[qa_indx];
+ spin_lock_irqsave(&sqp->qc_lock, iflags);
+ sqcp = &sqp->qc_arr[qc_indx];
scp = sqcp->a_cmnd;
if (unlikely(scp == NULL)) {
- spin_unlock_irqrestore(&queued_arr_lock, iflags);
+ spin_unlock_irqrestore(&sqp->qc_lock, iflags);
pr_err("scp is NULL\n");
return;
}
@@ -3393,8 +3417,8 @@ static void sdebug_q_cmd_complete(struct sdebug_defer *sd_dp)
retiring = 1;
sqcp->a_cmnd = NULL;
- if (unlikely(!test_and_clear_bit(qa_indx, queued_in_use_bm))) {
- spin_unlock_irqrestore(&queued_arr_lock, iflags);
+ if (unlikely(!test_and_clear_bit(qc_indx, sqp->in_use_bm))) {
+ spin_unlock_irqrestore(&sqp->qc_lock, iflags);
pr_err("Unexpected completion\n");
return;
}
@@ -3403,18 +3427,18 @@ static void sdebug_q_cmd_complete(struct sdebug_defer *sd_dp)
int k, retval;
retval = atomic_read(&retired_max_queue);
- if (qa_indx >= retval) {
- spin_unlock_irqrestore(&queued_arr_lock, iflags);
+ if (qc_indx >= retval) {
+ spin_unlock_irqrestore(&sqp->qc_lock, iflags);
pr_err("index %d too large\n", retval);
return;
}
- k = find_last_bit(queued_in_use_bm, retval);
+ k = find_last_bit(sqp->in_use_bm, retval);
if ((k < sdebug_max_queue) || (k == retval))
atomic_set(&retired_max_queue, 0);
else
atomic_set(&retired_max_queue, k + 1);
}
- spin_unlock_irqrestore(&queued_arr_lock, iflags);
+ spin_unlock_irqrestore(&sqp->qc_lock, iflags);
scp->scsi_done(scp); /* callback to mid level */
}
@@ -3533,47 +3557,53 @@ static void scsi_debug_slave_destroy(struct scsi_device *sdp)
}
}
+static void stop_qc_helper(struct sdebug_defer *sd_dp)
+{
+ if (!sd_dp)
+ return;
+ if ((sdebug_jdelay > 0) || (sdebug_ndelay > 0))
+ hrtimer_cancel(&sd_dp->hrt);
+ else if (sdebug_jdelay < 0)
+ cancel_work_sync(&sd_dp->ew.work);
+}
+
/* If @cmnd found deletes its timer or work queue and returns true; else
returns false */
static bool stop_queued_cmnd(struct scsi_cmnd *cmnd)
{
unsigned long iflags;
- int k, qmax, r_qmax;
+ int j, k, qmax, r_qmax;
+ struct sdebug_queue *sqp;
struct sdebug_queued_cmd *sqcp;
struct sdebug_dev_info *devip;
struct sdebug_defer *sd_dp;
- spin_lock_irqsave(&queued_arr_lock, iflags);
- qmax = sdebug_max_queue;
- r_qmax = atomic_read(&retired_max_queue);
- if (r_qmax > qmax)
- qmax = r_qmax;
- for (k = 0; k < qmax; ++k) {
- if (test_bit(k, queued_in_use_bm)) {
- sqcp = &queued_arr[k];
- if (cmnd != sqcp->a_cmnd)
- continue;
- /* found command */
- devip = (struct sdebug_dev_info *)
- cmnd->device->hostdata;
- if (devip)
- atomic_dec(&devip->num_in_q);
- sqcp->a_cmnd = NULL;
- sd_dp = sqcp->sd_dp;
- spin_unlock_irqrestore(&queued_arr_lock,
- iflags);
- if (sdebug_jdelay > 0 || sdebug_ndelay > 0) {
- if (sd_dp)
- hrtimer_cancel(&sd_dp->hrt);
- } else if (sdebug_jdelay < 0) {
- if (sd_dp)
- cancel_work_sync(&sd_dp->ew.work);
+ for (j = 0, sqp = sdebug_q_arr; j < submit_queues; ++j, ++sqp) {
+ spin_lock_irqsave(&sqp->qc_lock, iflags);
+ qmax = sdebug_max_queue;
+ r_qmax = atomic_read(&retired_max_queue);
+ if (r_qmax > qmax)
+ qmax = r_qmax;
+ for (k = 0; k < qmax; ++k) {
+ if (test_bit(k, sqp->in_use_bm)) {
+ sqcp = &sqp->qc_arr[k];
+ if (cmnd != sqcp->a_cmnd)
+ continue;
+ /* found */
+ devip = (struct sdebug_dev_info *)
+ cmnd->device->hostdata;
+ if (devip)
+ atomic_dec(&devip->num_in_q);
+ sqcp->a_cmnd = NULL;
+ sd_dp = sqcp->sd_dp;
+ spin_unlock_irqrestore(&sqp->qc_lock, iflags);
+ stop_qc_helper(sd_dp);
+ clear_bit(k, sqp->in_use_bm);
+ return true;
}
- clear_bit(k, queued_in_use_bm);
- return true;
}
+ spin_unlock_irqrestore(&sqp->qc_lock, iflags);
}
- spin_unlock_irqrestore(&queued_arr_lock, iflags);
return false;
}
@@ -3581,48 +3611,48 @@ static bool stop_queued_cmnd(struct scsi_cmnd *cmnd)
static void stop_all_queued(void)
{
unsigned long iflags;
- int k;
+ int j, k;
+ struct sdebug_queue *sqp;
struct sdebug_queued_cmd *sqcp;
struct sdebug_dev_info *devip;
struct sdebug_defer *sd_dp;
- spin_lock_irqsave(&queued_arr_lock, iflags);
- for (k = 0; k < SCSI_DEBUG_CANQUEUE; ++k) {
- if (test_bit(k, queued_in_use_bm)) {
- sqcp = &queued_arr[k];
- if (NULL == sqcp->a_cmnd)
- continue;
- devip = (struct sdebug_dev_info *)
- sqcp->a_cmnd->device->hostdata;
- if (devip)
- atomic_dec(&devip->num_in_q);
- sqcp->a_cmnd = NULL;
- sd_dp = sqcp->sd_dp;
- spin_unlock_irqrestore(&queued_arr_lock, iflags);
- if (sdebug_jdelay > 0 || sdebug_ndelay > 0) {
- if (sd_dp)
- hrtimer_cancel(&sd_dp->hrt);
- } else if (sdebug_jdelay < 0) {
- if (sd_dp)
- cancel_work_sync(&sd_dp->ew.work);
+ for (j = 0, sqp = sdebug_q_arr; j < submit_queues; ++j, ++sqp) {
+ spin_lock_irqsave(&sqp->qc_lock, iflags);
+ for (k = 0; k < SDEBUG_CANQUEUE; ++k) {
+ if (test_bit(k, sqp->in_use_bm)) {
+ sqcp = &sqp->qc_arr[k];
+ if (sqcp->a_cmnd == NULL)
+ continue;
+ devip = (struct sdebug_dev_info *)
+ sqcp->a_cmnd->device->hostdata;
+ if (devip)
+ atomic_dec(&devip->num_in_q);
+ sqcp->a_cmnd = NULL;
+ sd_dp = sqcp->sd_dp;
+ spin_unlock_irqrestore(&sqp->qc_lock, iflags);
+ stop_qc_helper(sd_dp);
+ clear_bit(k, sqp->in_use_bm);
+ spin_lock_irqsave(&sqp->qc_lock, iflags);
}
- clear_bit(k, queued_in_use_bm);
- spin_lock_irqsave(&queued_arr_lock, iflags);
}
+ spin_unlock_irqrestore(&sqp->qc_lock, iflags);
}
- spin_unlock_irqrestore(&queued_arr_lock, iflags);
}
/* Free queued command memory on heap */
static void free_all_queued(void)
{
- int k;
+ int j, k;
+ struct sdebug_queue *sqp;
struct sdebug_queued_cmd *sqcp;
- for (k = 0; k < SCSI_DEBUG_CANQUEUE; ++k) {
- sqcp = &queued_arr[k];
- kfree(sqcp->sd_dp);
- sqcp->sd_dp = NULL;
+ for (j = 0, sqp = sdebug_q_arr; j < submit_queues; ++j, ++sqp) {
+ for (k = 0; k < SDEBUG_CANQUEUE; ++k) {
+ sqcp = &sqp->qc_arr[k];
+ kfree(sqcp->sd_dp);
+ sqcp->sd_dp = NULL;
+ }
}
}
@@ -3801,24 +3831,79 @@ static void __init sdebug_build_parts(unsigned char *ramp,
}
}
+static void block_unblock_all_queues(bool block)
+{
+ int j;
+ struct sdebug_queue *sqp;
+
+ for (j = 0, sqp = sdebug_q_arr; j < submit_queues; ++j, ++sqp)
+ atomic_set(&sqp->blocked, (int)block);
+}
+
+/* Adjust (by rounding down) each cmnd_count so abs(every_nth)-1 commands
+ * will be processed normally on each queue before triggers occur.
+ */
+static void tweak_cmnd_count(void)
+{
+ int j, count, modulo;
+ struct sdebug_queue *sqp;
+
+ modulo = abs(sdebug_every_nth);
+ if (modulo < 2)
+ return;
+ block_unblock_all_queues(true);
+ for (j = 0, sqp = sdebug_q_arr; j < submit_queues; ++j, ++sqp) {
+ count = atomic_read(&sqp->cmnd_count);
+ atomic_set(&sqp->cmnd_count, (count / modulo) * modulo);
+ }
+ block_unblock_all_queues(false);
+}
+
+static void clear_queue_stats(void)
+{
+ int j;
+ struct sdebug_queue *sqp;
+
+ for (j = 0, sqp = sdebug_q_arr; j < submit_queues; ++j, ++sqp) {
+ atomic_set(&sqp->cmnd_count, 0);
+ atomic_set(&sqp->completions, 0);
+ atomic_set(&sqp->misqueues, 0);
+ atomic_set(&sqp->a_tsf, 0);
+ }
+}
+
+static void setup_inject(struct sdebug_queue *sqp,
+ struct sdebug_queued_cmd *sqcp)
+{
+ if ((atomic_read(&sqp->cmnd_count) % abs(sdebug_every_nth)) > 0)
+ return;
+ sqcp->inj_recovered = !!(SDEBUG_OPT_RECOVERED_ERR & sdebug_opts);
+ sqcp->inj_transport = !!(SDEBUG_OPT_TRANSPORT_ERR & sdebug_opts);
+ sqcp->inj_dif = !!(SDEBUG_OPT_DIF_ERR & sdebug_opts);
+ sqcp->inj_dix = !!(SDEBUG_OPT_DIX_ERR & sdebug_opts);
+ sqcp->inj_short = !!(SDEBUG_OPT_SHORT_TRANSFER & sdebug_opts);
+}
+
+/* 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
+ * SCSI_MLQUEUE_HOST_BUSY if temporarily out of resources.
+ */
static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
int scsi_result, int delta_jiff)
{
unsigned long iflags;
int k, num_in_q, qdepth, inject;
+ struct sdebug_queue *sqp;
struct sdebug_queued_cmd *sqcp = NULL;
struct scsi_device *sdp;
struct sdebug_defer *sd_dp;
- if (unlikely(WARN_ON(!cmnd)))
- return SCSI_MLQUEUE_HOST_BUSY;
-
if (unlikely(devip == NULL)) {
if (scsi_result == 0)
scsi_result = DID_NO_CONNECT << 16;
goto respond_in_thread;
}
-
sdp = cmnd->device;
if (unlikely(sdebug_verbose && scsi_result))
@@ -3828,31 +3913,36 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
goto respond_in_thread;
/* schedule the response at a later time if resources permit */
- spin_lock_irqsave(&queued_arr_lock, iflags);
+ sqp = get_queue();
+ spin_lock_irqsave(&sqp->qc_lock, iflags);
+ if (unlikely(atomic_read(&sqp->blocked))) {
+ spin_unlock_irqrestore(&sqp->qc_lock, iflags);
+ return SCSI_MLQUEUE_HOST_BUSY;
+ }
num_in_q = atomic_read(&devip->num_in_q);
qdepth = cmnd->device->queue_depth;
inject = 0;
if (unlikely((qdepth > 0) && (num_in_q >= qdepth))) {
if (scsi_result) {
- spin_unlock_irqrestore(&queued_arr_lock, iflags);
+ spin_unlock_irqrestore(&sqp->qc_lock, iflags);
goto respond_in_thread;
} else
scsi_result = device_qfull_result;
- } else if (unlikely((sdebug_every_nth != 0) &&
+ } else if (unlikely(sdebug_every_nth &&
(SDEBUG_OPT_RARE_TSF & sdebug_opts) &&
(scsi_result == 0))) {
if ((num_in_q == (qdepth - 1)) &&
- (atomic_inc_return(&sdebug_a_tsf) >=
+ (atomic_inc_return(&sqp->a_tsf) >=
abs(sdebug_every_nth))) {
- atomic_set(&sdebug_a_tsf, 0);
+ atomic_set(&sqp->a_tsf, 0);
inject = 1;
scsi_result = device_qfull_result;
}
}
- k = find_first_zero_bit(queued_in_use_bm, sdebug_max_queue);
+ k = find_first_zero_bit(sqp->in_use_bm, sdebug_max_queue);
if (unlikely(k >= sdebug_max_queue)) {
- spin_unlock_irqrestore(&queued_arr_lock, iflags);
+ spin_unlock_irqrestore(&sqp->qc_lock, iflags);
if (scsi_result)
goto respond_in_thread;
else if (SDEBUG_OPT_ALL_TSF & sdebug_opts)
@@ -3868,13 +3958,16 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
else
return SCSI_MLQUEUE_HOST_BUSY;
}
- __set_bit(k, queued_in_use_bm);
+ __set_bit(k, sqp->in_use_bm);
atomic_inc(&devip->num_in_q);
- sqcp = &queued_arr[k];
+ sqcp = &sqp->qc_arr[k];
sqcp->a_cmnd = cmnd;
+ cmnd->host_scribble = (unsigned char *)sqcp;
cmnd->result = scsi_result;
- spin_unlock_irqrestore(&queued_arr_lock, iflags);
sd_dp = sqcp->sd_dp;
+ spin_unlock_irqrestore(&sqp->qc_lock, iflags);
+ if (unlikely(sdebug_every_nth && sdebug_any_injecting_opt))
+ setup_inject(sqp, sqcp);
if (delta_jiff > 0 || sdebug_ndelay > 0) {
ktime_t kt;
@@ -3891,18 +3984,20 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
return SCSI_MLQUEUE_HOST_BUSY;
sqcp->sd_dp = sd_dp;
hrtimer_init(&sd_dp->hrt, CLOCK_MONOTONIC,
- HRTIMER_MODE_REL);
+ HRTIMER_MODE_REL_PINNED);
sd_dp->hrt.function = sdebug_q_cmd_hrt_complete;
- sd_dp->qa_indx = k;
+ sd_dp->sq_indx = sqp - sdebug_q_arr;
+ sd_dp->qc_indx = k;
}
- hrtimer_start(&sd_dp->hrt, kt, HRTIMER_MODE_REL);
- } else { /* jdelay < 0 */
+ hrtimer_start(&sd_dp->hrt, kt, HRTIMER_MODE_REL_PINNED);
+ } else { /* jdelay < 0, use work queue */
if (NULL == sd_dp) {
sd_dp = kzalloc(sizeof(*sqcp->sd_dp), GFP_ATOMIC);
if (NULL == sd_dp)
return SCSI_MLQUEUE_HOST_BUSY;
sqcp->sd_dp = sd_dp;
- sd_dp->qa_indx = k;
+ sd_dp->sq_indx = sqp - sdebug_q_arr;
+ sd_dp->qc_indx = k;
INIT_WORK(&sd_dp->ew.work, sdebug_q_cmd_wq_complete);
}
schedule_work(&sd_dp->ew.work);
@@ -3958,7 +4053,9 @@ module_param_named(ptype, sdebug_ptype, int, 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);
+module_param_named(statistics, sdebug_statistics, bool, S_IRUGO | S_IWUSR);
module_param_named(strict, sdebug_strict, bool, S_IRUGO | S_IWUSR);
+module_param_named(submit_queues, submit_queues, int, S_IRUGO);
module_param_named(unmap_alignment, sdebug_unmap_alignment, int, S_IRUGO);
module_param_named(unmap_granularity, sdebug_unmap_granularity, int, S_IRUGO);
module_param_named(unmap_max_blocks, sdebug_unmap_max_blocks, int, S_IRUGO);
@@ -4005,7 +4102,9 @@ MODULE_PARM_DESC(ptype, "SCSI peripheral type(def=0[disk])");
MODULE_PARM_DESC(removable, "claim to have removable media (def=0)");
MODULE_PARM_DESC(scsi_level, "SCSI level to simulate(def=6[SPC-4])");
MODULE_PARM_DESC(sector_size, "logical block size in bytes (def=512)");
+MODULE_PARM_DESC(statistics, "collect statistics on commands, queues (def=1)");
MODULE_PARM_DESC(strict, "stricter checks: reserved field in cdb (def=0)");
+MODULE_PARM_DESC(submit_queues, "support for block multi-queue (def=1)");
MODULE_PARM_DESC(unmap_alignment, "lowest aligned thin provisioning lba (def=0)");
MODULE_PARM_DESC(unmap_granularity, "thin provisioning granularity in blocks (def=1)");
MODULE_PARM_DESC(unmap_max_blocks, "max # of blocks can be unmapped in one cmd (def=0xffffffff)");
@@ -4018,10 +4117,17 @@ static char sdebug_info[256];
static const char * scsi_debug_info(struct Scsi_Host * shp)
{
- sprintf(sdebug_info,
- "scsi_debug, version %s [%s], dev_size_mb=%d, opts=0x%x",
- SDEBUG_VERSION, sdebug_version_date, sdebug_dev_size_mb,
- sdebug_opts);
+ int k;
+
+ k = scnprintf(sdebug_info, sizeof(sdebug_info),
+ "%s: version %s [%s], dev_size_mb=%d, opts=0x%x\n",
+ my_name, SDEBUG_VERSION, sdebug_version_date,
+ sdebug_dev_size_mb, sdebug_opts);
+ if (k >= (sizeof(sdebug_info) - 1))
+ return sdebug_info;
+ scnprintf(sdebug_info + k, sizeof(sdebug_info) - k,
+ "%s: submit_queues=%d, statistics=%d\n", my_name,
+ submit_queues, (int)sdebug_statistics);
return sdebug_info;
}
@@ -4043,7 +4149,7 @@ static int scsi_debug_write_info(struct Scsi_Host *host, char *buffer,
sdebug_verbose = !!(SDEBUG_OPT_NOISE & opts);
sdebug_any_injecting_opt = !!(SDEBUG_OPT_ALL_INJECTING & opts);
if (sdebug_every_nth != 0)
- atomic_set(&sdebug_cmnd_count, 0);
+ tweak_cmnd_count();
return length;
}
@@ -4052,39 +4158,42 @@ static int scsi_debug_write_info(struct Scsi_Host *host, char *buffer,
* output are not atomics so might be inaccurate in a busy system. */
static int scsi_debug_show_info(struct seq_file *m, struct Scsi_Host *host)
{
- int f, l;
- char b[32];
-
- if (sdebug_every_nth > 0)
- snprintf(b, sizeof(b), " (curr:%d)",
- ((SDEBUG_OPT_RARE_TSF & sdebug_opts) ?
- atomic_read(&sdebug_a_tsf) :
- atomic_read(&sdebug_cmnd_count)));
- else
- b[0] = '\0';
-
- seq_printf(m, "scsi_debug adapter driver, version %s [%s]\n"
- "num_tgts=%d, shared (ram) size=%d MB, opts=0x%x, "
- "every_nth=%d%s\n"
- "delay=%d, ndelay=%d, max_luns=%d, q_completions=%d\n"
- "sector_size=%d bytes, cylinders=%d, heads=%d, sectors=%d\n"
- "command aborts=%d; RESETs: device=%d, target=%d, bus=%d, "
- "host=%d\ndix_reads=%d dix_writes=%d dif_errors=%d "
- "usec_in_jiffy=%lu\n",
- SDEBUG_VERSION, sdebug_version_date,
- sdebug_num_tgts, sdebug_dev_size_mb, sdebug_opts,
- sdebug_every_nth, b, sdebug_jdelay, sdebug_ndelay,
- sdebug_max_luns, atomic_read(&sdebug_completions),
- sdebug_sector_size, sdebug_cylinders_per, sdebug_heads,
- sdebug_sectors_per, num_aborts, num_dev_resets,
- num_target_resets, num_bus_resets, num_host_resets,
- dix_reads, dix_writes, dif_errors, TICK_NSEC / 1000);
-
- f = find_first_bit(queued_in_use_bm, sdebug_max_queue);
- if (f != sdebug_max_queue) {
- l = find_last_bit(queued_in_use_bm, sdebug_max_queue);
- seq_printf(m, " %s BUSY: first,last bits set: %d,%d\n",
- "queued_in_use_bm", f, l);
+ int f, j, l;
+ struct sdebug_queue *sqp;
+
+ seq_printf(m, "scsi_debug adapter driver, version %s [%s]\n",
+ SDEBUG_VERSION, sdebug_version_date);
+ seq_printf(m, "num_tgts=%d, %ssize=%d MB, opts=0x%x, every_nth=%d\n",
+ sdebug_num_tgts, "shared (ram) ", sdebug_dev_size_mb,
+ sdebug_opts, sdebug_every_nth);
+ seq_printf(m, "delay=%d, ndelay=%d, max_luns=%d, sector_size=%d %s\n",
+ sdebug_jdelay, sdebug_ndelay, sdebug_max_luns,
+ sdebug_sector_size, "bytes");
+ seq_printf(m, "cylinders=%d, heads=%d, sectors=%d, command aborts=%d\n",
+ sdebug_cylinders_per, sdebug_heads, sdebug_sectors_per,
+ num_aborts);
+ seq_printf(m, "RESETs: device=%d, target=%d, bus=%d, host=%d\n",
+ num_dev_resets, num_target_resets, num_bus_resets,
+ num_host_resets);
+ seq_printf(m, "dix_reads=%d, dix_writes=%d, dif_errors=%d\n",
+ dix_reads, dix_writes, dif_errors);
+ seq_printf(m, "%s=%lu, %s=%d, mq_available=%d, submit_queues=%d\n",
+ "usec_in_jiffy", TICK_NSEC / 1000, "statistics",
+ sdebug_statistics, sdebug_mq_available, submit_queues);
+
+ for (j = 0, sqp = sdebug_q_arr; j < submit_queues; ++j, ++sqp) {
+ seq_printf(m, "queue %d:\n", j);
+ seq_printf(m, " cmnd_count=%d, %s=%d, %s=%d, a_tsf=%d\n",
+ atomic_read(&sqp->cmnd_count),
+ "completions", atomic_read(&sqp->completions),
+ "misqueues", atomic_read(&sqp->misqueues),
+ atomic_read(&sqp->a_tsf));
+ f = find_first_bit(sqp->in_use_bm, sdebug_max_queue);
+ if (f != sdebug_max_queue) {
+ l = find_last_bit(sqp->in_use_bm, sdebug_max_queue);
+ seq_printf(m, " in_use_bm BUSY: %s: %d,%d\n",
+ "first,last bits", f, l);
+ }
}
return 0;
}
@@ -4093,7 +4202,9 @@ static ssize_t delay_show(struct device_driver *ddp, char *buf)
{
return scnprintf(buf, PAGE_SIZE, "%d\n", sdebug_jdelay);
}
-/* Returns -EBUSY if jdelay is being changed and commands are queued */
+/* Returns -EBUSY if jdelay is being changed and commands are queued. The unit
+ * of delay is jiffies.
+ */
static ssize_t delay_store(struct device_driver *ddp, const char *buf,
size_t count)
{
@@ -4102,21 +4213,27 @@ static ssize_t delay_store(struct device_driver *ddp, const char *buf,
if (count > 0 && sscanf(buf, "%d", &jdelay) == 1) {
res = count;
if (sdebug_jdelay != jdelay) {
- unsigned long iflags;
- int k;
-
- spin_lock_irqsave(&queued_arr_lock, iflags);
- k = find_first_bit(queued_in_use_bm, sdebug_max_queue);
- if (k != sdebug_max_queue)
- res = -EBUSY; /* have queued commands */
- else {
+ int j, k;
+ struct sdebug_queue *sqp;
+
+ block_unblock_all_queues(true);
+ for (j = 0, sqp = sdebug_q_arr; j < submit_queues;
+ ++j, ++sqp) {
+ k = find_first_bit(sqp->in_use_bm,
+ sdebug_max_queue);
+ if (k != sdebug_max_queue) {
+ res = -EBUSY; /* queued commands */
+ break;
+ }
+ }
+ if (res > 0) {
/* make sure sdebug_defer instances get
* re-allocated for new delay variant */
free_all_queued();
sdebug_jdelay = jdelay;
sdebug_ndelay = 0;
}
- spin_unlock_irqrestore(&queued_arr_lock, iflags);
+ block_unblock_all_queues(false);
}
return res;
}
@@ -4133,18 +4250,26 @@ static ssize_t ndelay_show(struct device_driver *ddp, char *buf)
static ssize_t ndelay_store(struct device_driver *ddp, const char *buf,
size_t count)
{
- unsigned long iflags;
- int ndelay, res, k;
+ int ndelay, res;
if ((count > 0) && (1 == sscanf(buf, "%d", &ndelay)) &&
- (ndelay >= 0) && (ndelay < 1000000000)) {
+ (ndelay >= 0) && (ndelay < (1000 * 1000 * 1000))) {
res = count;
if (sdebug_ndelay != ndelay) {
- spin_lock_irqsave(&queued_arr_lock, iflags);
- k = find_first_bit(queued_in_use_bm, sdebug_max_queue);
- if (k != sdebug_max_queue)
- res = -EBUSY; /* have queued commands */
- else {
+ int j, k;
+ struct sdebug_queue *sqp;
+
+ block_unblock_all_queues(true);
+ for (j = 0, sqp = sdebug_q_arr; j < submit_queues;
+ ++j, ++sqp) {
+ k = find_first_bit(sqp->in_use_bm,
+ sdebug_max_queue);
+ if (k != sdebug_max_queue) {
+ res = -EBUSY; /* queued commands */
+ break;
+ }
+ }
+ if (res > 0) {
/* make sure sdebug_defer instances get
* re-allocated for new delay variant */
free_all_queued();
@@ -4152,7 +4277,7 @@ static ssize_t ndelay_store(struct device_driver *ddp, const char *buf,
sdebug_jdelay = ndelay ? JDELAY_OVERRIDDEN
: DEF_JDELAY;
}
- spin_unlock_irqrestore(&queued_arr_lock, iflags);
+ block_unblock_all_queues(false);
}
return res;
}
@@ -4185,8 +4310,7 @@ opts_done:
sdebug_opts = opts;
sdebug_verbose = !!(SDEBUG_OPT_NOISE & opts);
sdebug_any_injecting_opt = !!(SDEBUG_OPT_ALL_INJECTING & opts);
- atomic_set(&sdebug_cmnd_count, 0);
- atomic_set(&sdebug_a_tsf, 0);
+ tweak_cmnd_count();
return count;
}
static DRIVER_ATTR_RW(opts);
@@ -4316,7 +4440,7 @@ static ssize_t every_nth_store(struct device_driver *ddp, const char *buf,
if ((count > 0) && (1 == sscanf(buf, "%d", &nth))) {
sdebug_every_nth = nth;
- atomic_set(&sdebug_cmnd_count, 0);
+ tweak_cmnd_count();
return count;
}
return -EINVAL;
@@ -4371,21 +4495,27 @@ static ssize_t max_queue_show(struct device_driver *ddp, char *buf)
static ssize_t max_queue_store(struct device_driver *ddp, const char *buf,
size_t count)
{
- unsigned long iflags;
- int n, k;
+ int j, n, k, a;
+ struct sdebug_queue *sqp;
if ((count > 0) && (1 == sscanf(buf, "%d", &n)) && (n > 0) &&
- (n <= SCSI_DEBUG_CANQUEUE)) {
- spin_lock_irqsave(&queued_arr_lock, iflags);
- k = find_last_bit(queued_in_use_bm, SCSI_DEBUG_CANQUEUE);
+ (n <= SDEBUG_CANQUEUE)) {
+ block_unblock_all_queues(true);
+ k = 0;
+ for (j = 0, sqp = sdebug_q_arr; j < submit_queues;
+ ++j, ++sqp) {
+ a = find_last_bit(sqp->in_use_bm, SDEBUG_CANQUEUE);
+ if (a > k)
+ k = a;
+ }
sdebug_max_queue = n;
- if (SCSI_DEBUG_CANQUEUE == k)
+ if (k == SDEBUG_CANQUEUE)
atomic_set(&retired_max_queue, 0);
else if (k >= n)
atomic_set(&retired_max_queue, k + 1);
else
atomic_set(&retired_max_queue, 0);
- spin_unlock_irqrestore(&queued_arr_lock, iflags);
+ block_unblock_all_queues(false);
return count;
}
return -EINVAL;
@@ -4484,12 +4614,40 @@ static ssize_t vpd_use_hostno_store(struct device_driver *ddp, const char *buf,
}
static DRIVER_ATTR_RW(vpd_use_hostno);
+static ssize_t statistics_show(struct device_driver *ddp, char *buf)
+{
+ return scnprintf(buf, PAGE_SIZE, "%d\n", (int)sdebug_statistics);
+}
+static ssize_t statistics_store(struct device_driver *ddp, const char *buf,
+ size_t count)
+{
+ int n;
+
+ if ((count > 0) && (sscanf(buf, "%d", &n) == 1) && (n >= 0)) {
+ if (n > 0)
+ sdebug_statistics = true;
+ else {
+ clear_queue_stats();
+ sdebug_statistics = false;
+ }
+ return count;
+ }
+ return -EINVAL;
+}
+static DRIVER_ATTR_RW(statistics);
+
static ssize_t sector_size_show(struct device_driver *ddp, char *buf)
{
return scnprintf(buf, PAGE_SIZE, "%u\n", sdebug_sector_size);
}
static DRIVER_ATTR_RO(sector_size);
+static ssize_t submit_queues_show(struct device_driver *ddp, char *buf)
+{
+ return scnprintf(buf, PAGE_SIZE, "%d\n", submit_queues);
+}
+static DRIVER_ATTR_RO(submit_queues);
+
static ssize_t dix_show(struct device_driver *ddp, char *buf)
{
return scnprintf(buf, PAGE_SIZE, "%d\n", sdebug_dix);
@@ -4610,6 +4768,8 @@ static struct attribute *sdebug_drv_attrs[] = {
&driver_attr_add_host.attr,
&driver_attr_vpd_use_hostno.attr,
&driver_attr_sector_size.attr,
+ &driver_attr_statistics.attr,
+ &driver_attr_submit_queues.attr,
&driver_attr_dix.attr,
&driver_attr_dif.attr,
&driver_attr_guard.attr,
@@ -4632,8 +4792,6 @@ static int __init scsi_debug_init(void)
int k;
int ret;
- atomic_set(&sdebug_cmnd_count, 0);
- atomic_set(&sdebug_completions, 0);
atomic_set(&retired_max_queue, 0);
if (sdebug_ndelay >= 1000 * 1000 * 1000) {
@@ -4692,6 +4850,17 @@ static int __init scsi_debug_init(void)
return -EINVAL;
}
+ if (submit_queues < 1) {
+ pr_err("submit_queues must be 1 or more\n");
+ return -EINVAL;
+ }
+ sdebug_q_arr = kcalloc(submit_queues, sizeof(struct sdebug_queue),
+ GFP_KERNEL);
+ if (sdebug_q_arr == NULL)
+ return -ENOMEM;
+ for (k = 0; k < submit_queues; ++k)
+ spin_lock_init(&sdebug_q_arr[k].qc_lock);
+
if (sdebug_dev_size_mb < 1)
sdebug_dev_size_mb = 1; /* force minimum 1 MB ramdisk */
sz = (unsigned long)sdebug_dev_size_mb * 1048576;
@@ -4719,7 +4888,8 @@ static int __init scsi_debug_init(void)
fake_storep = vmalloc(sz);
if (NULL == fake_storep) {
pr_err("out of memory, 1\n");
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto free_q_arr;
}
memset(fake_storep, 0, sz);
if (sdebug_num_parts > 0)
@@ -4758,7 +4928,8 @@ static int __init scsi_debug_init(void)
sdebug_unmap_granularity <=
sdebug_unmap_alignment) {
pr_err("ERR: unmap_granularity <= unmap_alignment\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto free_vm;
}
map_size = lba_to_map_index(sdebug_store_sectors - 1) + 1;
@@ -4819,7 +4990,8 @@ free_vm:
vfree(map_storep);
vfree(dif_storep);
vfree(fake_storep);
-
+free_q_arr:
+ kfree(sdebug_q_arr);
return ret;
}
@@ -4837,6 +5009,7 @@ static void __exit scsi_debug_exit(void)
vfree(dif_storep);
vfree(fake_storep);
+ kfree(sdebug_q_arr);
}
device_initcall(scsi_debug_init);
@@ -4925,62 +5098,45 @@ static void sdebug_remove_adapter(void)
static int sdebug_change_qdepth(struct scsi_device *sdev, int qdepth)
{
int num_in_q = 0;
- unsigned long iflags;
struct sdebug_dev_info *devip;
- spin_lock_irqsave(&queued_arr_lock, iflags);
+ block_unblock_all_queues(true);
devip = (struct sdebug_dev_info *)sdev->hostdata;
if (NULL == devip) {
- spin_unlock_irqrestore(&queued_arr_lock, iflags);
+ block_unblock_all_queues(false);
return -ENODEV;
}
num_in_q = atomic_read(&devip->num_in_q);
- spin_unlock_irqrestore(&queued_arr_lock, iflags);
if (qdepth < 1)
qdepth = 1;
- /* allow to exceed max host queued_arr elements for testing */
- if (qdepth > SCSI_DEBUG_CANQUEUE + 10)
- qdepth = SCSI_DEBUG_CANQUEUE + 10;
+ /* allow to exceed max host qc_arr elements for testing */
+ if (qdepth > SDEBUG_CANQUEUE + 10)
+ qdepth = SDEBUG_CANQUEUE + 10;
scsi_change_queue_depth(sdev, qdepth);
if (SDEBUG_OPT_Q_NOISE & sdebug_opts) {
- sdev_printk(KERN_INFO, sdev,
- "%s: qdepth=%d, num_in_q=%d\n",
+ sdev_printk(KERN_INFO, sdev, "%s: qdepth=%d, num_in_q=%d\n",
__func__, qdepth, num_in_q);
}
+ block_unblock_all_queues(false);
return sdev->queue_depth;
}
-static int check_inject(struct scsi_cmnd *scp)
+static bool fake_timeout(struct sdebug_queue *sqp, struct scsi_cmnd *scp)
{
- struct sdebug_scmd_extra_t *ep = scsi_cmd_priv(scp);
-
- memset(ep, 0, sizeof(struct sdebug_scmd_extra_t));
-
- if (atomic_inc_return(&sdebug_cmnd_count) >= abs(sdebug_every_nth)) {
- atomic_set(&sdebug_cmnd_count, 0);
+ if (sqp == NULL)
+ sqp = get_queue();
+ if (0 == (atomic_read(&sqp->cmnd_count) % abs(sdebug_every_nth))) {
if (sdebug_every_nth < -1)
sdebug_every_nth = -1;
if (SDEBUG_OPT_TIMEOUT & sdebug_opts)
- return 1; /* ignore command causing timeout */
+ return true; /* ignore command causing timeout */
else if (SDEBUG_OPT_MAC_TIMEOUT & sdebug_opts &&
scsi_medium_access_command(scp))
- return 1; /* time out reads and writes */
- if (sdebug_any_injecting_opt) {
- if (SDEBUG_OPT_RECOVERED_ERR & sdebug_opts)
- ep->inj_recovered = true;
- if (SDEBUG_OPT_TRANSPORT_ERR & sdebug_opts)
- ep->inj_transport = true;
- if (SDEBUG_OPT_DIF_ERR & sdebug_opts)
- ep->inj_dif = true;
- if (SDEBUG_OPT_DIX_ERR & sdebug_opts)
- ep->inj_dix = true;
- if (SDEBUG_OPT_SHORT_TRANSFER & sdebug_opts)
- ep->inj_short = true;
- }
+ return true; /* time out reads and writes */
}
- return 0;
+ return false;
}
static int scsi_debug_queuecommand(struct Scsi_Host *shost,
@@ -4991,6 +5147,7 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,
const struct opcode_info_t *oip;
const struct opcode_info_t *r_oip;
struct sdebug_dev_info *devip;
+ struct sdebug_queue *sqp = NULL;
u8 *cmd = scp->cmnd;
int (*r_pfp)(struct scsi_cmnd *, struct sdebug_dev_info *);
int k, na;
@@ -5001,6 +5158,10 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,
bool has_wlun_rl;
scsi_set_resid(scp, 0);
+ if (sdebug_statistics) {
+ sqp = get_queue();
+ atomic_inc(&sqp->cmnd_count);
+ }
if (unlikely(sdebug_verbose &&
!(SDEBUG_OPT_NO_CDB_NOISE & sdebug_opts))) {
char b[120];
@@ -5093,7 +5254,7 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,
if (errsts)
goto check_cond;
}
- if (unlikely((F_M_ACCESS & flags) && devip->stopped)) {
+ if (unlikely((F_M_ACCESS & flags) && atomic_read(&devip->stopped))) {
mk_sense_buffer(scp, NOT_READY, LOGICAL_UNIT_NOT_READY, 0x2);
if (sdebug_verbose)
sdev_printk(KERN_INFO, sdp, "%s reports: Not ready: "
@@ -5105,7 +5266,7 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,
if (sdebug_fake_rw && (F_FAKE_RW & flags))
goto fini;
if (unlikely(sdebug_every_nth)) {
- if (check_inject(scp))
+ if (fake_timeout(sqp, scp))
return 0; /* ignore command: make trouble */
}
if (likely(oip->pfp))
@@ -5139,7 +5300,7 @@ static struct scsi_host_template sdebug_driver_template = {
.eh_target_reset_handler = scsi_debug_target_reset,
.eh_bus_reset_handler = scsi_debug_bus_reset,
.eh_host_reset_handler = scsi_debug_host_reset,
- .can_queue = SCSI_DEBUG_CANQUEUE,
+ .can_queue = SDEBUG_CANQUEUE,
.this_id = 7,
.sg_tablesize = SG_MAX_SEGMENTS,
.cmd_per_lun = DEF_CMD_PER_LUN,
@@ -5147,7 +5308,6 @@ static struct scsi_host_template sdebug_driver_template = {
.use_clustering = DISABLE_CLUSTERING,
.module = THIS_MODULE,
.track_queue_depth = 1,
- .cmd_size = sizeof(struct sdebug_scmd_extra_t),
};
static int sdebug_driver_probe(struct device * dev)
@@ -5168,6 +5328,16 @@ static int sdebug_driver_probe(struct device * dev)
error = -ENODEV;
return error;
}
+ if (submit_queues > nr_cpu_ids) {
+ pr_warn("%s: trim submit_queues (was %d) to nr_cpu_ids=%d\n",
+ my_name, submit_queues, nr_cpu_ids);
+ submit_queues = nr_cpu_ids;
+ }
+ /* Decide whether to tell scsi subsystem that we want mq */
+ /* Following should give the same answer for each host */
+ sdebug_mq_available = shost_use_blk_mq(hpnt);
+ if (sdebug_mq_available && (submit_queues > 1))
+ hpnt->nr_hw_queues = submit_queues;
sdbg_host->shost = hpnt;
*((struct sdebug_host_info **)hpnt->hostdata) = sdbg_host;
@@ -5225,6 +5395,8 @@ static int sdebug_driver_probe(struct device * dev)
sdebug_verbose = !!(SDEBUG_OPT_NOISE & sdebug_opts);
sdebug_any_injecting_opt = !!(SDEBUG_OPT_ALL_INJECTING & sdebug_opts);
+ if (sdebug_every_nth) /* need stats counters for every_nth */
+ sdebug_statistics = true;
error = scsi_add_host(hpnt, &sdbg_host->dev);
if (error) {
pr_err("scsi_add_host failed\n");
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2.5 4/6] scsi_debug: vpd and mode page work
2016-05-01 2:44 [PATCH v2.5 0/6] scsi_debug: rebase second half of series Douglas Gilbert
` (2 preceding siblings ...)
2016-05-01 2:44 ` [PATCH v2.5 3/6] scsi_debug: add multiple queue support Douglas Gilbert
@ 2016-05-01 2:44 ` Douglas Gilbert
2016-05-02 8:38 ` Hannes Reinecke
2016-05-01 2:44 ` [PATCH v2.5 5/6] scsi_debug: uuid for lu name Douglas Gilbert
2016-05-01 2:44 ` [PATCH v2.5 6/6] scsi_debug: use locally assigned naa Douglas Gilbert
5 siblings, 1 reply; 24+ messages in thread
From: Douglas Gilbert @ 2016-05-01 2:44 UTC (permalink / raw)
To: linux-scsi; +Cc: martin.petersen, tomas.winkler, emilne, bart.vanassche
Cleanup some mode and vpd pages. Stop reporting SBC (disk) pages
when peripheral type is something else (e.g. tape). Update
version descriptors. Expand LBPRZ flag handling.
Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
---
drivers/scsi/scsi_debug.c | 187 ++++++++++++++++++++++++++--------------------
1 file changed, 108 insertions(+), 79 deletions(-)
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 0932111..814067d 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -125,7 +125,7 @@ static const char *sdebug_version_date = "20160430";
#define DEF_PHYSBLK_EXP 0
#define DEF_PTYPE TYPE_DISK
#define DEF_REMOVABLE false
-#define DEF_SCSI_LEVEL 6 /* INQUIRY, byte2 [6->SPC-4] */
+#define DEF_SCSI_LEVEL 7 /* INQUIRY, byte2 [6->SPC-4; 7->SPC-5] */
#define DEF_SECTOR_SIZE 512
#define DEF_UNMAP_ALIGNMENT 0
#define DEF_UNMAP_GRANULARITY 1
@@ -657,7 +657,11 @@ static const int device_qfull_result =
(DID_OK << 16) | (COMMAND_COMPLETE << 8) | SAM_STAT_TASK_SET_FULL;
-static inline unsigned int scsi_debug_lbp(void)
+/* Only do the extra work involved in logical block provisioning if one or
+ * more of the lbpu, lbpws or lbpws10 parameters are given and we are doing
+ * real reads and writes (i.e. not skipping them for speed).
+ */
+static inline bool scsi_debug_lbp(void)
{
return 0 == sdebug_fake_rw &&
(sdebug_lbpu || sdebug_lbpws || sdebug_lbpws10);
@@ -918,10 +922,10 @@ static const u64 naa5_comp_b = 0x5333333000000000ULL;
static const u64 naa5_comp_c = 0x5111111000000000ULL;
/* Device identification VPD page. Returns number of bytes placed in arr */
-static int inquiry_evpd_83(unsigned char * arr, int port_group_id,
- int target_dev_id, int dev_id_num,
- const char * dev_id_str,
- int dev_id_str_len)
+static int inquiry_vpd_83(unsigned char *arr, int port_group_id,
+ int target_dev_id, int dev_id_num,
+ const char *dev_id_str,
+ int dev_id_str_len)
{
int num, port_a;
char b[32];
@@ -1000,14 +1004,14 @@ static unsigned char vpd84_data[] = {
};
/* Software interface identification VPD page */
-static int inquiry_evpd_84(unsigned char * arr)
+static int inquiry_vpd_84(unsigned char *arr)
{
memcpy(arr, vpd84_data, sizeof(vpd84_data));
return sizeof(vpd84_data);
}
/* Management network addresses VPD page */
-static int inquiry_evpd_85(unsigned char * arr)
+static int inquiry_vpd_85(unsigned char *arr)
{
int num = 0;
const char * na1 = "https://www.kernel.org/config";
@@ -1042,7 +1046,7 @@ static int inquiry_evpd_85(unsigned char * arr)
}
/* SCSI ports VPD page */
-static int inquiry_evpd_88(unsigned char * arr, int target_dev_id)
+static int inquiry_vpd_88(unsigned char *arr, int target_dev_id)
{
int num = 0;
int port_a, port_b;
@@ -1129,7 +1133,7 @@ static unsigned char vpd89_data[] = {
};
/* ATA Information VPD page */
-static int inquiry_evpd_89(unsigned char * arr)
+static int inquiry_vpd_89(unsigned char *arr)
{
memcpy(arr, vpd89_data, sizeof(vpd89_data));
return sizeof(vpd89_data);
@@ -1144,7 +1148,7 @@ static unsigned char vpdb0_data[] = {
};
/* Block limits VPD page (SBC-3) */
-static int inquiry_evpd_b0(unsigned char * arr)
+static int inquiry_vpd_b0(unsigned char *arr)
{
unsigned int gran;
@@ -1187,7 +1191,7 @@ static int inquiry_evpd_b0(unsigned char * arr)
}
/* Block device characteristics VPD page (SBC-3) */
-static int inquiry_evpd_b1(unsigned char *arr)
+static int inquiry_vpd_b1(unsigned char *arr)
{
memset(arr, 0, 0x3c);
arr[0] = 0;
@@ -1198,24 +1202,22 @@ static int inquiry_evpd_b1(unsigned char *arr)
return 0x3c;
}
-/* Logical block provisioning VPD page (SBC-3) */
-static int inquiry_evpd_b2(unsigned char *arr)
+/* Logical block provisioning VPD page (SBC-4) */
+static int inquiry_vpd_b2(unsigned char *arr)
{
memset(arr, 0, 0x4);
arr[0] = 0; /* threshold exponent */
-
if (sdebug_lbpu)
arr[1] = 1 << 7;
-
if (sdebug_lbpws)
arr[1] |= 1 << 6;
-
if (sdebug_lbpws10)
arr[1] |= 1 << 5;
-
- if (sdebug_lbprz)
- arr[1] |= 1 << 2;
-
+ if (sdebug_lbprz && scsi_debug_lbp())
+ arr[1] |= (sdebug_lbprz & 0x7) << 2; /* sbc4r07 and later */
+ /* anc_sup=0; dp=0 (no provisioning group descriptor) */
+ /* minimum_percentage=0; provisioning_type=0 (unknown) */
+ /* threshold_percentage=0 */
return 0x4;
}
@@ -1228,12 +1230,13 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
unsigned char * arr;
unsigned char *cmd = scp->cmnd;
int alloc_len, n, ret;
- bool have_wlun;
+ bool have_wlun, is_disk;
alloc_len = get_unaligned_be16(cmd + 3);
arr = kzalloc(SDEBUG_MAX_INQ_ARR_SZ, GFP_ATOMIC);
if (! arr)
return DID_REQUEUE << 16;
+ is_disk = (sdebug_ptype == TYPE_DISK);
have_wlun = scsi_is_wlun(scp->device->lun);
if (have_wlun)
pq_pdt = TYPE_WLUN; /* present, wlun */
@@ -1271,11 +1274,12 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
arr[n++] = 0x86; /* extended inquiry */
arr[n++] = 0x87; /* mode page policy */
arr[n++] = 0x88; /* SCSI ports */
- arr[n++] = 0x89; /* ATA information */
- arr[n++] = 0xb0; /* Block limits (SBC) */
- arr[n++] = 0xb1; /* Block characteristics (SBC) */
- if (scsi_debug_lbp()) /* Logical Block Prov. (SBC) */
- arr[n++] = 0xb2;
+ if (is_disk) { /* SBC only */
+ arr[n++] = 0x89; /* ATA information */
+ arr[n++] = 0xb0; /* Block limits */
+ arr[n++] = 0xb1; /* Block characteristics */
+ arr[n++] = 0xb2; /* Logical Block Prov */
+ }
arr[3] = n - 4; /* number of supported VPD pages */
} else if (0x80 == cmd[2]) { /* unit serial number */
arr[1] = cmd[2]; /*sanity */
@@ -1283,21 +1287,21 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
memcpy(&arr[4], lu_id_str, len);
} else if (0x83 == cmd[2]) { /* device identification */
arr[1] = cmd[2]; /*sanity */
- arr[3] = inquiry_evpd_83(&arr[4], port_group_id,
- target_dev_id, lu_id_num,
- lu_id_str, len);
+ arr[3] = inquiry_vpd_83(&arr[4], port_group_id,
+ target_dev_id, lu_id_num,
+ lu_id_str, len);
} else if (0x84 == cmd[2]) { /* Software interface ident. */
arr[1] = cmd[2]; /*sanity */
- arr[3] = inquiry_evpd_84(&arr[4]);
+ arr[3] = inquiry_vpd_84(&arr[4]);
} else if (0x85 == cmd[2]) { /* Management network addresses */
arr[1] = cmd[2]; /*sanity */
- arr[3] = inquiry_evpd_85(&arr[4]);
+ arr[3] = inquiry_vpd_85(&arr[4]);
} else if (0x86 == cmd[2]) { /* extended inquiry */
arr[1] = cmd[2]; /*sanity */
arr[3] = 0x3c; /* number of following entries */
if (sdebug_dif == SD_DIF_TYPE3_PROTECTION)
arr[4] = 0x4; /* SPT: GRD_CHK:1 */
- else if (sdebug_dif)
+ else if (have_dif_prot)
arr[4] = 0x5; /* SPT: GRD_CHK:1, REF_CHK:1 */
else
arr[4] = 0x0; /* no protection stuff */
@@ -1311,20 +1315,20 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
arr[10] = 0x82; /* mlus, per initiator port */
} else if (0x88 == cmd[2]) { /* SCSI Ports */
arr[1] = cmd[2]; /*sanity */
- arr[3] = inquiry_evpd_88(&arr[4], target_dev_id);
- } else if (0x89 == cmd[2]) { /* ATA information */
+ arr[3] = inquiry_vpd_88(&arr[4], target_dev_id);
+ } else if (is_disk && 0x89 == cmd[2]) { /* ATA information */
arr[1] = cmd[2]; /*sanity */
- n = inquiry_evpd_89(&arr[4]);
+ n = inquiry_vpd_89(&arr[4]);
put_unaligned_be16(n, arr + 2);
- } else if (0xb0 == cmd[2]) { /* Block limits (SBC) */
+ } else if (is_disk && 0xb0 == cmd[2]) { /* Block limits */
arr[1] = cmd[2]; /*sanity */
- arr[3] = inquiry_evpd_b0(&arr[4]);
- } else if (0xb1 == cmd[2]) { /* Block characteristics (SBC) */
+ arr[3] = inquiry_vpd_b0(&arr[4]);
+ } else if (is_disk && 0xb1 == cmd[2]) { /* Block char. */
arr[1] = cmd[2]; /*sanity */
- arr[3] = inquiry_evpd_b1(&arr[4]);
- } else if (0xb2 == cmd[2]) { /* Logical Block Prov. (SBC) */
+ arr[3] = inquiry_vpd_b1(&arr[4]);
+ } else if (is_disk && 0xb2 == cmd[2]) { /* LB Prov. */
arr[1] = cmd[2]; /*sanity */
- arr[3] = inquiry_evpd_b2(&arr[4]);
+ arr[3] = inquiry_vpd_b2(&arr[4]);
} else {
mk_sense_invalid_fld(scp, SDEB_IN_CDB, 2, -1);
kfree(arr);
@@ -1351,15 +1355,17 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
memcpy(&arr[16], inq_product_id, 16);
memcpy(&arr[32], inq_product_rev, 4);
/* version descriptors (2 bytes each) follow */
- arr[58] = 0x0; arr[59] = 0xa2; /* SAM-5 rev 4 */
- arr[60] = 0x4; arr[61] = 0x68; /* SPC-4 rev 37 */
+ put_unaligned_be16(0xc0, arr + 58); /* SAM-6 no version claimed */
+ put_unaligned_be16(0x5c0, arr + 60); /* SPC-5 no version claimed */
n = 62;
- if (sdebug_ptype == TYPE_DISK) {
- arr[n++] = 0x4; arr[n++] = 0xc5; /* SBC-4 rev 36 */
- } else if (sdebug_ptype == TYPE_TAPE) {
- arr[n++] = 0x5; arr[n++] = 0x25; /* SSC-4 rev 3 */
- }
- arr[n++] = 0x20; arr[n++] = 0xe6; /* SPL-3 rev 7 */
+ if (is_disk) { /* SBC-4 no version claimed */
+ put_unaligned_be16(0x600, arr + n);
+ n += 2;
+ } else if (sdebug_ptype == TYPE_TAPE) { /* SSC-4 rev 3 */
+ put_unaligned_be16(0x525, arr + n);
+ n += 2;
+ }
+ 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));
kfree(arr);
@@ -1495,13 +1501,17 @@ static int resp_readcap16(struct scsi_cmnd * scp,
if (scsi_debug_lbp()) {
arr[14] |= 0x80; /* LBPME */
- if (sdebug_lbprz)
- arr[14] |= 0x40; /* LBPRZ */
+ /* from sbc4r07, this LBPRZ field is 1 bit, but the LBPRZ in
+ * the LB Provisioning VPD page is 3 bits. Note that lbprz=2
+ * in the wider field maps to 0 in this field.
+ */
+ if (sdebug_lbprz & 1) /* precisely what the draft requires */
+ arr[14] |= 0x40;
}
arr[15] = sdebug_lowest_aligned & 0xff;
- if (sdebug_dif) {
+ if (have_dif_prot) {
arr[12] = (sdebug_dif - 1) << 1; /* P_TYPE */
arr[12] |= 1; /* PROT_EN */
}
@@ -1931,22 +1941,23 @@ static int resp_sas_sha_m_spg(unsigned char * p, int pcontrol)
static int resp_mode_sense(struct scsi_cmnd *scp,
struct sdebug_dev_info *devip)
{
- unsigned char dbd, llbaa;
int pcontrol, pcode, subpcode, bd_len;
unsigned char dev_spec;
- int alloc_len, msense_6, offset, len, target_dev_id;
+ int alloc_len, offset, len, target_dev_id;
int target = scp->device->id;
unsigned char * ap;
unsigned char arr[SDEBUG_MAX_MSENSE_SZ];
unsigned char *cmd = scp->cmnd;
+ bool dbd, llbaa, msense_6, is_disk, bad_pcode;
- dbd = !!(cmd[1] & 0x8);
+ dbd = !!(cmd[1] & 0x8); /* disable block descriptors */
pcontrol = (cmd[2] & 0xc0) >> 6;
pcode = cmd[2] & 0x3f;
subpcode = cmd[3];
msense_6 = (MODE_SENSE == cmd[0]);
- llbaa = msense_6 ? 0 : !!(cmd[1] & 0x10);
- if ((sdebug_ptype == TYPE_DISK) && (dbd == 0))
+ llbaa = msense_6 ? false : !!(cmd[1] & 0x10);
+ is_disk = (sdebug_ptype == TYPE_DISK);
+ if (is_disk && !dbd)
bd_len = llbaa ? 16 : 8;
else
bd_len = 0;
@@ -1959,7 +1970,7 @@ static int resp_mode_sense(struct scsi_cmnd *scp,
target_dev_id = ((devip->sdbg_host->shost->host_no + 1) * 2000) +
(devip->target * 1000) - 3;
/* for disks set DPOFUA bit and clear write protect (WP) bit */
- if (sdebug_ptype == TYPE_DISK)
+ if (is_disk)
dev_spec = 0x10; /* =0x90 if WP=1 implies read-only */
else
dev_spec = 0x0;
@@ -1998,6 +2009,8 @@ static int resp_mode_sense(struct scsi_cmnd *scp,
mk_sense_invalid_fld(scp, SDEB_IN_CDB, 3, -1);
return check_condition_result;
}
+ bad_pcode = false;
+
switch (pcode) {
case 0x1: /* Read-Write error recovery page, direct access */
len = resp_err_recov_pg(ap, pcontrol, target);
@@ -2008,12 +2021,18 @@ static int resp_mode_sense(struct scsi_cmnd *scp,
offset += len;
break;
case 0x3: /* Format device page, direct access */
- len = resp_format_pg(ap, pcontrol, target);
- offset += len;
+ if (is_disk) {
+ len = resp_format_pg(ap, pcontrol, target);
+ offset += len;
+ } else
+ bad_pcode = true;
break;
case 0x8: /* Caching page, direct access */
- len = resp_caching_pg(ap, pcontrol, target);
- offset += len;
+ if (is_disk) {
+ len = resp_caching_pg(ap, pcontrol, target);
+ offset += len;
+ } else
+ bad_pcode = true;
break;
case 0xa: /* Control Mode page, all devices */
len = resp_ctrl_m_pg(ap, pcontrol, target);
@@ -2042,8 +2061,12 @@ static int resp_mode_sense(struct scsi_cmnd *scp,
if ((0 == subpcode) || (0xff == subpcode)) {
len = resp_err_recov_pg(ap, pcontrol, target);
len += resp_disconnect_pg(ap + len, pcontrol, target);
- len += resp_format_pg(ap + len, pcontrol, target);
- len += resp_caching_pg(ap + len, pcontrol, target);
+ if (is_disk) {
+ len += resp_format_pg(ap + len, pcontrol,
+ target);
+ len += resp_caching_pg(ap + len, pcontrol,
+ target);
+ }
len += resp_ctrl_m_pg(ap + len, pcontrol, target);
len += resp_sas_sf_m_pg(ap + len, pcontrol, target);
if (0xff == subpcode) {
@@ -2052,13 +2075,17 @@ static int resp_mode_sense(struct scsi_cmnd *scp,
len += resp_sas_sha_m_spg(ap + len, pcontrol);
}
len += resp_iec_m_pg(ap + len, pcontrol, target);
+ offset += len;
} else {
mk_sense_invalid_fld(scp, SDEB_IN_CDB, 3, -1);
return check_condition_result;
}
- offset += len;
break;
default:
+ bad_pcode = true;
+ break;
+ }
+ if (bad_pcode) {
mk_sense_invalid_fld(scp, SDEB_IN_CDB, 2, 5);
return check_condition_result;
}
@@ -2749,9 +2776,10 @@ static void unmap_region(sector_t lba, unsigned int len)
lba + sdebug_unmap_granularity <= end &&
index < map_size) {
clear_bit(index, map_storep);
- if (sdebug_lbprz) {
+ if (sdebug_lbprz) { /* for LBPRZ=2 return 0xff_s */
memset(fake_storep +
- lba * sdebug_sector_size, 0,
+ lba * sdebug_sector_size,
+ (sdebug_lbprz & 1) ? 0 : 0xff,
sdebug_sector_size *
sdebug_unmap_granularity);
}
@@ -4086,7 +4114,8 @@ MODULE_PARM_DESC(host_lock, "host_lock is ignored (def=0)");
MODULE_PARM_DESC(lbpu, "enable LBP, support UNMAP command (def=0)");
MODULE_PARM_DESC(lbpws, "enable LBP, support WRITE SAME(16) with UNMAP bit (def=0)");
MODULE_PARM_DESC(lbpws10, "enable LBP, support WRITE SAME(10) with UNMAP bit (def=0)");
-MODULE_PARM_DESC(lbprz, "unmapped blocks return 0 on read (def=1)");
+MODULE_PARM_DESC(lbprz,
+ "on read unmapped LBs return 0 when 1 (def), return 0xff when 2");
MODULE_PARM_DESC(lowest_aligned, "lowest aligned lba (def=0)");
MODULE_PARM_DESC(max_luns, "number of LUNs per target to simulate(def=1)");
MODULE_PARM_DESC(max_queue, "max number of queued commands (1 to max(def))");
@@ -4100,7 +4129,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(ptype, "SCSI peripheral type(def=0[disk])");
MODULE_PARM_DESC(removable, "claim to have removable media (def=0)");
-MODULE_PARM_DESC(scsi_level, "SCSI level to simulate(def=6[SPC-4])");
+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)");
MODULE_PARM_DESC(statistics, "collect statistics on commands, queues (def=1)");
MODULE_PARM_DESC(strict, "stricter checks: reserved field in cdb (def=0)");
@@ -4113,21 +4142,21 @@ MODULE_PARM_DESC(virtual_gb, "virtual gigabyte (GiB) size (def=0 -> use dev_size
MODULE_PARM_DESC(vpd_use_hostno, "0 -> dev ids ignore hostno (def=1 -> unique dev ids)");
MODULE_PARM_DESC(write_same_length, "Maximum blocks per WRITE SAME cmd (def=0xffff)");
-static char sdebug_info[256];
+#define SDEBUG_INFO_LEN 256
+static char sdebug_info[SDEBUG_INFO_LEN];
static const char * scsi_debug_info(struct Scsi_Host * shp)
{
int k;
- k = scnprintf(sdebug_info, sizeof(sdebug_info),
- "%s: version %s [%s], dev_size_mb=%d, opts=0x%x\n",
- my_name, SDEBUG_VERSION, sdebug_version_date,
- sdebug_dev_size_mb, sdebug_opts);
- if (k >= (sizeof(sdebug_info) - 1))
+ k = scnprintf(sdebug_info, SDEBUG_INFO_LEN, "%s: version %s [%s]\n",
+ my_name, SDEBUG_VERSION, sdebug_version_date);
+ if (k >= (SDEBUG_INFO_LEN - 1))
return sdebug_info;
- scnprintf(sdebug_info + k, sizeof(sdebug_info) - k,
- "%s: submit_queues=%d, statistics=%d\n", my_name,
- submit_queues, (int)sdebug_statistics);
+ scnprintf(sdebug_info + k, SDEBUG_INFO_LEN - k,
+ " dev_size_mb=%d, opts=0x%x, submit_queues=%d, %s=%d",
+ sdebug_dev_size_mb, sdebug_opts, submit_queues,
+ "statistics", (int)sdebug_statistics);
return sdebug_info;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2.5 5/6] scsi_debug: uuid for lu name
2016-05-01 2:44 [PATCH v2.5 0/6] scsi_debug: rebase second half of series Douglas Gilbert
` (3 preceding siblings ...)
2016-05-01 2:44 ` [PATCH v2.5 4/6] scsi_debug: vpd and mode page work Douglas Gilbert
@ 2016-05-01 2:44 ` Douglas Gilbert
2016-05-02 8:38 ` Hannes Reinecke
2016-05-04 22:37 ` Bart Van Assche
2016-05-01 2:44 ` [PATCH v2.5 6/6] scsi_debug: use locally assigned naa Douglas Gilbert
5 siblings, 2 replies; 24+ messages in thread
From: Douglas Gilbert @ 2016-05-01 2:44 UTC (permalink / raw)
To: linux-scsi; +Cc: martin.petersen, tomas.winkler, emilne, bart.vanassche
Permit changing of a LU name from a (fake) IEEE registered NAA (5)
to a locally assigned UUID. Using a UUID (RFC 4122) for a SCSI
designation descriptor (e.g. a LU name) was added in spc5r08.pdf
(a draft INCITS standard) on 25 January 2016. Add parameter
uuid_ctl to use a separate UUID for each LU (storage device) name.
Additional option for all LU names to have the same UUID (since
their storage is shared). Previous action of using NAA identifier
for LU name remains the default.
Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
---
drivers/scsi/scsi_debug.c | 61 +++++++++++++++++++++++++++++++++++++++--------
1 file changed, 51 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 814067d..c14811e 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -41,6 +41,7 @@
#include <linux/interrupt.h>
#include <linux/atomic.h>
#include <linux/hrtimer.h>
+#include <linux/uuid.h>
#include <net/checksum.h>
@@ -137,6 +138,7 @@ static const char *sdebug_version_date = "20160430";
#define DEF_STRICT 0
#define DEF_STATISTICS true
#define DEF_SUBMIT_QUEUES 1
+#define DEF_UUID_CTL 0
#define JDELAY_OVERRIDDEN -9999
#define SDEBUG_LUN_0_VAL 0
@@ -241,6 +243,7 @@ struct sdebug_dev_info {
unsigned int channel;
unsigned int target;
u64 lun;
+ uuid_be lu_name;
struct sdebug_host_info *sdbg_host;
unsigned long uas_bm[1];
atomic_t num_in_q;
@@ -596,6 +599,7 @@ static unsigned int sdebug_unmap_granularity = DEF_UNMAP_GRANULARITY;
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_removable = DEF_REMOVABLE;
static bool sdebug_clustering;
static bool sdebug_host_lock = DEF_HOST_LOCK;
@@ -924,8 +928,8 @@ static const u64 naa5_comp_c = 0x5111111000000000ULL;
/* Device identification VPD page. Returns number of bytes placed in arr */
static int inquiry_vpd_83(unsigned char *arr, int port_group_id,
int target_dev_id, int dev_id_num,
- const char *dev_id_str,
- int dev_id_str_len)
+ const char *dev_id_str, int dev_id_str_len,
+ const uuid_be *lu_name)
{
int num, port_a;
char b[32];
@@ -942,13 +946,25 @@ static int inquiry_vpd_83(unsigned char *arr, int port_group_id,
arr[3] = num;
num += 4;
if (dev_id_num >= 0) {
- /* NAA-5, Logical unit identifier (binary) */
- arr[num++] = 0x1; /* binary (not necessarily sas) */
- arr[num++] = 0x3; /* PIV=0, lu, naa */
- arr[num++] = 0x0;
- arr[num++] = 0x8;
- put_unaligned_be64(naa5_comp_b + dev_id_num, arr + num);
- num += 8;
+ if (sdebug_uuid_ctl) {
+ /* Locally assigned UUID */
+ arr[num++] = 0x1; /* binary (not necessarily sas) */
+ arr[num++] = 0xa; /* PIV=0, lu, naa */
+ arr[num++] = 0x0;
+ arr[num++] = 0x12;
+ arr[num++] = 0x10; /* uuid type=1, locally assigned */
+ arr[num++] = 0x0;
+ memcpy(arr + num, lu_name, 16);
+ num += 16;
+ } else {
+ /* NAA-5, Logical unit identifier (binary) */
+ arr[num++] = 0x1; /* binary (not necessarily sas) */
+ arr[num++] = 0x3; /* PIV=0, lu, naa */
+ arr[num++] = 0x0;
+ arr[num++] = 0x8;
+ put_unaligned_be64(naa5_comp_b + dev_id_num, arr + num);
+ num += 8;
+ }
/* Target relative port number */
arr[num++] = 0x61; /* proto=sas, binary */
arr[num++] = 0x94; /* PIV=1, target port, rel port */
@@ -1289,7 +1305,8 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
arr[1] = cmd[2]; /*sanity */
arr[3] = inquiry_vpd_83(&arr[4], port_group_id,
target_dev_id, lu_id_num,
- lu_id_str, len);
+ lu_id_str, len,
+ &devip->lu_name);
} else if (0x84 == cmd[2]) { /* Software interface ident. */
arr[1] = cmd[2]; /*sanity */
arr[3] = inquiry_vpd_84(&arr[4]);
@@ -3487,6 +3504,9 @@ static void sdebug_q_cmd_wq_complete(struct work_struct *work)
sdebug_q_cmd_complete(sd_dp);
}
+static bool got_shared_uuid;
+static uuid_be shared_uuid;
+
static struct sdebug_dev_info *sdebug_device_create(
struct sdebug_host_info *sdbg_host, gfp_t flags)
{
@@ -3494,6 +3514,17 @@ static struct sdebug_dev_info *sdebug_device_create(
devip = kzalloc(sizeof(*devip), flags);
if (devip) {
+ if (sdebug_uuid_ctl == 1)
+ uuid_be_gen(&devip->lu_name);
+ else if (sdebug_uuid_ctl == 2) {
+ if (got_shared_uuid)
+ devip->lu_name = shared_uuid;
+ else {
+ uuid_be_gen(&shared_uuid);
+ got_shared_uuid = true;
+ devip->lu_name = shared_uuid;
+ }
+ }
devip->sdbg_host = sdbg_host;
list_add_tail(&devip->dev_list, &sdbg_host->dev_info_list);
}
@@ -4089,6 +4120,7 @@ module_param_named(unmap_granularity, sdebug_unmap_granularity, int, S_IRUGO);
module_param_named(unmap_max_blocks, sdebug_unmap_max_blocks, int, S_IRUGO);
module_param_named(unmap_max_desc, sdebug_unmap_max_desc, int, S_IRUGO);
module_param_named(virtual_gb, sdebug_virtual_gb, int, S_IRUGO | S_IWUSR);
+module_param_named(uuid_ctl, sdebug_uuid_ctl, int, S_IRUGO);
module_param_named(vpd_use_hostno, sdebug_vpd_use_hostno, int,
S_IRUGO | S_IWUSR);
module_param_named(write_same_length, sdebug_write_same_length, int,
@@ -4138,6 +4170,8 @@ MODULE_PARM_DESC(unmap_alignment, "lowest aligned thin provisioning lba (def=0)"
MODULE_PARM_DESC(unmap_granularity, "thin provisioning granularity in blocks (def=1)");
MODULE_PARM_DESC(unmap_max_blocks, "max # of blocks can be unmapped in one cmd (def=0xffffffff)");
MODULE_PARM_DESC(unmap_max_desc, "max # of ranges that can be unmapped in one cmd (def=256)");
+MODULE_PARM_DESC(uuid_ctl,
+ "1->use uuid for lu name, 0->don't, 2->all use same (def=0)");
MODULE_PARM_DESC(virtual_gb, "virtual gigabyte (GiB) size (def=0 -> use dev_size_mb)");
MODULE_PARM_DESC(vpd_use_hostno, "0 -> dev ids ignore hostno (def=1 -> unique dev ids)");
MODULE_PARM_DESC(write_same_length, "Maximum blocks per WRITE SAME cmd (def=0xffff)");
@@ -4770,6 +4804,12 @@ static ssize_t strict_store(struct device_driver *ddp, const char *buf,
}
static DRIVER_ATTR_RW(strict);
+static ssize_t uuid_ctl_show(struct device_driver *ddp, char *buf)
+{
+ return scnprintf(buf, PAGE_SIZE, "%d\n", !!sdebug_uuid_ctl);
+}
+static DRIVER_ATTR_RO(uuid_ctl);
+
/* Note: The following array creates attribute files in the
/sys/bus/pseudo/drivers/scsi_debug directory. The advantage of these
@@ -4808,6 +4848,7 @@ static struct attribute *sdebug_drv_attrs[] = {
&driver_attr_host_lock.attr,
&driver_attr_ndelay.attr,
&driver_attr_strict.attr,
+ &driver_attr_uuid_ctl.attr,
NULL,
};
ATTRIBUTE_GROUPS(sdebug_drv);
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2.5 6/6] scsi_debug: use locally assigned naa
2016-05-01 2:44 [PATCH v2.5 0/6] scsi_debug: rebase second half of series Douglas Gilbert
` (4 preceding siblings ...)
2016-05-01 2:44 ` [PATCH v2.5 5/6] scsi_debug: uuid for lu name Douglas Gilbert
@ 2016-05-01 2:44 ` Douglas Gilbert
2016-05-02 8:39 ` Hannes Reinecke
2016-05-04 22:37 ` Bart Van Assche
5 siblings, 2 replies; 24+ messages in thread
From: Douglas Gilbert @ 2016-05-01 2:44 UTC (permalink / raw)
To: linux-scsi; +Cc: martin.petersen, tomas.winkler, emilne, bart.vanassche
For reported SAS addresses replace fake IEEE registered NAAs (5)
with locally assigned NAAs (3).
Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
---
drivers/scsi/scsi_debug.c | 35 ++++++++++++++++++-----------------
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index c14811e..459a4d7 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -921,9 +921,10 @@ static int fetch_to_dev_buffer(struct scsi_cmnd *scp, unsigned char *arr,
static const char * inq_vendor_id = "Linux ";
static const char * inq_product_id = "scsi_debug ";
static const char *inq_product_rev = "0186"; /* version less '.' */
-static const u64 naa5_comp_a = 0x5222222000000000ULL;
-static const u64 naa5_comp_b = 0x5333333000000000ULL;
-static const u64 naa5_comp_c = 0x5111111000000000ULL;
+/* Use some locally assigned NAAs for SAS addresses. */
+static const u64 naa3_comp_a = 0x3222222000000000ULL;
+static const u64 naa3_comp_b = 0x3333333000000000ULL;
+static const u64 naa3_comp_c = 0x3111111000000000ULL;
/* Device identification VPD page. Returns number of bytes placed in arr */
static int inquiry_vpd_83(unsigned char *arr, int port_group_id,
@@ -957,12 +958,12 @@ static int inquiry_vpd_83(unsigned char *arr, int port_group_id,
memcpy(arr + num, lu_name, 16);
num += 16;
} else {
- /* NAA-5, Logical unit identifier (binary) */
+ /* NAA-3, Logical unit identifier (binary) */
arr[num++] = 0x1; /* binary (not necessarily sas) */
arr[num++] = 0x3; /* PIV=0, lu, naa */
arr[num++] = 0x0;
arr[num++] = 0x8;
- put_unaligned_be64(naa5_comp_b + dev_id_num, arr + num);
+ put_unaligned_be64(naa3_comp_b + dev_id_num, arr + num);
num += 8;
}
/* Target relative port number */
@@ -975,14 +976,14 @@ static int inquiry_vpd_83(unsigned char *arr, int port_group_id,
arr[num++] = 0x0;
arr[num++] = 0x1; /* relative port A */
}
- /* NAA-5, Target port identifier */
+ /* NAA-3, Target port identifier */
arr[num++] = 0x61; /* proto=sas, binary */
arr[num++] = 0x93; /* piv=1, target port, naa */
arr[num++] = 0x0;
arr[num++] = 0x8;
- put_unaligned_be64(naa5_comp_a + port_a, arr + num);
+ put_unaligned_be64(naa3_comp_a + port_a, arr + num);
num += 8;
- /* NAA-5, Target port group identifier */
+ /* NAA-3, Target port group identifier */
arr[num++] = 0x61; /* proto=sas, binary */
arr[num++] = 0x95; /* piv=1, target port group id */
arr[num++] = 0x0;
@@ -991,19 +992,19 @@ static int inquiry_vpd_83(unsigned char *arr, int port_group_id,
arr[num++] = 0;
put_unaligned_be16(port_group_id, arr + num);
num += 2;
- /* NAA-5, Target device identifier */
+ /* NAA-3, Target device identifier */
arr[num++] = 0x61; /* proto=sas, binary */
arr[num++] = 0xa3; /* piv=1, target device, naa */
arr[num++] = 0x0;
arr[num++] = 0x8;
- put_unaligned_be64(naa5_comp_a + target_dev_id, arr + num);
+ put_unaligned_be64(naa3_comp_a + target_dev_id, arr + num);
num += 8;
/* SCSI name string: Target device identifier */
arr[num++] = 0x63; /* proto=sas, UTF-8 */
arr[num++] = 0xa8; /* piv=1, target device, SCSI name string */
arr[num++] = 0x0;
arr[num++] = 24;
- memcpy(arr + num, "naa.52222220", 12);
+ memcpy(arr + num, "naa.32222220", 12);
num += 12;
snprintf(b, sizeof(b), "%08X", target_dev_id);
memcpy(arr + num, b, 8);
@@ -1082,7 +1083,7 @@ static int inquiry_vpd_88(unsigned char *arr, int target_dev_id)
arr[num++] = 0x93; /* PIV=1, target port, NAA */
arr[num++] = 0x0; /* reserved */
arr[num++] = 0x8; /* length */
- put_unaligned_be64(naa5_comp_a + port_a, arr + num);
+ put_unaligned_be64(naa3_comp_a + port_a, arr + num);
num += 8;
arr[num++] = 0x0; /* reserved */
arr[num++] = 0x0; /* reserved */
@@ -1097,7 +1098,7 @@ static int inquiry_vpd_88(unsigned char *arr, int target_dev_id)
arr[num++] = 0x93; /* PIV=1, target port, NAA */
arr[num++] = 0x0; /* reserved */
arr[num++] = 0x8; /* length */
- put_unaligned_be64(naa5_comp_a + port_b, arr + num);
+ put_unaligned_be64(naa3_comp_a + port_b, arr + num);
num += 8;
return num;
@@ -1927,10 +1928,10 @@ static int resp_sas_pcd_m_spg(unsigned char * p, int pcontrol, int target,
};
int port_a, port_b;
- put_unaligned_be64(naa5_comp_a, sas_pcd_m_pg + 16);
- put_unaligned_be64(naa5_comp_c + 1, sas_pcd_m_pg + 24);
- put_unaligned_be64(naa5_comp_a, sas_pcd_m_pg + 64);
- put_unaligned_be64(naa5_comp_c + 1, sas_pcd_m_pg + 72);
+ put_unaligned_be64(naa3_comp_a, sas_pcd_m_pg + 16);
+ put_unaligned_be64(naa3_comp_c + 1, sas_pcd_m_pg + 24);
+ put_unaligned_be64(naa3_comp_a, sas_pcd_m_pg + 64);
+ put_unaligned_be64(naa3_comp_c + 1, sas_pcd_m_pg + 72);
port_a = target_dev_id + 1;
port_b = port_a + 1;
memcpy(p, sas_pcd_m_pg, sizeof(sas_pcd_m_pg));
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2.5 2/6] scsi_debug: rework resp_report_luns
2016-05-01 2:44 ` [PATCH v2.5 2/6] scsi_debug: rework resp_report_luns Douglas Gilbert
@ 2016-05-02 8:25 ` Hannes Reinecke
2016-05-02 8:29 ` Winkler, Tomas
2016-05-03 23:58 ` Bart Van Assche
1 sibling, 1 reply; 24+ messages in thread
From: Hannes Reinecke @ 2016-05-02 8:25 UTC (permalink / raw)
To: Douglas Gilbert, linux-scsi
Cc: martin.petersen, tomas.winkler, emilne, bart.vanassche
On 05/01/2016 04:44 AM, Douglas Gilbert wrote:
> Based on "[PATH V2] scsi_debug: rework resp_report_luns" patch
> sent by Tomas Winkler on Thursday, 26 Feb 2015. His notes:
> 1. Remove duplicated boundary checks which simplify the fill-in
> loop
> 2. Use more of scsi generic API
> Replace fixed length response array a with heap allocation
> allowing up to 256 normal LUNs per target.
>
> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
> ---
> drivers/scsi/scsi_debug.c | 135 +++++++++++++++++++++++++++++-----------------
> 1 file changed, 87 insertions(+), 48 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2.5 1/6] scsi_debug: use pdt constants
2016-05-01 2:44 ` [PATCH v2.5 1/6] scsi_debug: use pdt constants Douglas Gilbert
@ 2016-05-02 8:25 ` Hannes Reinecke
2016-05-03 23:56 ` Bart Van Assche
1 sibling, 0 replies; 24+ messages in thread
From: Hannes Reinecke @ 2016-05-02 8:25 UTC (permalink / raw)
To: Douglas Gilbert, linux-scsi
Cc: martin.petersen, tomas.winkler, emilne, bart.vanassche
On 05/01/2016 04:44 AM, Douglas Gilbert wrote:
> Use TYPE_* constants for SCSI peripheral device types instead
> of numbers. Further cleanups requested by checkpatch.pl .
>
> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
> ---
> drivers/scsi/scsi_debug.c | 86 +++++++++++++++++++++++++----------------------
> 1 file changed, 45 insertions(+), 41 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 24+ messages in thread
* RE: [PATCH v2.5 2/6] scsi_debug: rework resp_report_luns
2016-05-02 8:25 ` Hannes Reinecke
@ 2016-05-02 8:29 ` Winkler, Tomas
0 siblings, 0 replies; 24+ messages in thread
From: Winkler, Tomas @ 2016-05-02 8:29 UTC (permalink / raw)
To: Hannes Reinecke, Douglas Gilbert, linux-scsi
Cc: martin.petersen, emilne, bart.vanassche
> -----Original Message-----
> From: Hannes Reinecke [mailto:hare@suse.de]
> Sent: Monday, May 02, 2016 11:25
> To: Douglas Gilbert <dgilbert@interlog.com>; linux-scsi@vger.kernel.org
> Cc: martin.petersen@oracle.com; Winkler, Tomas
> <tomas.winkler@intel.com>; emilne@redhat.com;
> bart.vanassche@sandisk.com
> Subject: Re: [PATCH v2.5 2/6] scsi_debug: rework resp_report_luns
>
> On 05/01/2016 04:44 AM, Douglas Gilbert wrote:
> > Based on "[PATH V2] scsi_debug: rework resp_report_luns" patch sent by
> > Tomas Winkler on Thursday, 26 Feb 2015. His notes:
> > 1. Remove duplicated boundary checks which simplify the fill-in
> > loop
> > 2. Use more of scsi generic API
> > Replace fixed length response array a with heap allocation allowing up
> > to 256 normal LUNs per target.
> >
> > Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
> > ---
> > drivers/scsi/scsi_debug.c | 135
> > +++++++++++++++++++++++++++++-----------------
> > 1 file changed, 87 insertions(+), 48 deletions(-)
> >
> Reviewed-by: Hannes Reinecke <hare@suse.com>
Ack again
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2.5 3/6] scsi_debug: add multiple queue support
2016-05-01 2:44 ` [PATCH v2.5 3/6] scsi_debug: add multiple queue support Douglas Gilbert
@ 2016-05-02 8:35 ` Hannes Reinecke
2016-05-02 15:35 ` Douglas Gilbert
2016-05-04 22:09 ` Bart Van Assche
2016-05-04 22:32 ` Bart Van Assche
1 sibling, 2 replies; 24+ messages in thread
From: Hannes Reinecke @ 2016-05-02 8:35 UTC (permalink / raw)
To: Douglas Gilbert, linux-scsi
Cc: martin.petersen, tomas.winkler, emilne, bart.vanassche
On 05/01/2016 04:44 AM, Douglas Gilbert wrote:
> Add submit_queue parameter (minimum and default: 1; maximum:
> nr_cpu_ids) that controls how many queues are built, each with
> their own lock and in_use bit vector. Add statistics parameter
> which is default on.
>
> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
> ---
> drivers/scsi/scsi_debug.c | 680 +++++++++++++++++++++++++++++-----------------
> 1 file changed, 426 insertions(+), 254 deletions(-)
>
Two general questions for this:
- Why do you get rid of the embedded command payload?
Where's the benefit of allocating the commands yourself?
- Wouldn't it be better to move to a per-cpu structure per queue?
Each queue will be tacked to a CPU anyway, so you could be using
per-cpu structures. Otherwise you'll run into synchronization
issues, and any performance gain you might get from scsi-mq is
lost as you to synchronize on the lower level.
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2.5 4/6] scsi_debug: vpd and mode page work
2016-05-01 2:44 ` [PATCH v2.5 4/6] scsi_debug: vpd and mode page work Douglas Gilbert
@ 2016-05-02 8:38 ` Hannes Reinecke
2016-05-02 15:48 ` Douglas Gilbert
0 siblings, 1 reply; 24+ messages in thread
From: Hannes Reinecke @ 2016-05-02 8:38 UTC (permalink / raw)
To: Douglas Gilbert, linux-scsi
Cc: martin.petersen, tomas.winkler, emilne, bart.vanassche
On 05/01/2016 04:44 AM, Douglas Gilbert wrote:
> Cleanup some mode and vpd pages. Stop reporting SBC (disk) pages
> when peripheral type is something else (e.g. tape). Update
> version descriptors. Expand LBPRZ flag handling.
>
> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
> ---
> drivers/scsi/scsi_debug.c | 187 ++++++++++++++++++++++++++--------------------
> 1 file changed, 108 insertions(+), 79 deletions(-)
>
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 0932111..814067d 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -125,7 +125,7 @@ static const char *sdebug_version_date = "20160430";
> #define DEF_PHYSBLK_EXP 0
> #define DEF_PTYPE TYPE_DISK
> #define DEF_REMOVABLE false
> -#define DEF_SCSI_LEVEL 6 /* INQUIRY, byte2 [6->SPC-4] */
> +#define DEF_SCSI_LEVEL 7 /* INQUIRY, byte2 [6->SPC-4; 7->SPC-5] */
> #define DEF_SECTOR_SIZE 512
> #define DEF_UNMAP_ALIGNMENT 0
> #define DEF_UNMAP_GRANULARITY 1
> @@ -657,7 +657,11 @@ static const int device_qfull_result =
> (DID_OK << 16) | (COMMAND_COMPLETE << 8) | SAM_STAT_TASK_SET_FULL;
>
>
> -static inline unsigned int scsi_debug_lbp(void)
> +/* Only do the extra work involved in logical block provisioning if one or
> + * more of the lbpu, lbpws or lbpws10 parameters are given and we are doing
> + * real reads and writes (i.e. not skipping them for speed).
> + */
> +static inline bool scsi_debug_lbp(void)
> {
> return 0 == sdebug_fake_rw &&
> (sdebug_lbpu || sdebug_lbpws || sdebug_lbpws10);
> @@ -918,10 +922,10 @@ static const u64 naa5_comp_b = 0x5333333000000000ULL;
> static const u64 naa5_comp_c = 0x5111111000000000ULL;
>
> /* Device identification VPD page. Returns number of bytes placed in arr */
> -static int inquiry_evpd_83(unsigned char * arr, int port_group_id,
> - int target_dev_id, int dev_id_num,
> - const char * dev_id_str,
> - int dev_id_str_len)
> +static int inquiry_vpd_83(unsigned char *arr, int port_group_id,
> + int target_dev_id, int dev_id_num,
> + const char *dev_id_str,
> + int dev_id_str_len)
> {
> int num, port_a;
> char b[32];
> @@ -1000,14 +1004,14 @@ static unsigned char vpd84_data[] = {
> };
>
> /* Software interface identification VPD page */
> -static int inquiry_evpd_84(unsigned char * arr)
> +static int inquiry_vpd_84(unsigned char *arr)
> {
> memcpy(arr, vpd84_data, sizeof(vpd84_data));
> return sizeof(vpd84_data);
> }
>
> /* Management network addresses VPD page */
> -static int inquiry_evpd_85(unsigned char * arr)
> +static int inquiry_vpd_85(unsigned char *arr)
> {
> int num = 0;
> const char * na1 = "https://www.kernel.org/config";
> @@ -1042,7 +1046,7 @@ static int inquiry_evpd_85(unsigned char * arr)
> }
>
> /* SCSI ports VPD page */
> -static int inquiry_evpd_88(unsigned char * arr, int target_dev_id)
> +static int inquiry_vpd_88(unsigned char *arr, int target_dev_id)
> {
> int num = 0;
> int port_a, port_b;
> @@ -1129,7 +1133,7 @@ static unsigned char vpd89_data[] = {
> };
>
> /* ATA Information VPD page */
> -static int inquiry_evpd_89(unsigned char * arr)
> +static int inquiry_vpd_89(unsigned char *arr)
> {
> memcpy(arr, vpd89_data, sizeof(vpd89_data));
> return sizeof(vpd89_data);
> @@ -1144,7 +1148,7 @@ static unsigned char vpdb0_data[] = {
> };
>
> /* Block limits VPD page (SBC-3) */
> -static int inquiry_evpd_b0(unsigned char * arr)
> +static int inquiry_vpd_b0(unsigned char *arr)
> {
> unsigned int gran;
>
> @@ -1187,7 +1191,7 @@ static int inquiry_evpd_b0(unsigned char * arr)
> }
>
> /* Block device characteristics VPD page (SBC-3) */
> -static int inquiry_evpd_b1(unsigned char *arr)
> +static int inquiry_vpd_b1(unsigned char *arr)
> {
> memset(arr, 0, 0x3c);
> arr[0] = 0;
> @@ -1198,24 +1202,22 @@ static int inquiry_evpd_b1(unsigned char *arr)
> return 0x3c;
> }
>
> -/* Logical block provisioning VPD page (SBC-3) */
> -static int inquiry_evpd_b2(unsigned char *arr)
> +/* Logical block provisioning VPD page (SBC-4) */
> +static int inquiry_vpd_b2(unsigned char *arr)
> {
> memset(arr, 0, 0x4);
> arr[0] = 0; /* threshold exponent */
> -
> if (sdebug_lbpu)
> arr[1] = 1 << 7;
> -
> if (sdebug_lbpws)
> arr[1] |= 1 << 6;
> -
> if (sdebug_lbpws10)
> arr[1] |= 1 << 5;
> -
> - if (sdebug_lbprz)
> - arr[1] |= 1 << 2;
> -
> + if (sdebug_lbprz && scsi_debug_lbp())
> + arr[1] |= (sdebug_lbprz & 0x7) << 2; /* sbc4r07 and later */
> + /* anc_sup=0; dp=0 (no provisioning group descriptor) */
> + /* minimum_percentage=0; provisioning_type=0 (unknown) */
> + /* threshold_percentage=0 */
> return 0x4;
> }
>
> @@ -1228,12 +1230,13 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
> unsigned char * arr;
> unsigned char *cmd = scp->cmnd;
> int alloc_len, n, ret;
> - bool have_wlun;
> + bool have_wlun, is_disk;
>
> alloc_len = get_unaligned_be16(cmd + 3);
> arr = kzalloc(SDEBUG_MAX_INQ_ARR_SZ, GFP_ATOMIC);
> if (! arr)
> return DID_REQUEUE << 16;
> + is_disk = (sdebug_ptype == TYPE_DISK);
> have_wlun = scsi_is_wlun(scp->device->lun);
> if (have_wlun)
> pq_pdt = TYPE_WLUN; /* present, wlun */
> @@ -1271,11 +1274,12 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
> arr[n++] = 0x86; /* extended inquiry */
> arr[n++] = 0x87; /* mode page policy */
> arr[n++] = 0x88; /* SCSI ports */
> - arr[n++] = 0x89; /* ATA information */
> - arr[n++] = 0xb0; /* Block limits (SBC) */
> - arr[n++] = 0xb1; /* Block characteristics (SBC) */
> - if (scsi_debug_lbp()) /* Logical Block Prov. (SBC) */
> - arr[n++] = 0xb2;
> + if (is_disk) { /* SBC only */
> + arr[n++] = 0x89; /* ATA information */
> + arr[n++] = 0xb0; /* Block limits */
> + arr[n++] = 0xb1; /* Block characteristics */
> + arr[n++] = 0xb2; /* Logical Block Prov */
> + }
> arr[3] = n - 4; /* number of supported VPD pages */
> } else if (0x80 == cmd[2]) { /* unit serial number */
> arr[1] = cmd[2]; /*sanity */
> @@ -1283,21 +1287,21 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
> memcpy(&arr[4], lu_id_str, len);
> } else if (0x83 == cmd[2]) { /* device identification */
> arr[1] = cmd[2]; /*sanity */
> - arr[3] = inquiry_evpd_83(&arr[4], port_group_id,
> - target_dev_id, lu_id_num,
> - lu_id_str, len);
> + arr[3] = inquiry_vpd_83(&arr[4], port_group_id,
> + target_dev_id, lu_id_num,
> + lu_id_str, len);
> } else if (0x84 == cmd[2]) { /* Software interface ident. */
> arr[1] = cmd[2]; /*sanity */
> - arr[3] = inquiry_evpd_84(&arr[4]);
> + arr[3] = inquiry_vpd_84(&arr[4]);
> } else if (0x85 == cmd[2]) { /* Management network addresses */
> arr[1] = cmd[2]; /*sanity */
> - arr[3] = inquiry_evpd_85(&arr[4]);
> + arr[3] = inquiry_vpd_85(&arr[4]);
> } else if (0x86 == cmd[2]) { /* extended inquiry */
> arr[1] = cmd[2]; /*sanity */
> arr[3] = 0x3c; /* number of following entries */
> if (sdebug_dif == SD_DIF_TYPE3_PROTECTION)
> arr[4] = 0x4; /* SPT: GRD_CHK:1 */
> - else if (sdebug_dif)
> + else if (have_dif_prot)
> arr[4] = 0x5; /* SPT: GRD_CHK:1, REF_CHK:1 */
> else
> arr[4] = 0x0; /* no protection stuff */
> @@ -1311,20 +1315,20 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
> arr[10] = 0x82; /* mlus, per initiator port */
> } else if (0x88 == cmd[2]) { /* SCSI Ports */
> arr[1] = cmd[2]; /*sanity */
> - arr[3] = inquiry_evpd_88(&arr[4], target_dev_id);
> - } else if (0x89 == cmd[2]) { /* ATA information */
> + arr[3] = inquiry_vpd_88(&arr[4], target_dev_id);
> + } else if (is_disk && 0x89 == cmd[2]) { /* ATA information */
> arr[1] = cmd[2]; /*sanity */
> - n = inquiry_evpd_89(&arr[4]);
> + n = inquiry_vpd_89(&arr[4]);
> put_unaligned_be16(n, arr + 2);
> - } else if (0xb0 == cmd[2]) { /* Block limits (SBC) */
> + } else if (is_disk && 0xb0 == cmd[2]) { /* Block limits */
> arr[1] = cmd[2]; /*sanity */
> - arr[3] = inquiry_evpd_b0(&arr[4]);
> - } else if (0xb1 == cmd[2]) { /* Block characteristics (SBC) */
> + arr[3] = inquiry_vpd_b0(&arr[4]);
> + } else if (is_disk && 0xb1 == cmd[2]) { /* Block char. */
> arr[1] = cmd[2]; /*sanity */
> - arr[3] = inquiry_evpd_b1(&arr[4]);
> - } else if (0xb2 == cmd[2]) { /* Logical Block Prov. (SBC) */
> + arr[3] = inquiry_vpd_b1(&arr[4]);
> + } else if (is_disk && 0xb2 == cmd[2]) { /* LB Prov. */
> arr[1] = cmd[2]; /*sanity */
> - arr[3] = inquiry_evpd_b2(&arr[4]);
> + arr[3] = inquiry_vpd_b2(&arr[4]);
> } else {
> mk_sense_invalid_fld(scp, SDEB_IN_CDB, 2, -1);
> kfree(arr);
Probably beside the point, but why do you provide the ATA
information VPD? Upon seeing this any tool _might_ be induced to
think of it as an SATL device, and start sending interesting
ATA_12 or ATA_16 commands ...
Otherwise the patch looks okay:
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2.5 5/6] scsi_debug: uuid for lu name
2016-05-01 2:44 ` [PATCH v2.5 5/6] scsi_debug: uuid for lu name Douglas Gilbert
@ 2016-05-02 8:38 ` Hannes Reinecke
2016-05-04 22:37 ` Bart Van Assche
1 sibling, 0 replies; 24+ messages in thread
From: Hannes Reinecke @ 2016-05-02 8:38 UTC (permalink / raw)
To: Douglas Gilbert, linux-scsi
Cc: martin.petersen, tomas.winkler, emilne, bart.vanassche
On 05/01/2016 04:44 AM, Douglas Gilbert wrote:
> Permit changing of a LU name from a (fake) IEEE registered NAA (5)
> to a locally assigned UUID. Using a UUID (RFC 4122) for a SCSI
> designation descriptor (e.g. a LU name) was added in spc5r08.pdf
> (a draft INCITS standard) on 25 January 2016. Add parameter
> uuid_ctl to use a separate UUID for each LU (storage device) name.
> Additional option for all LU names to have the same UUID (since
> their storage is shared). Previous action of using NAA identifier
> for LU name remains the default.
>
> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
> ---
> drivers/scsi/scsi_debug.c | 61 +++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 51 insertions(+), 10 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2.5 6/6] scsi_debug: use locally assigned naa
2016-05-01 2:44 ` [PATCH v2.5 6/6] scsi_debug: use locally assigned naa Douglas Gilbert
@ 2016-05-02 8:39 ` Hannes Reinecke
2016-05-04 22:37 ` Bart Van Assche
1 sibling, 0 replies; 24+ messages in thread
From: Hannes Reinecke @ 2016-05-02 8:39 UTC (permalink / raw)
To: Douglas Gilbert, linux-scsi
Cc: martin.petersen, tomas.winkler, emilne, bart.vanassche
On 05/01/2016 04:44 AM, Douglas Gilbert wrote:
> For reported SAS addresses replace fake IEEE registered NAAs (5)
> with locally assigned NAAs (3).
>
> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
> ---
> drivers/scsi/scsi_debug.c | 35 ++++++++++++++++++-----------------
> 1 file changed, 18 insertions(+), 17 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2.5 3/6] scsi_debug: add multiple queue support
2016-05-02 8:35 ` Hannes Reinecke
@ 2016-05-02 15:35 ` Douglas Gilbert
2016-05-04 22:09 ` Bart Van Assche
1 sibling, 0 replies; 24+ messages in thread
From: Douglas Gilbert @ 2016-05-02 15:35 UTC (permalink / raw)
To: Hannes Reinecke, linux-scsi
Cc: martin.petersen, tomas.winkler, emilne, bart.vanassche
On 2016-05-02 04:35 AM, Hannes Reinecke wrote:
> On 05/01/2016 04:44 AM, Douglas Gilbert wrote:
>> Add submit_queue parameter (minimum and default: 1; maximum:
>> nr_cpu_ids) that controls how many queues are built, each with
>> their own lock and in_use bit vector. Add statistics parameter
>> which is default on.
>>
>> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
>> ---
>> drivers/scsi/scsi_debug.c | 680 +++++++++++++++++++++++++++++-----------------
>> 1 file changed, 426 insertions(+), 254 deletions(-)
>>
> Two general questions for this:
>
> - Why do you get rid of the embedded command payload?
I'm not sure what payload you a referring to. And this patch
only adds multiple queues, I can't see that it removes anything.
> Where's the benefit of allocating the commands yourself?
The commands are either replied to "in thread" (e.g. when delay=0
or an error is detected), or queued on a hr timer or work item.
A pointer to the command is held in the queue (the same as before
this patch). The only allocations associated with commands are to
build data-in buffers for responses. (e.g. an INQUIRY command for
a VPD page).
> - Wouldn't it be better to move to a per-cpu structure per queue?
> Each queue will be tacked to a CPU anyway, so you could be using
> per-cpu structures. Otherwise you'll run into synchronization
> issues, and any performance gain you might get from scsi-mq is
> lost as you to synchronize on the lower level.
I offer this patch as being necessary, but probably not sufficient
for implementing full scsi "mq". The interface itself for
scsi/block mq does not seem to be documented and is possibly in a
state of flux.
From testing this patch (e.g. by observing
"cat /proc/scsi/scsi_debug/<host_num>" while fio is running, the
CPU affinity is very good without any per-cpu magic ***. The
"misqueues" count (that is (cpu) miscues on queues) records the
number of times a timer or a workqueue gets its callback on a
different cpu, is extremely low, typically zero. I could get
non-zero numbers if I ran something else (e.g. a kernel build)
while fio was running, still the misqueues were well under 1% of
commands queued.
That said, I see very little performance improvement with
submit_queues=4 (the number of processors on my two test machines)
compared to submit_queues=1 which is effectively what the driver was
doing before this patch. So I'm open to suggestions, especially in
the form of code :-)
Also if we went for per-cpu structures should we worry about the
complex issue of a cpu being hot unplugged (or plugged back in)
while its queue was holding unfinished commands?
Doug Gilbert
*** you are probably correct that without the per-cpu "magic" lock
contention is likely the cause of so little performance
improvement in the multiple submit queues case.
Also while testing with fio, submit_queues=<num_of_cpus>,
'top -H' shows each fio thread at around 100%.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2.5 4/6] scsi_debug: vpd and mode page work
2016-05-02 8:38 ` Hannes Reinecke
@ 2016-05-02 15:48 ` Douglas Gilbert
0 siblings, 0 replies; 24+ messages in thread
From: Douglas Gilbert @ 2016-05-02 15:48 UTC (permalink / raw)
To: Hannes Reinecke, linux-scsi
Cc: martin.petersen, tomas.winkler, emilne, bart.vanassche
On 2016-05-02 04:38 AM, Hannes Reinecke wrote:
> On 05/01/2016 04:44 AM, Douglas Gilbert wrote:
>> Cleanup some mode and vpd pages. Stop reporting SBC (disk) pages
>> when peripheral type is something else (e.g. tape). Update
>> version descriptors. Expand LBPRZ flag handling.
>>
>> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
>> ---
>> drivers/scsi/scsi_debug.c | 187 ++++++++++++++++++++++++++--------------------
>> 1 file changed, 108 insertions(+), 79 deletions(-)
>>
>> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
>> index 0932111..814067d 100644
>> --- a/drivers/scsi/scsi_debug.c
>> +++ b/drivers/scsi/scsi_debug.c
>> @@ -125,7 +125,7 @@ static const char *sdebug_version_date = "20160430";
>> #define DEF_PHYSBLK_EXP 0
>> #define DEF_PTYPE TYPE_DISK
>> #define DEF_REMOVABLE false
>> -#define DEF_SCSI_LEVEL 6 /* INQUIRY, byte2 [6->SPC-4] */
>> +#define DEF_SCSI_LEVEL 7 /* INQUIRY, byte2 [6->SPC-4; 7->SPC-5] */
>> #define DEF_SECTOR_SIZE 512
>> #define DEF_UNMAP_ALIGNMENT 0
>> #define DEF_UNMAP_GRANULARITY 1
>> @@ -657,7 +657,11 @@ static const int device_qfull_result =
>> (DID_OK << 16) | (COMMAND_COMPLETE << 8) | SAM_STAT_TASK_SET_FULL;
>>
>>
>> -static inline unsigned int scsi_debug_lbp(void)
>> +/* Only do the extra work involved in logical block provisioning if one or
>> + * more of the lbpu, lbpws or lbpws10 parameters are given and we are doing
>> + * real reads and writes (i.e. not skipping them for speed).
>> + */
>> +static inline bool scsi_debug_lbp(void)
>> {
>> return 0 == sdebug_fake_rw &&
>> (sdebug_lbpu || sdebug_lbpws || sdebug_lbpws10);
>> @@ -918,10 +922,10 @@ static const u64 naa5_comp_b = 0x5333333000000000ULL;
>> static const u64 naa5_comp_c = 0x5111111000000000ULL;
>>
>> /* Device identification VPD page. Returns number of bytes placed in arr */
>> -static int inquiry_evpd_83(unsigned char * arr, int port_group_id,
>> - int target_dev_id, int dev_id_num,
>> - const char * dev_id_str,
>> - int dev_id_str_len)
>> +static int inquiry_vpd_83(unsigned char *arr, int port_group_id,
>> + int target_dev_id, int dev_id_num,
>> + const char *dev_id_str,
>> + int dev_id_str_len)
>> {
>> int num, port_a;
>> char b[32];
>> @@ -1000,14 +1004,14 @@ static unsigned char vpd84_data[] = {
>> };
>>
>> /* Software interface identification VPD page */
>> -static int inquiry_evpd_84(unsigned char * arr)
>> +static int inquiry_vpd_84(unsigned char *arr)
>> {
>> memcpy(arr, vpd84_data, sizeof(vpd84_data));
>> return sizeof(vpd84_data);
>> }
>>
>> /* Management network addresses VPD page */
>> -static int inquiry_evpd_85(unsigned char * arr)
>> +static int inquiry_vpd_85(unsigned char *arr)
>> {
>> int num = 0;
>> const char * na1 = "https://www.kernel.org/config";
>> @@ -1042,7 +1046,7 @@ static int inquiry_evpd_85(unsigned char * arr)
>> }
>>
>> /* SCSI ports VPD page */
>> -static int inquiry_evpd_88(unsigned char * arr, int target_dev_id)
>> +static int inquiry_vpd_88(unsigned char *arr, int target_dev_id)
>> {
>> int num = 0;
>> int port_a, port_b;
>> @@ -1129,7 +1133,7 @@ static unsigned char vpd89_data[] = {
>> };
>>
>> /* ATA Information VPD page */
>> -static int inquiry_evpd_89(unsigned char * arr)
>> +static int inquiry_vpd_89(unsigned char *arr)
>> {
>> memcpy(arr, vpd89_data, sizeof(vpd89_data));
>> return sizeof(vpd89_data);
>> @@ -1144,7 +1148,7 @@ static unsigned char vpdb0_data[] = {
>> };
>>
>> /* Block limits VPD page (SBC-3) */
>> -static int inquiry_evpd_b0(unsigned char * arr)
>> +static int inquiry_vpd_b0(unsigned char *arr)
>> {
>> unsigned int gran;
>>
>> @@ -1187,7 +1191,7 @@ static int inquiry_evpd_b0(unsigned char * arr)
>> }
>>
>> /* Block device characteristics VPD page (SBC-3) */
>> -static int inquiry_evpd_b1(unsigned char *arr)
>> +static int inquiry_vpd_b1(unsigned char *arr)
>> {
>> memset(arr, 0, 0x3c);
>> arr[0] = 0;
>> @@ -1198,24 +1202,22 @@ static int inquiry_evpd_b1(unsigned char *arr)
>> return 0x3c;
>> }
>>
>> -/* Logical block provisioning VPD page (SBC-3) */
>> -static int inquiry_evpd_b2(unsigned char *arr)
>> +/* Logical block provisioning VPD page (SBC-4) */
>> +static int inquiry_vpd_b2(unsigned char *arr)
>> {
>> memset(arr, 0, 0x4);
>> arr[0] = 0; /* threshold exponent */
>> -
>> if (sdebug_lbpu)
>> arr[1] = 1 << 7;
>> -
>> if (sdebug_lbpws)
>> arr[1] |= 1 << 6;
>> -
>> if (sdebug_lbpws10)
>> arr[1] |= 1 << 5;
>> -
>> - if (sdebug_lbprz)
>> - arr[1] |= 1 << 2;
>> -
>> + if (sdebug_lbprz && scsi_debug_lbp())
>> + arr[1] |= (sdebug_lbprz & 0x7) << 2; /* sbc4r07 and later */
>> + /* anc_sup=0; dp=0 (no provisioning group descriptor) */
>> + /* minimum_percentage=0; provisioning_type=0 (unknown) */
>> + /* threshold_percentage=0 */
>> return 0x4;
>> }
>>
>> @@ -1228,12 +1230,13 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
>> unsigned char * arr;
>> unsigned char *cmd = scp->cmnd;
>> int alloc_len, n, ret;
>> - bool have_wlun;
>> + bool have_wlun, is_disk;
>>
>> alloc_len = get_unaligned_be16(cmd + 3);
>> arr = kzalloc(SDEBUG_MAX_INQ_ARR_SZ, GFP_ATOMIC);
>> if (! arr)
>> return DID_REQUEUE << 16;
>> + is_disk = (sdebug_ptype == TYPE_DISK);
>> have_wlun = scsi_is_wlun(scp->device->lun);
>> if (have_wlun)
>> pq_pdt = TYPE_WLUN; /* present, wlun */
>> @@ -1271,11 +1274,12 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
>> arr[n++] = 0x86; /* extended inquiry */
>> arr[n++] = 0x87; /* mode page policy */
>> arr[n++] = 0x88; /* SCSI ports */
>> - arr[n++] = 0x89; /* ATA information */
>> - arr[n++] = 0xb0; /* Block limits (SBC) */
>> - arr[n++] = 0xb1; /* Block characteristics (SBC) */
>> - if (scsi_debug_lbp()) /* Logical Block Prov. (SBC) */
>> - arr[n++] = 0xb2;
>> + if (is_disk) { /* SBC only */
>> + arr[n++] = 0x89; /* ATA information */
>> + arr[n++] = 0xb0; /* Block limits */
>> + arr[n++] = 0xb1; /* Block characteristics */
>> + arr[n++] = 0xb2; /* Logical Block Prov */
>> + }
>> arr[3] = n - 4; /* number of supported VPD pages */
>> } else if (0x80 == cmd[2]) { /* unit serial number */
>> arr[1] = cmd[2]; /*sanity */
>> @@ -1283,21 +1287,21 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
>> memcpy(&arr[4], lu_id_str, len);
>> } else if (0x83 == cmd[2]) { /* device identification */
>> arr[1] = cmd[2]; /*sanity */
>> - arr[3] = inquiry_evpd_83(&arr[4], port_group_id,
>> - target_dev_id, lu_id_num,
>> - lu_id_str, len);
>> + arr[3] = inquiry_vpd_83(&arr[4], port_group_id,
>> + target_dev_id, lu_id_num,
>> + lu_id_str, len);
>> } else if (0x84 == cmd[2]) { /* Software interface ident. */
>> arr[1] = cmd[2]; /*sanity */
>> - arr[3] = inquiry_evpd_84(&arr[4]);
>> + arr[3] = inquiry_vpd_84(&arr[4]);
>> } else if (0x85 == cmd[2]) { /* Management network addresses */
>> arr[1] = cmd[2]; /*sanity */
>> - arr[3] = inquiry_evpd_85(&arr[4]);
>> + arr[3] = inquiry_vpd_85(&arr[4]);
>> } else if (0x86 == cmd[2]) { /* extended inquiry */
>> arr[1] = cmd[2]; /*sanity */
>> arr[3] = 0x3c; /* number of following entries */
>> if (sdebug_dif == SD_DIF_TYPE3_PROTECTION)
>> arr[4] = 0x4; /* SPT: GRD_CHK:1 */
>> - else if (sdebug_dif)
>> + else if (have_dif_prot)
>> arr[4] = 0x5; /* SPT: GRD_CHK:1, REF_CHK:1 */
>> else
>> arr[4] = 0x0; /* no protection stuff */
>> @@ -1311,20 +1315,20 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
>> arr[10] = 0x82; /* mlus, per initiator port */
>> } else if (0x88 == cmd[2]) { /* SCSI Ports */
>> arr[1] = cmd[2]; /*sanity */
>> - arr[3] = inquiry_evpd_88(&arr[4], target_dev_id);
>> - } else if (0x89 == cmd[2]) { /* ATA information */
>> + arr[3] = inquiry_vpd_88(&arr[4], target_dev_id);
>> + } else if (is_disk && 0x89 == cmd[2]) { /* ATA information */
>> arr[1] = cmd[2]; /*sanity */
>> - n = inquiry_evpd_89(&arr[4]);
>> + n = inquiry_vpd_89(&arr[4]);
>> put_unaligned_be16(n, arr + 2);
>> - } else if (0xb0 == cmd[2]) { /* Block limits (SBC) */
>> + } else if (is_disk && 0xb0 == cmd[2]) { /* Block limits */
>> arr[1] = cmd[2]; /*sanity */
>> - arr[3] = inquiry_evpd_b0(&arr[4]);
>> - } else if (0xb1 == cmd[2]) { /* Block characteristics (SBC) */
>> + arr[3] = inquiry_vpd_b0(&arr[4]);
>> + } else if (is_disk && 0xb1 == cmd[2]) { /* Block char. */
>> arr[1] = cmd[2]; /*sanity */
>> - arr[3] = inquiry_evpd_b1(&arr[4]);
>> - } else if (0xb2 == cmd[2]) { /* Logical Block Prov. (SBC) */
>> + arr[3] = inquiry_vpd_b1(&arr[4]);
>> + } else if (is_disk && 0xb2 == cmd[2]) { /* LB Prov. */
>> arr[1] = cmd[2]; /*sanity */
>> - arr[3] = inquiry_evpd_b2(&arr[4]);
>> + arr[3] = inquiry_vpd_b2(&arr[4]);
>> } else {
>> mk_sense_invalid_fld(scp, SDEB_IN_CDB, 2, -1);
>> kfree(arr);
> Probably beside the point, but why do you provide the ATA
> information VPD? Upon seeing this any tool _might_ be induced to
> think of it as an SATL device, and start sending interesting
> ATA_12 or ATA_16 commands ...
Yes that is a bit curious (and I put that code there some time ago).
If an application client does respond by sending ATA_12, ATA_16
or ATA_32 (new from T10) then it will get an illegal request (invalid
command operation code) from this driver. And the application client
should react gracefully to that.
So that could be removed, but that should be done as a separate patch
just in case somebody's test harness is relying on the current
behavior. The change in this patch is to make sure those SAT and SBC
related VPD pages are not returned if the pdt is other than a disk.
[Which brings up the question of RBC and ZBC).
Doug Gilbert
> Otherwise the patch looks okay:
>
> Reviewed-by: Hannes Reinecke <hare@suse.com>
>
> Cheers,
>
> Hannes
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2.5 1/6] scsi_debug: use pdt constants
2016-05-01 2:44 ` [PATCH v2.5 1/6] scsi_debug: use pdt constants Douglas Gilbert
2016-05-02 8:25 ` Hannes Reinecke
@ 2016-05-03 23:56 ` Bart Van Assche
1 sibling, 0 replies; 24+ messages in thread
From: Bart Van Assche @ 2016-05-03 23:56 UTC (permalink / raw)
To: Douglas Gilbert, linux-scsi; +Cc: martin.petersen, tomas.winkler, emilne
On 04/30/2016 07:44 PM, Douglas Gilbert wrote:
> Use TYPE_* constants for SCSI peripheral device types instead
> of numbers. Further cleanups requested by checkpatch.pl .
>
> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2.5 2/6] scsi_debug: rework resp_report_luns
2016-05-01 2:44 ` [PATCH v2.5 2/6] scsi_debug: rework resp_report_luns Douglas Gilbert
2016-05-02 8:25 ` Hannes Reinecke
@ 2016-05-03 23:58 ` Bart Van Assche
1 sibling, 0 replies; 24+ messages in thread
From: Bart Van Assche @ 2016-05-03 23:58 UTC (permalink / raw)
To: Douglas Gilbert, linux-scsi; +Cc: martin.petersen, tomas.winkler, emilne
On 04/30/2016 07:44 PM, Douglas Gilbert wrote:
> Based on "[PATH V2] scsi_debug: rework resp_report_luns" patch
> sent by Tomas Winkler on Thursday, 26 Feb 2015. His notes:
> 1. Remove duplicated boundary checks which simplify the fill-in
> loop
> 2. Use more of scsi generic API
> Replace fixed length response array a with heap allocation
> allowing up to 256 normal LUNs per target.
>
> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2.5 3/6] scsi_debug: add multiple queue support
2016-05-02 8:35 ` Hannes Reinecke
2016-05-02 15:35 ` Douglas Gilbert
@ 2016-05-04 22:09 ` Bart Van Assche
1 sibling, 0 replies; 24+ messages in thread
From: Bart Van Assche @ 2016-05-04 22:09 UTC (permalink / raw)
To: Hannes Reinecke, Douglas Gilbert, linux-scsi
Cc: martin.petersen, tomas.winkler, emilne, bart.vanassche
On 05/02/2016 01:35 AM, Hannes Reinecke wrote:
> On 05/01/2016 04:44 AM, Douglas Gilbert wrote:
>> Add submit_queue parameter (minimum and default: 1; maximum:
>> nr_cpu_ids) that controls how many queues are built, each with
>> their own lock and in_use bit vector. Add statistics parameter
>> which is default on.
>>
>> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
>> ---
>> drivers/scsi/scsi_debug.c | 680 +++++++++++++++++++++++++++++-----------------
>> 1 file changed, 426 insertions(+), 254 deletions(-)
>>
> Two general questions for this:
>
> - Why do you get rid of the embedded command payload?
> Where's the benefit of allocating the commands yourself?
> - Wouldn't it be better to move to a per-cpu structure per queue?
> Each queue will be tacked to a CPU anyway, so you could be using
> per-cpu structures. Otherwise you'll run into synchronization
> issues, and any performance gain you might get from scsi-mq is
> lost as you to synchronize on the lower level.
With submit_queues == nr_cpu_ids the block layer will ensure that the
each submit queue is always run on the same CPU core. This means that
there is a high chance that sdebug_queue.qc_lock will be present in the
L1 cache and hence that the locking overhead will be minimal.
Bart.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2.5 3/6] scsi_debug: add multiple queue support
2016-05-01 2:44 ` [PATCH v2.5 3/6] scsi_debug: add multiple queue support Douglas Gilbert
2016-05-02 8:35 ` Hannes Reinecke
@ 2016-05-04 22:32 ` Bart Van Assche
2016-05-06 3:47 ` Douglas Gilbert
1 sibling, 1 reply; 24+ messages in thread
From: Bart Van Assche @ 2016-05-04 22:32 UTC (permalink / raw)
To: Douglas Gilbert, linux-scsi
Cc: martin.petersen, tomas.winkler, emilne, bart.vanassche
On 04/30/2016 07:44 PM, Douglas Gilbert wrote:
> +static struct sdebug_queue *get_queue(void)
> +{
> + struct sdebug_queue *sqp = sdebug_q_arr;
> +
> + return sqp + (raw_smp_processor_id() % submit_queues);
> +}
Does this function have the same purpose as blk_mq_map_queue()? If so,
why has this function been introduced instead of using blk_mq_map_queue()?
> @@ -5001,6 +5158,10 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,
> bool has_wlun_rl;
>
> scsi_set_resid(scp, 0);
> + if (sdebug_statistics) {
> + sqp = get_queue();
> + atomic_inc(&sqp->cmnd_count);
> + }
Why does scsi_debug_queuecommand() call get_queue() instead of
blk_mq_unique_tag() and blk_mq_unique_tag_to_hwq() which is what other
scsi-mq drivers do?
Is the role of the sqp->cmnd_count counter identical to that of
blk_mq_hw_ctx.queued? If so, can sqp->cmnd_count be left out and can
blk_mq_hw_ctx.queued be used instead?
> @@ -5168,6 +5328,16 @@ static int sdebug_driver_probe(struct device * dev)
> + if (sdebug_mq_available && (submit_queues > 1))
> + hpnt->nr_hw_queues = submit_queues;
There is already a submit_queues < 1 check in scsi_debug_init(). Is the
submit_queues > 1 check in sdebug_driver_probe() needed?
Bart.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2.5 5/6] scsi_debug: uuid for lu name
2016-05-01 2:44 ` [PATCH v2.5 5/6] scsi_debug: uuid for lu name Douglas Gilbert
2016-05-02 8:38 ` Hannes Reinecke
@ 2016-05-04 22:37 ` Bart Van Assche
1 sibling, 0 replies; 24+ messages in thread
From: Bart Van Assche @ 2016-05-04 22:37 UTC (permalink / raw)
To: Douglas Gilbert, linux-scsi
Cc: martin.petersen, tomas.winkler, emilne, bart.vanassche
On 04/30/2016 07:44 PM, Douglas Gilbert wrote:
> Permit changing of a LU name from a (fake) IEEE registered NAA (5)
> to a locally assigned UUID. Using a UUID (RFC 4122) for a SCSI
> designation descriptor (e.g. a LU name) was added in spc5r08.pdf
> (a draft INCITS standard) on 25 January 2016. Add parameter
> uuid_ctl to use a separate UUID for each LU (storage device) name.
> Additional option for all LU names to have the same UUID (since
> their storage is shared). Previous action of using NAA identifier
> for LU name remains the default.
>
> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2.5 6/6] scsi_debug: use locally assigned naa
2016-05-01 2:44 ` [PATCH v2.5 6/6] scsi_debug: use locally assigned naa Douglas Gilbert
2016-05-02 8:39 ` Hannes Reinecke
@ 2016-05-04 22:37 ` Bart Van Assche
1 sibling, 0 replies; 24+ messages in thread
From: Bart Van Assche @ 2016-05-04 22:37 UTC (permalink / raw)
To: Douglas Gilbert, linux-scsi; +Cc: martin.petersen, tomas.winkler, emilne
On 04/30/2016 07:44 PM, Douglas Gilbert wrote:
> For reported SAS addresses replace fake IEEE registered NAAs (5)
> with locally assigned NAAs (3).
>
> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2.5 3/6] scsi_debug: add multiple queue support
2016-05-04 22:32 ` Bart Van Assche
@ 2016-05-06 3:47 ` Douglas Gilbert
2016-05-06 4:20 ` Bart Van Assche
0 siblings, 1 reply; 24+ messages in thread
From: Douglas Gilbert @ 2016-05-06 3:47 UTC (permalink / raw)
To: Bart Van Assche, linux-scsi; +Cc: martin.petersen, tomas.winkler, emilne
On 2016-05-04 06:32 PM, Bart Van Assche wrote:
> On 04/30/2016 07:44 PM, Douglas Gilbert wrote:
>> +static struct sdebug_queue *get_queue(void)
>> +{
>> + struct sdebug_queue *sqp = sdebug_q_arr;
>> +
>> + return sqp + (raw_smp_processor_id() % submit_queues);
>> +}
>
> Does this function have the same purpose as blk_mq_map_queue()? If so, why has
> this function been introduced instead of using blk_mq_map_queue()?
No, it is copied from drivers/block/null_blk.c, the nullb_to_queue()
function. scsi_lib.c seems to be the only user of blk_mq_map_queue().
>> @@ -5001,6 +5158,10 @@ static int scsi_debug_queuecommand(struct Scsi_Host
>> *shost,
>> bool has_wlun_rl;
>>
>> scsi_set_resid(scp, 0);
>> + if (sdebug_statistics) {
>> + sqp = get_queue();
>> + atomic_inc(&sqp->cmnd_count);
>> + }
>
> Why does scsi_debug_queuecommand() call get_queue() instead of
> blk_mq_unique_tag() and blk_mq_unique_tag_to_hwq() which is what other scsi-mq
> drivers do?
Okay I have switched to using:
u32 tag = blk_mq_unique_tag(cmnd->request);
u16 hwq = blk_mq_unique_tag_to_hwq(tag);
return sqp + hwq;
and as far as I can tell it works just as well as:
return sqp + (raw_smp_processor_id() % submit_queues);
> Is the role of the sqp->cmnd_count counter identical to that of
> blk_mq_hw_ctx.queued? If so, can sqp->cmnd_count be left out and can
> blk_mq_hw_ctx.queued be used instead?
Not quite, it counts commands in, some of which may get
responded to in the same thread (e.g. when a SCSI illegal request
type error is being reported). But it is very close to my
"completions" count.
So I'll move those counters back into file scope which simplifies
scsi_debug's logic. The cmnd_counter is used in the injection of
pseudo errors "every_nth" command. Can blk_mq_hw_ctx.queued be
viewed from the user space?
>> @@ -5168,6 +5328,16 @@ static int sdebug_driver_probe(struct device * dev)
>> + if (sdebug_mq_available && (submit_queues > 1))
>> + hpnt->nr_hw_queues = submit_queues;
>
> There is already a submit_queues < 1 check in scsi_debug_init(). Is the
> submit_queues > 1 check in sdebug_driver_probe() needed?
I noticed that the other (2) scsi mq LLDs take care only to set
hpnt->nr_hw_queues when the submit_queues > 1 .
Tomorrow I plan to issue a "v3" of this patchset.
Doug Gilbert
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2.5 3/6] scsi_debug: add multiple queue support
2016-05-06 3:47 ` Douglas Gilbert
@ 2016-05-06 4:20 ` Bart Van Assche
0 siblings, 0 replies; 24+ messages in thread
From: Bart Van Assche @ 2016-05-06 4:20 UTC (permalink / raw)
To: dgilbert, linux-scsi; +Cc: martin.petersen, tomas.winkler, emilne
On 05/05/16 20:47, Douglas Gilbert wrote:
> On 2016-05-04 06:32 PM, Bart Van Assche wrote:
>> On 04/30/2016 07:44 PM, Douglas Gilbert wrote:
>>> +static struct sdebug_queue *get_queue(void)
>>> +{
>>> + struct sdebug_queue *sqp = sdebug_q_arr;
>>> +
>>> + return sqp + (raw_smp_processor_id() % submit_queues);
>>> +}
>>
>> Does this function have the same purpose as blk_mq_map_queue()? If so,
>> why has
>> this function been introduced instead of using blk_mq_map_queue()?
>
> No, it is copied from drivers/block/null_blk.c, the nullb_to_queue()
> function. scsi_lib.c seems to be the only user of blk_mq_map_queue().
>
>>> @@ -5001,6 +5158,10 @@ static int scsi_debug_queuecommand(struct
>>> Scsi_Host
>>> *shost,
>>> bool has_wlun_rl;
>>>
>>> scsi_set_resid(scp, 0);
>>> + if (sdebug_statistics) {
>>> + sqp = get_queue();
>>> + atomic_inc(&sqp->cmnd_count);
>>> + }
>>
>> Why does scsi_debug_queuecommand() call get_queue() instead of
>> blk_mq_unique_tag() and blk_mq_unique_tag_to_hwq() which is what other
>> scsi-mq
>> drivers do?
>
> Okay I have switched to using:
> u32 tag = blk_mq_unique_tag(cmnd->request);
> u16 hwq = blk_mq_unique_tag_to_hwq(tag);
> return sqp + hwq;
>
> and as far as I can tell it works just as well as:
> return sqp + (raw_smp_processor_id() % submit_queues);
Thanks for the feedback. I was not yet aware that null_blk also follows
this approach. Apparently null_blk has its own prep function and that
prep function needs access to the submit queue. Maybe that is why
null_blk has its own submit queue selection function. However, I'd like
to see that kind of code being moved from the null_blk driver into the
blk-mq core.
>> Is the role of the sqp->cmnd_count counter identical to that of
>> blk_mq_hw_ctx.queued? If so, can sqp->cmnd_count be left out and can
>> blk_mq_hw_ctx.queued be used instead?
>
> Not quite, it counts commands in, some of which may get
> responded to in the same thread (e.g. when a SCSI illegal request
> type error is being reported). But it is very close to my
> "completions" count.
>
> So I'll move those counters back into file scope which simplifies
> scsi_debug's logic. The cmnd_counter is used in the injection of
> pseudo errors "every_nth" command. Can blk_mq_hw_ctx.queued be
> viewed from the user space?
As far as I can see, yes. From block/blk-mq-sysfs.c:
static ssize_t blk_mq_hw_sysfs_queued_show(struct blk_mq_hw_ctx *hctx,
char *page)
{
return sprintf(page, "%lu\n", hctx->queued);
}
>>> @@ -5168,6 +5328,16 @@ static int sdebug_driver_probe(struct device *
>>> dev)
>>> + if (sdebug_mq_available && (submit_queues > 1))
>>> + hpnt->nr_hw_queues = submit_queues;
>>
>> There is already a submit_queues < 1 check in scsi_debug_init(). Is the
>> submit_queues > 1 check in sdebug_driver_probe() needed?
>
> I noticed that the other (2) scsi mq LLDs take care only to set
> hpnt->nr_hw_queues when the submit_queues > 1 .
You might have overlooked the ib_srp driver. That driver is a SCSI LLD
to which support for multiple hardware queues was added before it was
added to the lpfc and virtio_scsi drivers.
I haven't found any code in any of these three drivers that only
modifies nr_hw_queues if submit_queues > 1 ? Anyway, this is a detail
and something I do not consider important.
Bart.
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2016-05-06 4:21 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-01 2:44 [PATCH v2.5 0/6] scsi_debug: rebase second half of series Douglas Gilbert
2016-05-01 2:44 ` [PATCH v2.5 1/6] scsi_debug: use pdt constants Douglas Gilbert
2016-05-02 8:25 ` Hannes Reinecke
2016-05-03 23:56 ` Bart Van Assche
2016-05-01 2:44 ` [PATCH v2.5 2/6] scsi_debug: rework resp_report_luns Douglas Gilbert
2016-05-02 8:25 ` Hannes Reinecke
2016-05-02 8:29 ` Winkler, Tomas
2016-05-03 23:58 ` Bart Van Assche
2016-05-01 2:44 ` [PATCH v2.5 3/6] scsi_debug: add multiple queue support Douglas Gilbert
2016-05-02 8:35 ` Hannes Reinecke
2016-05-02 15:35 ` Douglas Gilbert
2016-05-04 22:09 ` Bart Van Assche
2016-05-04 22:32 ` Bart Van Assche
2016-05-06 3:47 ` Douglas Gilbert
2016-05-06 4:20 ` Bart Van Assche
2016-05-01 2:44 ` [PATCH v2.5 4/6] scsi_debug: vpd and mode page work Douglas Gilbert
2016-05-02 8:38 ` Hannes Reinecke
2016-05-02 15:48 ` Douglas Gilbert
2016-05-01 2:44 ` [PATCH v2.5 5/6] scsi_debug: uuid for lu name Douglas Gilbert
2016-05-02 8:38 ` Hannes Reinecke
2016-05-04 22:37 ` Bart Van Assche
2016-05-01 2:44 ` [PATCH v2.5 6/6] scsi_debug: use locally assigned naa Douglas Gilbert
2016-05-02 8:39 ` Hannes Reinecke
2016-05-04 22:37 ` Bart Van Assche
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.