[PATCH v5 00/10] scsi:scsi_debug: Add error injection for single device

[PATCH v5 00/10] scsi:scsi_debug: Add error injection for single device

[Wenchao Hao]

The original error injection mechanism was based on scsi_host which
could not inject fault for a single SCSI device.

This patchset provides the ability to inject errors for a single
SCSI device. Now we supports inject timeout errors, queuecommand
errors, and hostbyte, driverbyte, statusbyte, and sense data for
specific SCSI Command. Two new error injection is defined to make
abort command or reset LUN failed.

Besides error injection for single device, this patchset add a
new interface to make reset target failed for each scsi_target.

The first two patch add an debugfs interface to add and inquiry single
device's error injection info; the third patch defined how to remove
an injection which has been added. The following 5 patches use the
injection info and generate the related error type. The last one just
Add a new interface to make reset target failed.

V5:
  - Using rcu list to sync between error inject add, remove and check
  - Add module parameter "allow_restart" to control scsi_device's
    allow_restart flag

V4:
  - Fix BUG_ON triggered by schedule in atomic context when rmmod scsi_debug
    Closes: https://lore.kernel.org/oe-lkp/202308031027.5941ce5f-oliver.sang@intel.com

V3:
  - Add two more error types to fail abort command and lun reset
  - Fix memleak when rmmod scsi_debug without clearing errors injected
  - Fix memkeak because did not implement release in sdebug_error_fops
  - Fix possible NULL point access in scsi_debug_slave_destroy
  - Move specific error type's description to each single patch which
    implement this error type
  - Add interface to make target reset fail

V2:
  - Using debugfs rather than sysfs attribute interface to manage error

Wenchao Hao (10):
  scsi: scsi_debug: create scsi_debug directory in the debugfs
    filesystem
  scsi: scsi_debug: Add interface to manage single device's error inject
  scsi: scsi_debug: Define grammar to remove added error injection
  scsi: scsi_debug: timeout command if the error is injected
  scsi: scsi_debug: Return failed value if the error is injected
  scsi: scsi_debug: set command's result and sense data if the error is
    injected
  scsi: scsi_debug: Add new error injection abort failed
  scsi: scsi_debug: Add new error injection reset lun failed
  scsi: scsi_debug: Add debugfs interface to fail target reset
  scsi: scsi_debug: Add param to control sdev's allow_restart

 drivers/scsi/scsi_debug.c | 557 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 552 insertions(+), 5 deletions(-)

-- 
2.32.0

[PATCH v5 01/10] scsi: scsi_debug: create scsi_debug directory in the debugfs filesystem

[Wenchao Hao]

Create directory scsi_debug in the root of the debugfs filesystem.
Prepare to add interface for manage error injection.

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

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 9c0af50501f9..35c336271b13 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -41,6 +41,7 @@
 #include <linux/random.h>
 #include <linux/xarray.h>
 #include <linux/prefetch.h>
+#include <linux/debugfs.h>
 
 #include <net/checksum.h>
 
@@ -862,6 +863,8 @@ static const int device_qfull_result =
 
 static const int condition_met_result = SAM_STAT_CONDITION_MET;
 
+static struct dentry *sdebug_debugfs_root;
+
 
 /* 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
@@ -7011,6 +7014,8 @@ static int __init scsi_debug_init(void)
 		goto driver_unreg;
 	}
 
+	sdebug_debugfs_root = debugfs_create_dir("scsi_debug", NULL);
+
 	for (k = 0; k < hosts_to_add; k++) {
 		if (want_store && k == 0) {
 			ret = sdebug_add_host_helper(idx);
@@ -7057,6 +7062,7 @@ static void __exit scsi_debug_exit(void)
 
 	sdebug_erase_all_stores(false);
 	xa_destroy(per_store_ap);
+	debugfs_remove(sdebug_debugfs_root);
 }
 
 device_initcall(scsi_debug_init);
-- 
2.32.0

Re: [PATCH v5 01/10] scsi: scsi_debug: create scsi_debug directory in the debugfs filesystem

[Douglas Gilbert]

On 2023-09-22 05:28, Wenchao Hao wrote:
> Create directory scsi_debug in the root of the debugfs filesystem.
> Prepare to add interface for manage error injection.
> 
> Acked-by: Douglas Gilbert <dgilbert@interlog.com>
> Signed-off-by: Wenchao Hao <haowenchao2@huawei.com>
> ---
>   drivers/scsi/scsi_debug.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 9c0af50501f9..35c336271b13 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -41,6 +41,7 @@
>   #include <linux/random.h>
>   #include <linux/xarray.h>
>   #include <linux/prefetch.h>
> +#include <linux/debugfs.h>
>   
>   #include <net/checksum.h>
>   
> @@ -862,6 +863,8 @@ static const int device_qfull_result =
>   
>   static const int condition_met_result = SAM_STAT_CONDITION_MET;
>   
> +static struct dentry *sdebug_debugfs_root;
> +
>   
>   /* 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
> @@ -7011,6 +7014,8 @@ static int __init scsi_debug_init(void)
>   		goto driver_unreg;
>   	}
>   
> +	sdebug_debugfs_root = debugfs_create_dir("scsi_debug", NULL);

debugfs_create_dir() can fail and return NULL. Looking at other drivers, most
seem to assume it will work. Since the scsi_debug driver is often used to test
abnormal situations, perhaps adding something like:
     if (!sdebug_debugfs_root)
         pr_info("%s: failed to create initial debugfs directory\n", __func__);

might save someone a bit of time if a NULL dereference on sdebug_debugfs_root
follows later. That is what the mpt3sas driver does.

Doug Gilbert

> +
>   	for (k = 0; k < hosts_to_add; k++) {
>   		if (want_store && k == 0) {
>   			ret = sdebug_add_host_helper(idx);
> @@ -7057,6 +7062,7 @@ static void __exit scsi_debug_exit(void)
>   
>   	sdebug_erase_all_stores(false);
>   	xa_destroy(per_store_ap);
> +	debugfs_remove(sdebug_debugfs_root);
>   }
>   
>   device_initcall(scsi_debug_init);

Re: [PATCH v5 01/10] scsi: scsi_debug: create scsi_debug directory in the debugfs filesystem

[Wenchao Hao]

On 2023/9/28 9:13, Douglas Gilbert wrote:
> On 2023-09-22 05:28, Wenchao Hao wrote:
>> Create directory scsi_debug in the root of the debugfs filesystem.
>> Prepare to add interface for manage error injection.
>>
>> Acked-by: Douglas Gilbert <dgilbert@interlog.com>
>> Signed-off-by: Wenchao Hao <haowenchao2@huawei.com>
>> ---
>>   drivers/scsi/scsi_debug.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
>> index 9c0af50501f9..35c336271b13 100644
>> --- a/drivers/scsi/scsi_debug.c
>> +++ b/drivers/scsi/scsi_debug.c
>> @@ -41,6 +41,7 @@
>>   #include <linux/random.h>
>>   #include <linux/xarray.h>
>>   #include <linux/prefetch.h>
>> +#include <linux/debugfs.h>
>>   #include <net/checksum.h>
>> @@ -862,6 +863,8 @@ static const int device_qfull_result =
>>   static const int condition_met_result = SAM_STAT_CONDITION_MET;
>> +static struct dentry *sdebug_debugfs_root;
>> +
>>   /* 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
>> @@ -7011,6 +7014,8 @@ static int __init scsi_debug_init(void)
>>           goto driver_unreg;
>>       }
>> +    sdebug_debugfs_root = debugfs_create_dir("scsi_debug", NULL);
> 
> debugfs_create_dir() can fail and return NULL. Looking at other drivers, most
> seem to assume it will work. Since the scsi_debug driver is often used to test
> abnormal situations, perhaps adding something like:
>      if (!sdebug_debugfs_root)
>          pr_info("%s: failed to create initial debugfs directory\n", __func__);
> 
> might save someone a bit of time if a NULL dereference on sdebug_debugfs_root
> follows later. That is what the mpt3sas driver does.
> 

Yes, I would fix it by checking return value of debugfs related call
after your review suggestions for other patches.

> Doug Gilbert
> 
>> +
>>       for (k = 0; k < hosts_to_add; k++) {
>>           if (want_store && k == 0) {
>>               ret = sdebug_add_host_helper(idx);
>> @@ -7057,6 +7062,7 @@ static void __exit scsi_debug_exit(void)
>>       sdebug_erase_all_stores(false);
>>       xa_destroy(per_store_ap);
>> +    debugfs_remove(sdebug_debugfs_root);
>>   }
>>   device_initcall(scsi_debug_init);
> 
> 

[PATCH v5 02/10] scsi: scsi_debug: Add interface to manage single device's error inject

[Wenchao Hao]

This new facility uses the debugfs pseudo file system which is typically
mounted under the /sys/kernel/debug directory and requires root permissions
to access.

The interface file is found at /sys/kernel/debug/scsi_debug/<h:c:t:l>/error
where <h:c:t:l> identifies the device (logical unit (LU)) to inject errors
on.

For the following description the ${error} environment variable is assumed
to be set to/sys/kernel/debug/scsi_debug/1:0:0:0/error where 1:0:0:0 is a
pseudo device (LU) owned by the scsi_debug driver. Rules are written to
${error} in the normal sysfs fashion (e.g. 'echo "0 -2 0x12" > ${error}').

More than one rule can be active on a device at a time and inactive rules
(i.e. those whose Error count is 0) remain in the rule listing. The
existing rules can be read with 'cat ${error}' with one line output for
each rule.

The interface format is line-by-line, each line is an error injection rule.
Each rule contains integers separated by spaces, the first three columns
correspond to "Error code", "Error count" and "SCSI command", other
columns depend on Error code.

General rule format:
  +--------+------+-------------------------------------------------------+
  | Column | Type | Description                                           |
  +--------+------+-------------------------------------------------------+
  |   1    |  u8  | Error code                                            |
  |        |      |  0: timeout SCSI command                              |
  |        |      |  1: fail queuecommand, make queuecommand return       |
  |        |      |     given value                                       |
  |        |      |  2: fail command, finish command with SCSI status,    |
  |        |      |     sense key and ASC/ASCQ values                     |
  |        |      |  3: make abort commands for specific command fail     |
  |        |      |  4: make reset lun for specific command fail          |
  +--------+------+-------------------------------------------------------+
  |   2    |  s32 | Error count                                           |
  |        |      |  0: this rule will be ignored                         |
  |        |      |  positive: the rule will always take effect           |
  |        |      |  negative: the rule takes effect n times where -n is  |
  |        |      |            the value given. Ignored after n times     |
  +--------+------+-------------------------------------------------------+
  |   3    |  x8  | SCSI command opcode, 0xff for all commands            |
  +--------+------+-------------------------------------------------------+
  |  ...   |  xxx | Error type specific fields                            |
  +--------+------+-------------------------------------------------------+
Notes:
- when multiple error inject rules are added for the same SCSI command, the
  one with smaller error code will take effect (and the others will be
  ignored).
- if an same error (i.e. same Error code and SCSI command) is added, the
  older one will be overwritten.
- Currently, the basic types are (u8/u16/u32/u64/s8/s16/s32/s64) and the
  hexadecimal types (x8/x16/x32/x64)
- where a hexadecimal value is expected (e.g. Column 3: SCSI command
  opcode) the "0x" prefix is optional on the value (e.g. the INQUIRY opcode
  can be given as '0x12' or '12')
- when the Error count is negative, reading ${error} will show that value
  incrementing, stopping when it gets to 0

Acked-by: Douglas Gilbert <dgilbert@interlog.com>
Signed-off-by: Wenchao Hao <haowenchao2@huawei.com>
---
 drivers/scsi/scsi_debug.c | 206 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 202 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 35c336271b13..ca1e2f4878bc 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -286,6 +286,42 @@ struct sdeb_zone_state {	/* ZBC: per zone state */
 	sector_t z_wp;
 };
 
+enum sdebug_err_type {
+	ERR_TMOUT_CMD		= 0,	/* make specific scsi command timeout */
+	ERR_FAIL_QUEUE_CMD	= 1,	/* make specific scsi command's */
+					/* queuecmd return failed */
+	ERR_FAIL_CMD		= 2,	/* make specific scsi command's */
+					/* queuecmd return succeed but */
+					/* with errors set in scsi_cmnd */
+};
+
+struct sdebug_err_inject {
+	int type;
+	struct list_head list;
+	int cnt;
+	unsigned char cmd;
+	struct rcu_head rcu;
+
+	union {
+		/*
+		 * For ERR_FAIL_QUEUE_CMD
+		 */
+		int queuecmd_ret;
+
+		/*
+		 * For ERR_FAIL_CMD
+		 */
+		struct {
+			unsigned char host_byte;
+			unsigned char driver_byte;
+			unsigned char status_byte;
+			unsigned char sense_key;
+			unsigned char asc;
+			unsigned char asq;
+		};
+	};
+};
+
 struct sdebug_dev_info {
 	struct list_head dev_list;
 	unsigned int channel;
@@ -311,6 +347,10 @@ struct sdebug_dev_info {
 	unsigned int max_open;
 	ktime_t create_ts;	/* time since bootup that this device was created */
 	struct sdeb_zone_state *zstate;
+
+	struct dentry *debugfs_entry;
+	struct spinlock list_lock;
+	struct list_head inject_err_list;
 };
 
 struct sdebug_host_info {
@@ -865,6 +905,143 @@ static const int condition_met_result = SAM_STAT_CONDITION_MET;
 
 static struct dentry *sdebug_debugfs_root;
 
+static void sdebug_err_free(struct rcu_head *head)
+{
+	struct sdebug_err_inject *inject =
+		container_of(head, typeof(*inject), rcu);
+
+	kfree(inject);
+}
+
+static void sdebug_err_add(struct scsi_device *sdev, struct sdebug_err_inject *new)
+{
+	struct sdebug_dev_info *devip = (struct sdebug_dev_info *)sdev->hostdata;
+	struct sdebug_err_inject *err;
+
+	spin_lock(&devip->list_lock);
+	list_for_each_entry_rcu(err, &devip->inject_err_list, list) {
+		if (err->type == new->type && err->cmd == new->cmd) {
+			list_del_rcu(&err->list);
+			call_rcu(&err->rcu, sdebug_err_free);
+		}
+	}
+
+	list_add_tail_rcu(&new->list, &devip->inject_err_list);
+	spin_unlock(&devip->list_lock);
+}
+
+static int sdebug_error_show(struct seq_file *m, void *p)
+{
+	struct scsi_device *sdev = (struct scsi_device *)m->private;
+	struct sdebug_dev_info *devip = (struct sdebug_dev_info *)sdev->hostdata;
+	struct sdebug_err_inject *err;
+
+	seq_puts(m, "Type\tCount\tCommand\n");
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(err, &devip->inject_err_list, list) {
+		switch (err->type) {
+		case ERR_TMOUT_CMD:
+			seq_printf(m, "%d\t%d\t0x%x\n", err->type, err->cnt,
+				err->cmd);
+		break;
+
+		case ERR_FAIL_QUEUE_CMD:
+			seq_printf(m, "%d\t%d\t0x%x\t0x%x\n", err->type,
+				err->cnt, err->cmd, err->queuecmd_ret);
+		break;
+
+		case ERR_FAIL_CMD:
+			seq_printf(m, "%d\t%d\t0x%x\t0x%x 0x%x 0x%x 0x%x 0x%x 0x%x\n",
+				err->type, err->cnt, err->cmd,
+				err->host_byte, err->driver_byte,
+				err->status_byte, err->sense_key,
+				err->asc, err->asq);
+		break;
+		}
+	}
+	rcu_read_unlock();
+
+	return 0;
+}
+
+static int sdebug_error_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, sdebug_error_show, inode->i_private);
+}
+
+static ssize_t sdebug_error_write(struct file *file, const char __user *ubuf,
+		size_t count, loff_t *ppos)
+{
+	char *buf;
+	unsigned int inject_type;
+	struct sdebug_err_inject *inject;
+	struct scsi_device *sdev = (struct scsi_device *)file->f_inode->i_private;
+
+	buf = kmalloc(count, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	if (copy_from_user(buf, ubuf, count)) {
+		kfree(buf);
+		return -EFAULT;
+	}
+
+	if (sscanf(buf, "%d", &inject_type) != 1) {
+		kfree(buf);
+		return -EINVAL;
+	}
+
+	inject = kzalloc(sizeof(struct sdebug_err_inject), GFP_KERNEL);
+	if (!inject) {
+		kfree(buf);
+		return -ENOMEM;
+	}
+
+	switch (inject_type) {
+	case ERR_TMOUT_CMD:
+		if (sscanf(buf, "%d %d %hhx", &inject->type, &inject->cnt,
+			   &inject->cmd) != 3)
+			goto out_error;
+	break;
+
+	case ERR_FAIL_QUEUE_CMD:
+		if (sscanf(buf, "%d %d %hhx %x", &inject->type, &inject->cnt,
+			   &inject->cmd, &inject->queuecmd_ret) != 4)
+			goto out_error;
+	break;
+
+	case ERR_FAIL_CMD:
+		if (sscanf(buf, "%d %d %hhx %hhx %hhx %hhx %hhx %hhx %hhx",
+			   &inject->type, &inject->cnt, &inject->cmd,
+			   &inject->host_byte, &inject->driver_byte,
+			   &inject->status_byte, &inject->sense_key,
+			   &inject->asc, &inject->asq) != 9)
+			goto out_error;
+	break;
+
+	default:
+		goto out_error;
+	break;
+	}
+
+	kfree(buf);
+	sdebug_err_add(sdev, inject);
+
+	return count;
+
+out_error:
+	kfree(buf);
+	kfree(inject);
+	return -EINVAL;
+}
+
+static const struct file_operations sdebug_error_fops = {
+	.open	= sdebug_error_open,
+	.read	= seq_read,
+	.write	= sdebug_error_write,
+	.release = single_release,
+};
 
 /* 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
@@ -5099,6 +5276,8 @@ static struct sdebug_dev_info *sdebug_device_create(
 		}
 		devip->create_ts = ktime_get_boottime();
 		atomic_set(&devip->stopped, (sdeb_tur_ms_to_ready > 0 ? 2 : 0));
+		spin_lock_init(&devip->list_lock);
+		INIT_LIST_HEAD(&devip->inject_err_list);
 		list_add_tail(&devip->dev_list, &sdbg_host->dev_info_list);
 	}
 	return devip;
@@ -5144,6 +5323,7 @@ static int scsi_debug_slave_alloc(struct scsi_device *sdp)
 	if (sdebug_verbose)
 		pr_info("slave_alloc <%u %u %u %llu>\n",
 		       sdp->host->host_no, sdp->channel, sdp->id, sdp->lun);
+
 	return 0;
 }
 
@@ -5166,6 +5346,12 @@ static int scsi_debug_slave_configure(struct scsi_device *sdp)
 	if (sdebug_no_uld)
 		sdp->no_uld_attach = 1;
 	config_cdb_len(sdp);
+
+	devip->debugfs_entry = debugfs_create_dir(dev_name(&sdp->sdev_dev),
+				sdebug_debugfs_root);
+	debugfs_create_file("error", 0600, devip->debugfs_entry, sdp,
+				&sdebug_error_fops);
+
 	return 0;
 }
 
@@ -5173,15 +5359,27 @@ static void scsi_debug_slave_destroy(struct scsi_device *sdp)
 {
 	struct sdebug_dev_info *devip =
 		(struct sdebug_dev_info *)sdp->hostdata;
+	struct sdebug_err_inject *err;
 
 	if (sdebug_verbose)
 		pr_info("slave_destroy <%u %u %u %llu>\n",
 		       sdp->host->host_no, sdp->channel, sdp->id, sdp->lun);
-	if (devip) {
-		/* make this slot available for re-use */
-		devip->used = false;
-		sdp->hostdata = NULL;
+
+	if (!devip)
+		return;
+
+	spin_lock(&devip->list_lock);
+	list_for_each_entry_rcu(err, &devip->inject_err_list, list) {
+		list_del_rcu(&err->list);
+		call_rcu(&err->rcu, sdebug_err_free);
 	}
+	spin_unlock(&devip->list_lock);
+
+	debugfs_remove(devip->debugfs_entry);
+
+	/* make this slot available for re-use */
+	devip->used = false;
+	sdp->hostdata = NULL;
 }
 
 /* Returns true if we require the queued memory to be freed by the caller. */
-- 
2.32.0

[PATCH v5 03/10] scsi: scsi_debug: Define grammar to remove added error injection

[Wenchao Hao]

The grammar to remove error injection is a line with fixed 3 columns
separated by spaces.

First column is fixed to "-". It tells this is a removal operation.
Second column is the error code to match.
Third column is the scsi command to match.

For example the following command would remove timeout injection of
inquiry command.
    echo "- 0 0x12" > /sys/kernel/debug/scsi_debug/0:0:0:1/error

Acked-by: Douglas Gilbert <dgilbert@interlog.com>
Signed-off-by: Wenchao Hao <haowenchao2@huawei.com>
---
 drivers/scsi/scsi_debug.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index ca1e2f4878bc..6235e828c430 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -930,6 +930,34 @@ static void sdebug_err_add(struct scsi_device *sdev, struct sdebug_err_inject *n
 	spin_unlock(&devip->list_lock);
 }
 
+static int sdebug_err_remove(struct scsi_device *sdev, const char *buf, size_t count)
+{
+	struct sdebug_dev_info *devip = (struct sdebug_dev_info *)sdev->hostdata;
+	struct sdebug_err_inject *err;
+	int type;
+	unsigned char cmd;
+
+	if (sscanf(buf, "- %d %hhx", &type, &cmd) != 2) {
+		kfree(buf);
+		return -EINVAL;
+	}
+
+	spin_lock(&devip->list_lock);
+	list_for_each_entry_rcu(err, &devip->inject_err_list, list) {
+		if (err->type == type && err->cmd == cmd) {
+			list_del_rcu(&err->list);
+			call_rcu(&err->rcu, sdebug_err_free);
+			spin_unlock(&devip->list_lock);
+			kfree(buf);
+			return count;
+		}
+	}
+	spin_unlock(&devip->list_lock);
+
+	kfree(buf);
+	return -EINVAL;
+}
+
 static int sdebug_error_show(struct seq_file *m, void *p)
 {
 	struct scsi_device *sdev = (struct scsi_device *)m->private;
@@ -987,6 +1015,9 @@ static ssize_t sdebug_error_write(struct file *file, const char __user *ubuf,
 		return -EFAULT;
 	}
 
+	if (buf[0] == '-')
+		return sdebug_err_remove(sdev, buf, count);
+
 	if (sscanf(buf, "%d", &inject_type) != 1) {
 		kfree(buf);
 		return -EINVAL;
-- 
2.32.0

[PATCH v5 04/10] scsi: scsi_debug: timeout command if the error is injected

[Wenchao Hao]

If a timeout error is injected, return 0 from scsi_debug_queuecommand to
make the command timeout.

Timeout SCSI command format:
  +--------+------+-------------------------------------------------------+
  | Column | Type | Description                                           |
  +--------+------+-------------------------------------------------------+
  |   1    |  u8  | Error type, fixed to 0x0                              |
  +--------+------+-------------------------------------------------------+
  |   2    |  s32 | Error count                                           |
  |        |      |  0: this rule will be ignored                         |
  |        |      |  positive: the rule will always take effect           |
  |        |      |  negative: the rule takes effect n times where -n is  |
  |        |      |            the value given. Ignored after n times     |
  +--------+------+-------------------------------------------------------+
  |   3    |  x8  | SCSI command opcode, 0xff for all commands            |
  +--------+------+-------------------------------------------------------+
Examples:
    error=/sys/kernel/debug/scsi_debug/0:0:0:1/error
    echo "0 -10 0x12" > ${error}
will make the device's inquiry command timeout 10 times.
    echo "0 1 0x12" > ${error}
will make the device's inquiry timeout each time it is invoked on this
device.

Acked-by: Douglas Gilbert <dgilbert@interlog.com>
Signed-off-by: Wenchao Hao <haowenchao2@huawei.com>
---
 drivers/scsi/scsi_debug.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 6235e828c430..785ad5e5b4a4 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -7731,6 +7731,34 @@ static int sdebug_blk_mq_poll(struct Scsi_Host *shost, unsigned int queue_num)
 	return num_entries;
 }
 
+static int sdebug_timeout_cmd(struct scsi_cmnd *cmnd)
+{
+	struct scsi_device *sdp = cmnd->device;
+	struct sdebug_dev_info *devip = (struct sdebug_dev_info *)sdp->hostdata;
+	struct sdebug_err_inject *err;
+	unsigned char *cmd = cmnd->cmnd;
+	int ret = 0;
+
+	if (devip == NULL)
+		return 0;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(err, &devip->inject_err_list, list) {
+		if (err->type == ERR_TMOUT_CMD &&
+		    (err->cmd == cmd[0] || err->cmd == 0xff)) {
+			ret = !!err->cnt;
+			if (err->cnt < 0)
+				err->cnt++;
+
+			rcu_read_unlock();
+			return ret;
+		}
+	}
+	rcu_read_unlock();
+
+	return 0;
+}
+
 static int scsi_debug_queuecommand(struct Scsi_Host *shost,
 				   struct scsi_cmnd *scp)
 {
@@ -7789,6 +7817,12 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,
 		if (NULL == devip)
 			goto err_out;
 	}
+
+	if (sdebug_timeout_cmd(scp)) {
+		scmd_printk(KERN_INFO, scp, "timeout command 0x%x\n", opcode);
+		return 0;
+	}
+
 	if (unlikely(inject_now && !atomic_read(&sdeb_inject_pending)))
 		atomic_set(&sdeb_inject_pending, 1);
 
-- 
2.32.0

[PATCH v5 05/10] scsi: scsi_debug: Return failed value if the error is injected

[Wenchao Hao]

If a fail queuecommand error is injected, return the failed value defined
in the rule from queuecommand.

Make queuecommand return format:
  +--------+------+-------------------------------------------------------+
  | Column | Type | Description                                           |
  +--------+------+-------------------------------------------------------+
  |   1    |  u8  | Error type, fixed to 0x1                              |
  +--------+------+-------------------------------------------------------+
  |   2    |  s32 | Error count                                           |
  |        |      |  0: this rule will be ignored                         |
  |        |      |  positive: the rule will always take effect           |
  |        |      |  negative: the rule takes effect n times where -n is  |
  |        |      |            the value given. Ignored after n times     |
  +--------+------+-------------------------------------------------------+
  |   3    |  x8  | SCSI command opcode, 0xff for all commands            |
  +--------+------+-------------------------------------------------------+
  |   4    |  x32 | The queuecommand() return value we want               |
  +--------+------+-------------------------------------------------------+

Examples:
    error=/sys/kernel/debug/scsi_debug/0:0:0:1/error
    echo "1 1 0x12 0x1055" > ${error}
will make each INQUIRY command sent to that device return
0x1055 (SCSI_MLQUEUE_HOST_BUSY).

Acked-by: Douglas Gilbert <dgilbert@interlog.com>
Signed-off-by: Wenchao Hao <haowenchao2@huawei.com>
---
 drivers/scsi/scsi_debug.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 785ad5e5b4a4..dea40d983155 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -7759,6 +7759,34 @@ static int sdebug_timeout_cmd(struct scsi_cmnd *cmnd)
 	return 0;
 }
 
+static int sdebug_fail_queue_cmd(struct scsi_cmnd *cmnd)
+{
+	struct scsi_device *sdp = cmnd->device;
+	struct sdebug_dev_info *devip = (struct sdebug_dev_info *)sdp->hostdata;
+	struct sdebug_err_inject *err;
+	unsigned char *cmd = cmnd->cmnd;
+	int ret = 0;
+
+	if (devip == NULL)
+		return 0;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(err, &devip->inject_err_list, list) {
+		if (err->type == ERR_FAIL_QUEUE_CMD &&
+		    (err->cmd == cmd[0] || err->cmd == 0xff)) {
+			ret = err->cnt ? err->queuecmd_ret : 0;
+			if (err->cnt < 0)
+				err->cnt++;
+
+			rcu_read_unlock();
+			return ret;
+		}
+	}
+	rcu_read_unlock();
+
+	return 0;
+}
+
 static int scsi_debug_queuecommand(struct Scsi_Host *shost,
 				   struct scsi_cmnd *scp)
 {
@@ -7778,6 +7806,7 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,
 	u8 opcode = cmd[0];
 	bool has_wlun_rl;
 	bool inject_now;
+	int ret = 0;
 
 	scsi_set_resid(scp, 0);
 	if (sdebug_statistics) {
@@ -7823,6 +7852,13 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,
 		return 0;
 	}
 
+	ret = sdebug_fail_queue_cmd(scp);
+	if (ret) {
+		scmd_printk(KERN_INFO, scp, "fail queue command 0x%x with 0x%x\n",
+				opcode, ret);
+		return ret;
+	}
+
 	if (unlikely(inject_now && !atomic_read(&sdeb_inject_pending)))
 		atomic_set(&sdeb_inject_pending, 1);
 
-- 
2.32.0

[PATCH v5 06/10] scsi: scsi_debug: set command's result and sense data if the error is injected

[Wenchao Hao]

If a fail commnd error is injected, set the command's status and sense
data then finish this scsi command.

Set SCSI command's status and sense data format:
  +--------+------+-------------------------------------------------------+
  | Column | Type | Description                                           |
  +--------+------+-------------------------------------------------------+
  |   1    |  u8  | Error type, fixed to 0x2                              |
  +--------+------+-------------------------------------------------------+
  |   2    |  s32 | Error Count                                           |
  |        |      |  0: the rule will be ignored                          |
  |        |      |  positive: the rule will always take effect           |
  |        |      |  negative: the rule takes effect n times where -n is  |
  |        |      |            the value given. Ignored after n times     |
  +--------+------+-------------------------------------------------------+
  |   3    |  x8  | SCSI command opcode, 0xff for all commands            |
  +--------+------+-------------------------------------------------------+
  |   4    |  x8  | Host byte in scsi_cmd::status                         |
  |        |      | [scsi_cmd::status has 32 bits holding these 3 bytes]  |
  +--------+------+-------------------------------------------------------+
  |   5    |  x8  | Driver byte in scsi_cmd::status                       |
  +--------+------+-------------------------------------------------------+
  |   6    |  x8  | SCSI Status byte in scsi_cmd::status                  |
  +--------+------+-------------------------------------------------------+
  |   7    |  x8  | SCSI Sense Key in scsi_cmnd                           |
  +--------+------+-------------------------------------------------------+
  |   8    |  x8  | SCSI ASC in scsi_cmnd                                 |
  +--------+------+-------------------------------------------------------+
  |   9    |  x8  | SCSI ASCQ in scsi_cmnd                                |
  +--------+------+-------------------------------------------------------+
Examples:
    error=/sys/kernel/debug/scsi_debug/0:0:0:1/error
    echo "2 -10 0x88 0 0 0x2 0x3 0x11 0x0" >${error}
will make device's read command return with media error with additional
sense of "Unrecovered read error" (UNC):

Acked-by: Douglas Gilbert <dgilbert@interlog.com>
Signed-off-by: Wenchao Hao <haowenchao2@huawei.com>
---
 drivers/scsi/scsi_debug.c | 53 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index dea40d983155..fe1f7421f617 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -7787,6 +7787,48 @@ static int sdebug_fail_queue_cmd(struct scsi_cmnd *cmnd)
 	return 0;
 }
 
+static int sdebug_fail_cmd(struct scsi_cmnd *cmnd, int *retval,
+			   struct sdebug_err_inject *info)
+{
+	struct scsi_device *sdp = cmnd->device;
+	struct sdebug_dev_info *devip = (struct sdebug_dev_info *)sdp->hostdata;
+	struct sdebug_err_inject *err;
+	unsigned char *cmd = cmnd->cmnd;
+	int ret = 0;
+	int result;
+
+	if (devip == NULL)
+		return 0;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(err, &devip->inject_err_list, list) {
+		if (err->type == ERR_FAIL_CMD &&
+		    (err->cmd == cmd[0] || err->cmd == 0xff)) {
+			if (!err->cnt) {
+				rcu_read_unlock();
+				return 0;
+			}
+
+			ret = !!err->cnt;
+			rcu_read_unlock();
+			goto out_handle;
+		}
+	}
+	rcu_read_unlock();
+
+	return 0;
+
+out_handle:
+	if (err->cnt < 0)
+		err->cnt++;
+	mk_sense_buffer(cmnd, err->sense_key, err->asc, err->asq);
+	result = err->status_byte | err->host_byte << 16 | err->driver_byte << 24;
+	*info = *err;
+	*retval = schedule_resp(cmnd, devip, result, NULL, 0, 0);
+
+	return ret;
+}
+
 static int scsi_debug_queuecommand(struct Scsi_Host *shost,
 				   struct scsi_cmnd *scp)
 {
@@ -7807,6 +7849,7 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,
 	bool has_wlun_rl;
 	bool inject_now;
 	int ret = 0;
+	struct sdebug_err_inject err;
 
 	scsi_set_resid(scp, 0);
 	if (sdebug_statistics) {
@@ -7859,6 +7902,16 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,
 		return ret;
 	}
 
+	if (sdebug_fail_cmd(scp, &ret, &err)) {
+		scmd_printk(KERN_INFO, scp,
+			"fail command 0x%x with hostbyte=0x%x, "
+			"driverbyte=0x%x, statusbyte=0x%x, "
+			"sense_key=0x%x, asc=0x%x, asq=0x%x\n",
+			opcode, err.host_byte, err.driver_byte,
+			err.status_byte, err.sense_key, err.asc, err.asq);
+		return ret;
+	}
+
 	if (unlikely(inject_now && !atomic_read(&sdeb_inject_pending)))
 		atomic_set(&sdeb_inject_pending, 1);
 
-- 
2.32.0

Re: [PATCH v5 06/10] scsi: scsi_debug: set command's result and sense data if the error is injected

[Dan Carpenter]

Hi Wenchao,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Wenchao-Hao/scsi-scsi_debug-create-scsi_debug-directory-in-the-debugfs-filesystem/20230922-173226
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
patch link:    https://lore.kernel.org/r/20230922092906.2645265-7-haowenchao2%40huawei.com
patch subject: [PATCH v5 06/10] scsi: scsi_debug: set command's result and sense data if the error is injected
config: x86_64-randconfig-161-20231003 (https://download.01.org/0day-ci/archive/20231005/202310050209.VOl6gV40-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20231005/202310050209.VOl6gV40-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202310050209.VOl6gV40-lkp@intel.com/

smatch warnings:
drivers/scsi/scsi_debug.c:7912 scsi_debug_queuecommand() warn: missing error code? 'ret'

vim +/ret +7912 drivers/scsi/scsi_debug.c

fd32119b0deac1 Douglas Gilbert 2016-04-25  7832  static int scsi_debug_queuecommand(struct Scsi_Host *shost,
fd32119b0deac1 Douglas Gilbert 2016-04-25  7833  				   struct scsi_cmnd *scp)
c2248fc974df7b Douglas Gilbert 2014-11-24  7834  {
c2248fc974df7b Douglas Gilbert 2014-11-24  7835  	u8 sdeb_i;
c2248fc974df7b Douglas Gilbert 2014-11-24  7836  	struct scsi_device *sdp = scp->device;
c2248fc974df7b Douglas Gilbert 2014-11-24  7837  	const struct opcode_info_t *oip;
c2248fc974df7b Douglas Gilbert 2014-11-24  7838  	const struct opcode_info_t *r_oip;
c2248fc974df7b Douglas Gilbert 2014-11-24  7839  	struct sdebug_dev_info *devip;
c2248fc974df7b Douglas Gilbert 2014-11-24  7840  	u8 *cmd = scp->cmnd;
c2248fc974df7b Douglas Gilbert 2014-11-24  7841  	int (*r_pfp)(struct scsi_cmnd *, struct sdebug_dev_info *);
f66b85171a0ebd Martin Wilck    2018-02-14  7842  	int (*pfp)(struct scsi_cmnd *, struct sdebug_dev_info *) = NULL;
c2248fc974df7b Douglas Gilbert 2014-11-24  7843  	int k, na;
c2248fc974df7b Douglas Gilbert 2014-11-24  7844  	int errsts = 0;
ad0c7775e745d2 Douglas Gilbert 2020-08-21  7845  	u64 lun_index = sdp->lun & 0x3FFF;
c2248fc974df7b Douglas Gilbert 2014-11-24  7846  	u32 flags;
c2248fc974df7b Douglas Gilbert 2014-11-24  7847  	u16 sa;
c2248fc974df7b Douglas Gilbert 2014-11-24  7848  	u8 opcode = cmd[0];
c2248fc974df7b Douglas Gilbert 2014-11-24  7849  	bool has_wlun_rl;
3a90a63d02b8b7 Douglas Gilbert 2020-07-12  7850  	bool inject_now;
929aad8ff0578d Wenchao Hao     2023-09-22  7851  	int ret = 0;
cc36ffafc0f7e6 Wenchao Hao     2023-09-22  7852  	struct sdebug_err_inject err;
c2248fc974df7b Douglas Gilbert 2014-11-24  7853  
c2248fc974df7b Douglas Gilbert 2014-11-24  7854  	scsi_set_resid(scp, 0);
3a90a63d02b8b7 Douglas Gilbert 2020-07-12  7855  	if (sdebug_statistics) {
c483739430f107 Douglas Gilbert 2016-05-06  7856  		atomic_inc(&sdebug_cmnd_count);
3a90a63d02b8b7 Douglas Gilbert 2020-07-12  7857  		inject_now = inject_on_this_cmd();
3a90a63d02b8b7 Douglas Gilbert 2020-07-12  7858  	} else {
3a90a63d02b8b7 Douglas Gilbert 2020-07-12  7859  		inject_now = false;
3a90a63d02b8b7 Douglas Gilbert 2020-07-12  7860  	}
f46eb0e9fc763b Douglas Gilbert 2016-04-25  7861  	if (unlikely(sdebug_verbose &&
f46eb0e9fc763b Douglas Gilbert 2016-04-25  7862  		     !(SDEBUG_OPT_NO_CDB_NOISE & sdebug_opts))) {
c2248fc974df7b Douglas Gilbert 2014-11-24  7863  		char b[120];
c2248fc974df7b Douglas Gilbert 2014-11-24  7864  		int n, len, sb;
c2248fc974df7b Douglas Gilbert 2014-11-24  7865  
c2248fc974df7b Douglas Gilbert 2014-11-24  7866  		len = scp->cmd_len;
c2248fc974df7b Douglas Gilbert 2014-11-24  7867  		sb = (int)sizeof(b);
c2248fc974df7b Douglas Gilbert 2014-11-24  7868  		if (len > 32)
c2248fc974df7b Douglas Gilbert 2014-11-24  7869  			strcpy(b, "too long, over 32 bytes");
c2248fc974df7b Douglas Gilbert 2014-11-24  7870  		else {
c2248fc974df7b Douglas Gilbert 2014-11-24  7871  			for (k = 0, n = 0; k < len && n < sb; ++k)
c2248fc974df7b Douglas Gilbert 2014-11-24  7872  				n += scnprintf(b + n, sb - n, "%02x ",
c2248fc974df7b Douglas Gilbert 2014-11-24  7873  					       (u32)cmd[k]);
c2248fc974df7b Douglas Gilbert 2014-11-24  7874  		}
458df78b1c513d Bart Van Assche 2018-01-26  7875  		sdev_printk(KERN_INFO, sdp, "%s: tag=%#x, cmd %s\n", my_name,
a6e76e6f2c0efd Bart Van Assche 2021-08-09  7876  			    blk_mq_unique_tag(scsi_cmd_to_rq(scp)), b);
c2248fc974df7b Douglas Gilbert 2014-11-24  7877  	}
3a90a63d02b8b7 Douglas Gilbert 2020-07-12  7878  	if (unlikely(inject_now && (sdebug_opts & SDEBUG_OPT_HOST_BUSY)))
7ee6d1b4357ac2 Bart Van Assche 2017-12-07  7879  		return SCSI_MLQUEUE_HOST_BUSY;
34d55434ba1f39 Tomas Winkler   2015-07-28  7880  	has_wlun_rl = (sdp->lun == SCSI_W_LUN_REPORT_LUNS);
ad0c7775e745d2 Douglas Gilbert 2020-08-21  7881  	if (unlikely(lun_index >= sdebug_max_luns && !has_wlun_rl))
f46eb0e9fc763b Douglas Gilbert 2016-04-25  7882  		goto err_out;
c2248fc974df7b Douglas Gilbert 2014-11-24  7883  
c2248fc974df7b Douglas Gilbert 2014-11-24  7884  	sdeb_i = opcode_ind_arr[opcode];	/* fully mapped */
c2248fc974df7b Douglas Gilbert 2014-11-24  7885  	oip = &opcode_info_arr[sdeb_i];		/* safe if table consistent */
c2248fc974df7b Douglas Gilbert 2014-11-24  7886  	devip = (struct sdebug_dev_info *)sdp->hostdata;
f46eb0e9fc763b Douglas Gilbert 2016-04-25  7887  	if (unlikely(!devip)) {
f46eb0e9fc763b Douglas Gilbert 2016-04-25  7888  		devip = find_build_dev_info(sdp);
c2248fc974df7b Douglas Gilbert 2014-11-24  7889  		if (NULL == devip)
f46eb0e9fc763b Douglas Gilbert 2016-04-25  7890  			goto err_out;
c2248fc974df7b Douglas Gilbert 2014-11-24  7891  	}
3f07ff40cb2457 Wenchao Hao     2023-09-22  7892  
3f07ff40cb2457 Wenchao Hao     2023-09-22  7893  	if (sdebug_timeout_cmd(scp)) {
3f07ff40cb2457 Wenchao Hao     2023-09-22  7894  		scmd_printk(KERN_INFO, scp, "timeout command 0x%x\n", opcode);
3f07ff40cb2457 Wenchao Hao     2023-09-22  7895  		return 0;
3f07ff40cb2457 Wenchao Hao     2023-09-22  7896  	}
3f07ff40cb2457 Wenchao Hao     2023-09-22  7897  
929aad8ff0578d Wenchao Hao     2023-09-22  7898  	ret = sdebug_fail_queue_cmd(scp);
929aad8ff0578d Wenchao Hao     2023-09-22  7899  	if (ret) {
929aad8ff0578d Wenchao Hao     2023-09-22  7900  		scmd_printk(KERN_INFO, scp, "fail queue command 0x%x with 0x%x\n",
929aad8ff0578d Wenchao Hao     2023-09-22  7901  				opcode, ret);
929aad8ff0578d Wenchao Hao     2023-09-22  7902  		return ret;
929aad8ff0578d Wenchao Hao     2023-09-22  7903  	}
929aad8ff0578d Wenchao Hao     2023-09-22  7904  
cc36ffafc0f7e6 Wenchao Hao     2023-09-22  7905  	if (sdebug_fail_cmd(scp, &ret, &err)) {
cc36ffafc0f7e6 Wenchao Hao     2023-09-22  7906  		scmd_printk(KERN_INFO, scp,
cc36ffafc0f7e6 Wenchao Hao     2023-09-22  7907  			"fail command 0x%x with hostbyte=0x%x, "
cc36ffafc0f7e6 Wenchao Hao     2023-09-22  7908  			"driverbyte=0x%x, statusbyte=0x%x, "
cc36ffafc0f7e6 Wenchao Hao     2023-09-22  7909  			"sense_key=0x%x, asc=0x%x, asq=0x%x\n",
cc36ffafc0f7e6 Wenchao Hao     2023-09-22  7910  			opcode, err.host_byte, err.driver_byte,
cc36ffafc0f7e6 Wenchao Hao     2023-09-22  7911  			err.status_byte, err.sense_key, err.asc, err.asq);
cc36ffafc0f7e6 Wenchao Hao     2023-09-22 @7912  		return ret;

This returns zero but it should be an error code.

cc36ffafc0f7e6 Wenchao Hao     2023-09-22  7913  	}
cc36ffafc0f7e6 Wenchao Hao     2023-09-22  7914  
3a90a63d02b8b7 Douglas Gilbert 2020-07-12  7915  	if (unlikely(inject_now && !atomic_read(&sdeb_inject_pending)))
3a90a63d02b8b7 Douglas Gilbert 2020-07-12  7916  		atomic_set(&sdeb_inject_pending, 1);
3a90a63d02b8b7 Douglas Gilbert 2020-07-12  7917  
c2248fc974df7b Douglas Gilbert 2014-11-24  7918  	na = oip->num_attached;
c2248fc974df7b Douglas Gilbert 2014-11-24  7919  	r_pfp = oip->pfp;
c2248fc974df7b Douglas Gilbert 2014-11-24  7920  	if (na) {	/* multiple commands with this opcode */
c2248fc974df7b Douglas Gilbert 2014-11-24  7921  		r_oip = oip;

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v5 07/10] scsi: scsi_debug: Add new error injection abort failed

[Wenchao Hao]

Add error injection type 3 to make scsi_debug_abort() return FAILED.
Fail abort command foramt:

  +--------+------+-------------------------------------------------------+
  | Column | Type | Description                                           |
  +--------+------+-------------------------------------------------------+
  |   1    |  u8  | Error type, fixed to 0x3                              |
  +--------+------+-------------------------------------------------------+
  |   2    |  s32 | Error count                                           |
  |        |      |  0: this rule will be ignored                         |
  |        |      |  positive: the rule will always take effect           |
  |        |      |  negative: the rule takes effect n times where -n is  |
  |        |      |            the value given. Ignored after n times     |
  +--------+------+-------------------------------------------------------+
  |   3    |  x8  | SCSI command opcode, 0xff for all commands            |
  +--------+------+-------------------------------------------------------+

Examples:
    error=/sys/kernel/debug/scsi_debug/0:0:0:1/error
    echo "0 -10 0x12" > ${error}
will make the device return FAILED when abort inquiry command 10 times.

Signed-off-by: Wenchao Hao <haowenchao2@huawei.com>
---
 drivers/scsi/scsi_debug.c | 40 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index fe1f7421f617..8a16cb9642a6 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -293,6 +293,8 @@ enum sdebug_err_type {
 	ERR_FAIL_CMD		= 2,	/* make specific scsi command's */
 					/* queuecmd return succeed but */
 					/* with errors set in scsi_cmnd */
+	ERR_ABORT_CMD_FAILED	= 3,	/* control return FAILED from */
+					/* scsi_debug_abort() */
 };
 
 struct sdebug_err_inject {
@@ -970,6 +972,7 @@ static int sdebug_error_show(struct seq_file *m, void *p)
 	list_for_each_entry_rcu(err, &devip->inject_err_list, list) {
 		switch (err->type) {
 		case ERR_TMOUT_CMD:
+		case ERR_ABORT_CMD_FAILED:
 			seq_printf(m, "%d\t%d\t0x%x\n", err->type, err->cnt,
 				err->cmd);
 		break;
@@ -1031,6 +1034,7 @@ static ssize_t sdebug_error_write(struct file *file, const char __user *ubuf,
 
 	switch (inject_type) {
 	case ERR_TMOUT_CMD:
+	case ERR_ABORT_CMD_FAILED:
 		if (sscanf(buf, "%d %d %hhx", &inject->type, &inject->cnt,
 			   &inject->cmd) != 3)
 			goto out_error;
@@ -5504,9 +5508,39 @@ static void stop_all_queued(void)
 	mutex_unlock(&sdebug_host_list_mutex);
 }
 
+static int sdebug_fail_abort(struct scsi_cmnd *cmnd)
+{
+	struct scsi_device *sdp = cmnd->device;
+	struct sdebug_dev_info *devip = (struct sdebug_dev_info *)sdp->hostdata;
+	struct sdebug_err_inject *err;
+	unsigned char *cmd = cmnd->cmnd;
+	int ret = 0;
+
+	if (devip == NULL)
+		return 0;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(err, &devip->inject_err_list, list) {
+		if (err->type == ERR_ABORT_CMD_FAILED &&
+		    (err->cmd == cmd[0] || err->cmd == 0xff)) {
+			ret = !!err->cnt;
+			if (err->cnt < 0)
+				err->cnt++;
+
+			rcu_read_unlock();
+			return ret;
+		}
+	}
+	rcu_read_unlock();
+
+	return 0;
+}
+
 static int scsi_debug_abort(struct scsi_cmnd *SCpnt)
 {
 	bool ok = scsi_debug_abort_cmnd(SCpnt);
+	u8 *cmd = SCpnt->cmnd;
+	u8 opcode = cmd[0];
 
 	++num_aborts;
 
@@ -5515,6 +5549,12 @@ static int scsi_debug_abort(struct scsi_cmnd *SCpnt)
 			    "%s: command%s found\n", __func__,
 			    ok ? "" : " not");
 
+	if (sdebug_fail_abort(SCpnt)) {
+		scmd_printk(KERN_INFO, SCpnt, "fail abort command 0x%x\n",
+			    opcode);
+		return FAILED;
+	}
+
 	return SUCCESS;
 }
 
-- 
2.32.0

Re: [PATCH v5 07/10] scsi: scsi_debug: Add new error injection abort failed

[Douglas Gilbert]

On 2023-09-22 05:29, Wenchao Hao wrote:
> Add error injection type 3 to make scsi_debug_abort() return FAILED.
> Fail abort command foramt:

s/foramt/format/

> 
>    +--------+------+-------------------------------------------------------+
>    | Column | Type | Description                                           |
>    +--------+------+-------------------------------------------------------+
>    |   1    |  u8  | Error type, fixed to 0x3                              |
>    +--------+------+-------------------------------------------------------+
>    |   2    |  s32 | Error count                                           |
>    |        |      |  0: this rule will be ignored                         |
>    |        |      |  positive: the rule will always take effect           |
>    |        |      |  negative: the rule takes effect n times where -n is  |
>    |        |      |            the value given. Ignored after n times     |
>    +--------+------+-------------------------------------------------------+
>    |   3    |  x8  | SCSI command opcode, 0xff for all commands            |
>    +--------+------+-------------------------------------------------------+
> 
> Examples:
>      error=/sys/kernel/debug/scsi_debug/0:0:0:1/error
>      echo "0 -10 0x12" > ${error}
> will make the device return FAILED when abort inquiry command 10 times.

Tested with:
    # sg_raw -t 10 -r 1k /dev/sg1 12 00 00 00 60 00

After 10 seconds (the timeout specified to sg_raw) I saw this:
    >>> transport error: Host_status=0x03 [DID_TIME_OUT]

And the
     # cat /sys/kernel/debug/scsi_debug/1\:0\:0\:0/error

Count value changed from -10 up to 0 for each invocation of the INQUIRY
command. Thereafter the INQUIRY command worked.

Looks good.

   Tested-by: Douglas Gilbert <dgilbert@interlog.com>

> 
> Signed-off-by: Wenchao Hao <haowenchao2@huawei.com>
> ---
>   drivers/scsi/scsi_debug.c | 40 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 40 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index fe1f7421f617..8a16cb9642a6 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -293,6 +293,8 @@ enum sdebug_err_type {
>   	ERR_FAIL_CMD		= 2,	/* make specific scsi command's */
>   					/* queuecmd return succeed but */
>   					/* with errors set in scsi_cmnd */
> +	ERR_ABORT_CMD_FAILED	= 3,	/* control return FAILED from */
> +					/* scsi_debug_abort() */
>   };
>   
>   struct sdebug_err_inject {
> @@ -970,6 +972,7 @@ static int sdebug_error_show(struct seq_file *m, void *p)
>   	list_for_each_entry_rcu(err, &devip->inject_err_list, list) {
>   		switch (err->type) {
>   		case ERR_TMOUT_CMD:
> +		case ERR_ABORT_CMD_FAILED:
>   			seq_printf(m, "%d\t%d\t0x%x\n", err->type, err->cnt,
>   				err->cmd);
>   		break;
> @@ -1031,6 +1034,7 @@ static ssize_t sdebug_error_write(struct file *file, const char __user *ubuf,
>   
>   	switch (inject_type) {
>   	case ERR_TMOUT_CMD:
> +	case ERR_ABORT_CMD_FAILED:
>   		if (sscanf(buf, "%d %d %hhx", &inject->type, &inject->cnt,
>   			   &inject->cmd) != 3)
>   			goto out_error;
> @@ -5504,9 +5508,39 @@ static void stop_all_queued(void)
>   	mutex_unlock(&sdebug_host_list_mutex);
>   }
>   
> +static int sdebug_fail_abort(struct scsi_cmnd *cmnd)
> +{
> +	struct scsi_device *sdp = cmnd->device;
> +	struct sdebug_dev_info *devip = (struct sdebug_dev_info *)sdp->hostdata;
> +	struct sdebug_err_inject *err;
> +	unsigned char *cmd = cmnd->cmnd;
> +	int ret = 0;
> +
> +	if (devip == NULL)
> +		return 0;
> +
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(err, &devip->inject_err_list, list) {
> +		if (err->type == ERR_ABORT_CMD_FAILED &&
> +		    (err->cmd == cmd[0] || err->cmd == 0xff)) {
> +			ret = !!err->cnt;
> +			if (err->cnt < 0)
> +				err->cnt++;
> +
> +			rcu_read_unlock();
> +			return ret;
> +		}
> +	}
> +	rcu_read_unlock();
> +
> +	return 0;
> +}
> +
>   static int scsi_debug_abort(struct scsi_cmnd *SCpnt)
>   {
>   	bool ok = scsi_debug_abort_cmnd(SCpnt);
> +	u8 *cmd = SCpnt->cmnd;
> +	u8 opcode = cmd[0];
>   
>   	++num_aborts;
>   
> @@ -5515,6 +5549,12 @@ static int scsi_debug_abort(struct scsi_cmnd *SCpnt)
>   			    "%s: command%s found\n", __func__,
>   			    ok ? "" : " not");
>   
> +	if (sdebug_fail_abort(SCpnt)) {
> +		scmd_printk(KERN_INFO, SCpnt, "fail abort command 0x%x\n",
> +			    opcode);
> +		return FAILED;
> +	}
> +
>   	return SUCCESS;
>   }
>   

[PATCH v5 08/10] scsi: scsi_debug: Add new error injection reset lun failed

[Wenchao Hao]

Add error injection type 3 to make scsi_debug_device_reset() return FAILED.
Fail abort command foramt:

  +--------+------+-------------------------------------------------------+
  | Column | Type | Description                                           |
  +--------+------+-------------------------------------------------------+
  |   1    |  u8  | Error type, fixed to 0x4                              |
  +--------+------+-------------------------------------------------------+
  |   2    |  s32 | Error count                                           |
  |        |      |  0: this rule will be ignored                         |
  |        |      |  positive: the rule will always take effect           |
  |        |      |  negative: the rule takes effect n times where -n is  |
  |        |      |            the value given. Ignored after n times     |
  +--------+------+-------------------------------------------------------+
  |   3    |  x8  | SCSI command opcode, 0xff for all commands            |
  +--------+------+-------------------------------------------------------+

Examples:
    error=/sys/kernel/debug/scsi_debug/0:0:0:1/error
    echo "0 -10 0x12" > ${error}
will make the device return FAILED when try to reset lun with inquiry
command 10 times.
    error=/sys/kernel/debug/scsi_debug/0:0:0:1/error
    echo "0 -10 0xff" > ${error}
will make the device return FAILED when try to reset lun 10 times.

Usually we do not care about what command it is when trying to perform
reset LUN, so 0xff could be applied.

Signed-off-by: Wenchao Hao <haowenchao2@huawei.com>
---
 drivers/scsi/scsi_debug.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 8a16cb9642a6..db8ab6cad078 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -295,6 +295,8 @@ enum sdebug_err_type {
 					/* with errors set in scsi_cmnd */
 	ERR_ABORT_CMD_FAILED	= 3,	/* control return FAILED from */
 					/* scsi_debug_abort() */
+	ERR_LUN_RESET_FAILED	= 4,	/* control return FAILED from */
+					/* scsi_debug_device_reseLUN_RESET_FAILEDt() */
 };
 
 struct sdebug_err_inject {
@@ -973,6 +975,7 @@ static int sdebug_error_show(struct seq_file *m, void *p)
 		switch (err->type) {
 		case ERR_TMOUT_CMD:
 		case ERR_ABORT_CMD_FAILED:
+		case ERR_LUN_RESET_FAILED:
 			seq_printf(m, "%d\t%d\t0x%x\n", err->type, err->cnt,
 				err->cmd);
 		break;
@@ -1035,6 +1038,7 @@ static ssize_t sdebug_error_write(struct file *file, const char __user *ubuf,
 	switch (inject_type) {
 	case ERR_TMOUT_CMD:
 	case ERR_ABORT_CMD_FAILED:
+	case ERR_LUN_RESET_FAILED:
 		if (sscanf(buf, "%d %d %hhx", &inject->type, &inject->cnt,
 			   &inject->cmd) != 3)
 			goto out_error;
@@ -5578,10 +5582,40 @@ static void scsi_debug_stop_all_queued(struct scsi_device *sdp)
 				scsi_debug_stop_all_queued_iter, sdp);
 }
 
+static int sdebug_fail_lun_reset(struct scsi_cmnd *cmnd)
+{
+	struct scsi_device *sdp = cmnd->device;
+	struct sdebug_dev_info *devip = (struct sdebug_dev_info *)sdp->hostdata;
+	struct sdebug_err_inject *err;
+	unsigned char *cmd = cmnd->cmnd;
+	int ret = 0;
+
+	if (devip == NULL)
+		return 0;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(err, &devip->inject_err_list, list) {
+		if (err->type == ERR_LUN_RESET_FAILED &&
+		    (err->cmd == cmd[0] || err->cmd == 0xff)) {
+			ret = !!err->cnt;
+			if (err->cnt < 0)
+				err->cnt++;
+
+			rcu_read_unlock();
+			return ret;
+		}
+	}
+	rcu_read_unlock();
+
+	return 0;
+}
+
 static int scsi_debug_device_reset(struct scsi_cmnd *SCpnt)
 {
 	struct scsi_device *sdp = SCpnt->device;
 	struct sdebug_dev_info *devip = sdp->hostdata;
+	u8 *cmd = SCpnt->cmnd;
+	u8 opcode = cmd[0];
 
 	++num_dev_resets;
 
@@ -5592,6 +5626,11 @@ static int scsi_debug_device_reset(struct scsi_cmnd *SCpnt)
 	if (devip)
 		set_bit(SDEBUG_UA_POR, devip->uas_bm);
 
+	if (sdebug_fail_lun_reset(SCpnt)) {
+		scmd_printk(KERN_INFO, SCpnt, "fail lun reset 0x%x\n", opcode);
+		return FAILED;
+	}
+
 	return SUCCESS;
 }
 
-- 
2.32.0

Re: [PATCH v5 08/10] scsi: scsi_debug: Add new error injection reset lun failed

[Douglas Gilbert]

On 2023-09-22 05:29, Wenchao Hao wrote:
> Add error injection type 3 to make scsi_debug_device_reset() return FAILED.
> Fail abort command foramt:

s/foramt/format/

> 
>    +--------+------+-------------------------------------------------------+
>    | Column | Type | Description                                           |
>    +--------+------+-------------------------------------------------------+
>    |   1    |  u8  | Error type, fixed to 0x4                              |
>    +--------+------+-------------------------------------------------------+
>    |   2    |  s32 | Error count                                           |
>    |        |      |  0: this rule will be ignored                         |
>    |        |      |  positive: the rule will always take effect           |
>    |        |      |  negative: the rule takes effect n times where -n is  |
>    |        |      |            the value given. Ignored after n times     |
>    +--------+------+-------------------------------------------------------+
>    |   3    |  x8  | SCSI command opcode, 0xff for all commands            |
>    +--------+------+-------------------------------------------------------+
> 
> Examples:
>      error=/sys/kernel/debug/scsi_debug/0:0:0:1/error
>      echo "0 -10 0x12" > ${error}

These examples are misleading. Same with the one in patch 7/10 . The example
should be showing an invocation that exercises _this_ patch. So the first
byte of the echo should be 4 not the 0 shown above.

Doug Gilbert

> will make the device return FAILED when try to reset lun with inquiry
> command 10 times.
>      error=/sys/kernel/debug/scsi_debug/0:0:0:1/error
>      echo "0 -10 0xff" > ${error}
> will make the device return FAILED when try to reset lun 10 times.
> 
> Usually we do not care about what command it is when trying to perform
> reset LUN, so 0xff could be applied.
> 
> Signed-off-by: Wenchao Hao <haowenchao2@huawei.com>
> ---
>   drivers/scsi/scsi_debug.c | 39 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 39 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 8a16cb9642a6..db8ab6cad078 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -295,6 +295,8 @@ enum sdebug_err_type {
>   					/* with errors set in scsi_cmnd */
>   	ERR_ABORT_CMD_FAILED	= 3,	/* control return FAILED from */
>   					/* scsi_debug_abort() */
> +	ERR_LUN_RESET_FAILED	= 4,	/* control return FAILED from */
> +					/* scsi_debug_device_reseLUN_RESET_FAILEDt() */
>   };
>   
>   struct sdebug_err_inject {
> @@ -973,6 +975,7 @@ static int sdebug_error_show(struct seq_file *m, void *p)
>   		switch (err->type) {
>   		case ERR_TMOUT_CMD:
>   		case ERR_ABORT_CMD_FAILED:
> +		case ERR_LUN_RESET_FAILED:
>   			seq_printf(m, "%d\t%d\t0x%x\n", err->type, err->cnt,
>   				err->cmd);
>   		break;
> @@ -1035,6 +1038,7 @@ static ssize_t sdebug_error_write(struct file *file, const char __user *ubuf,
>   	switch (inject_type) {
>   	case ERR_TMOUT_CMD:
>   	case ERR_ABORT_CMD_FAILED:
> +	case ERR_LUN_RESET_FAILED:
>   		if (sscanf(buf, "%d %d %hhx", &inject->type, &inject->cnt,
>   			   &inject->cmd) != 3)
>   			goto out_error;
> @@ -5578,10 +5582,40 @@ static void scsi_debug_stop_all_queued(struct scsi_device *sdp)
>   				scsi_debug_stop_all_queued_iter, sdp);
>   }
>   
> +static int sdebug_fail_lun_reset(struct scsi_cmnd *cmnd)
> +{
> +	struct scsi_device *sdp = cmnd->device;
> +	struct sdebug_dev_info *devip = (struct sdebug_dev_info *)sdp->hostdata;
> +	struct sdebug_err_inject *err;
> +	unsigned char *cmd = cmnd->cmnd;
> +	int ret = 0;
> +
> +	if (devip == NULL)
> +		return 0;
> +
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(err, &devip->inject_err_list, list) {
> +		if (err->type == ERR_LUN_RESET_FAILED &&
> +		    (err->cmd == cmd[0] || err->cmd == 0xff)) {
> +			ret = !!err->cnt;
> +			if (err->cnt < 0)
> +				err->cnt++;
> +
> +			rcu_read_unlock();
> +			return ret;
> +		}
> +	}
> +	rcu_read_unlock();
> +
> +	return 0;
> +}
> +
>   static int scsi_debug_device_reset(struct scsi_cmnd *SCpnt)
>   {
>   	struct scsi_device *sdp = SCpnt->device;
>   	struct sdebug_dev_info *devip = sdp->hostdata;
> +	u8 *cmd = SCpnt->cmnd;
> +	u8 opcode = cmd[0];
>   
>   	++num_dev_resets;
>   
> @@ -5592,6 +5626,11 @@ static int scsi_debug_device_reset(struct scsi_cmnd *SCpnt)
>   	if (devip)
>   		set_bit(SDEBUG_UA_POR, devip->uas_bm);
>   
> +	if (sdebug_fail_lun_reset(SCpnt)) {
> +		scmd_printk(KERN_INFO, SCpnt, "fail lun reset 0x%x\n", opcode);
> +		return FAILED;
> +	}
> +
>   	return SUCCESS;
>   }
>   

Re: [PATCH v5 08/10] scsi: scsi_debug: Add new error injection reset lun failed

[Wenchao Hao]

On 2023/10/7 5:04, Douglas Gilbert wrote:
> On 2023-09-22 05:29, Wenchao Hao wrote:
>> Add error injection type 3 to make scsi_debug_device_reset() return FAILED.
>> Fail abort command foramt:
> 
> s/foramt/format/
> 
>>
>> Examples:
>>      error=/sys/kernel/debug/scsi_debug/0:0:0:1/error
>>      echo "0 -10 0x12" > ${error}
> 
> These examples are misleading. Same with the one in patch 7/10 . The example
> should be showing an invocation that exercises _this_ patch. So the first
> byte of the echo should be 4 not the 0 shown above.
> 
> Doug Gilbert
> 

Would update in next version. Would you continue reviewing patch 9/10 and 10/10?

[PATCH v5 09/10] scsi: scsi_debug: Add debugfs interface to fail target reset

[Wenchao Hao]

The interface is found at
/sys/kernel/debug/scsi_debug/target<h:c:t>/fail_reset where <h:c:t>
identifies the target to inject errors on. It's a simple bool type
interface which would make this target's reset fail if set to 'Y'.

Signed-off-by: Wenchao Hao <haowenchao2@huawei.com>
Reported-by: kernel test robot <oliver.sang@intel.com>
---
 drivers/scsi/scsi_debug.c | 106 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 105 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index db8ab6cad078..ab4a6f7de1ef 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -42,6 +42,7 @@
 #include <linux/xarray.h>
 #include <linux/prefetch.h>
 #include <linux/debugfs.h>
+#include <linux/async.h>
 
 #include <net/checksum.h>
 
@@ -357,6 +358,11 @@ struct sdebug_dev_info {
 	struct list_head inject_err_list;
 };
 
+struct sdebug_target_info {
+	bool reset_fail;
+	struct dentry *debugfs_entry;
+};
+
 struct sdebug_host_info {
 	struct list_head host_list;
 	int si_idx;	/* sdeb_store_info (per host) xarray index */
@@ -1082,6 +1088,83 @@ static const struct file_operations sdebug_error_fops = {
 	.release = single_release,
 };
 
+static int sdebug_target_reset_fail_show(struct seq_file *m, void *p)
+{
+	struct scsi_target *starget = (struct scsi_target *)m->private;
+	struct sdebug_target_info *targetip =
+		(struct sdebug_target_info *)starget->hostdata;
+
+	if (targetip)
+		seq_printf(m, "%c\n", targetip->reset_fail ? 'Y' : 'N');
+
+	return 0;
+}
+
+static int sdebug_target_reset_fail_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, sdebug_target_reset_fail_show, inode->i_private);
+}
+
+static ssize_t sdebug_target_reset_fail_write(struct file *file,
+		const char __user *ubuf, size_t count, loff_t *ppos)
+{
+	int ret;
+	struct scsi_target *starget =
+		(struct scsi_target *)file->f_inode->i_private;
+	struct sdebug_target_info *targetip =
+		(struct sdebug_target_info *)starget->hostdata;
+
+	if (targetip) {
+		ret = kstrtobool_from_user(ubuf, count, &targetip->reset_fail);
+		return ret < 0 ? ret : count;
+	}
+	return -ENODEV;
+}
+
+static const struct file_operations sdebug_target_reset_fail_fops = {
+	.open	= sdebug_target_reset_fail_open,
+	.read	= seq_read,
+	.write	= sdebug_target_reset_fail_write,
+	.release = single_release,
+};
+
+static int sdebug_target_alloc(struct scsi_target *starget)
+{
+	struct sdebug_target_info *targetip;
+
+	targetip = kzalloc(sizeof(struct sdebug_target_info), GFP_KERNEL);
+	if (!targetip)
+		return -ENOMEM;
+
+	targetip->debugfs_entry = debugfs_create_dir(dev_name(&starget->dev),
+				sdebug_debugfs_root);
+	debugfs_create_file("fail_reset", 0600, targetip->debugfs_entry, starget,
+				&sdebug_target_reset_fail_fops);
+
+	starget->hostdata = targetip;
+
+	return 0;
+}
+
+static void sdebug_tartget_cleanup_async(void *data, async_cookie_t cookie)
+{
+	struct sdebug_target_info *targetip = data;
+
+	debugfs_remove(targetip->debugfs_entry);
+	kfree(targetip);
+}
+
+static void sdebug_target_destroy(struct scsi_target *starget)
+{
+	struct sdebug_target_info *targetip;
+
+	targetip = (struct sdebug_target_info *)starget->hostdata;
+	if (targetip) {
+		starget->hostdata = NULL;
+		async_schedule(sdebug_tartget_cleanup_async, targetip);
+	}
+}
+
 /* 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).
@@ -5634,11 +5717,25 @@ static int scsi_debug_device_reset(struct scsi_cmnd *SCpnt)
 	return SUCCESS;
 }
 
+static int sdebug_fail_target_reset(struct scsi_cmnd *cmnd)
+{
+	struct scsi_target *starget = scsi_target(cmnd->device);
+	struct sdebug_target_info *targetip =
+		(struct sdebug_target_info *)starget->hostdata;
+
+	if (targetip)
+		return targetip->reset_fail;
+
+	return 0;
+}
+
 static int scsi_debug_target_reset(struct scsi_cmnd *SCpnt)
 {
 	struct scsi_device *sdp = SCpnt->device;
 	struct sdebug_host_info *sdbg_host = shost_to_sdebug_host(sdp->host);
 	struct sdebug_dev_info *devip;
+	u8 *cmd = SCpnt->cmnd;
+	u8 opcode = cmd[0];
 	int k = 0;
 
 	++num_target_resets;
@@ -5656,6 +5753,12 @@ static int scsi_debug_target_reset(struct scsi_cmnd *SCpnt)
 		sdev_printk(KERN_INFO, sdp,
 			    "%s: %d device(s) found in target\n", __func__, k);
 
+	if (sdebug_fail_target_reset(SCpnt)) {
+		scmd_printk(KERN_INFO, SCpnt, "fail target reset 0x%x\n",
+			    opcode);
+		return FAILED;
+	}
+
 	return SUCCESS;
 }
 
@@ -8109,7 +8212,6 @@ static int sdebug_init_cmd_priv(struct Scsi_Host *shost, struct scsi_cmnd *cmd)
 	return 0;
 }
 
-
 static struct scsi_host_template sdebug_driver_template = {
 	.show_info =		scsi_debug_show_info,
 	.write_info =		scsi_debug_write_info,
@@ -8139,6 +8241,8 @@ static struct scsi_host_template sdebug_driver_template = {
 	.track_queue_depth =	1,
 	.cmd_size = sizeof(struct sdebug_scsi_cmd),
 	.init_cmd_priv = sdebug_init_cmd_priv,
+	.target_alloc =		sdebug_target_alloc,
+	.target_destroy =	sdebug_target_destroy,
 };
 
 static int sdebug_driver_probe(struct device *dev)
-- 
2.32.0

Re: [PATCH v5 09/10] scsi: scsi_debug: Add debugfs interface to fail target reset

[Douglas Gilbert]

On 2023-09-22 05:29, Wenchao Hao wrote:
> The interface is found at
> /sys/kernel/debug/scsi_debug/target<h:c:t>/fail_reset where <h:c:t>
> identifies the target to inject errors on. It's a simple bool type
> interface which would make this target's reset fail if set to 'Y'.
> 
> Signed-off-by: Wenchao Hao <haowenchao2@huawei.com>
> Reported-by: kernel test robot <oliver.sang@intel.com>

Tested by setting 'echo 1 > /sys/bus/pseudo/drivers/scsi_debug/opts'
and observing 'tail -f /var/log/syslog'. Looks good including that
fail_reset is readable so its current state can be checked.

Tested-by: Douglas Gilbert <dgilbert@interlog.com>

<snip>

[PATCH v5 10/10] scsi: scsi_debug: Add param to control sdev's allow_restart

[Wenchao Hao]

Add new module param "allow_restart" to control if setup
scsi_device's allow_restart flag. This is used to test scsi
command finished with sense_key 0x6, asc 0x4 and ascq 0x2

Signed-off-by: Wenchao Hao <haowenchao2@huawei.com>
---
 drivers/scsi/scsi_debug.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index ab4a6f7de1ef..52a9ddea57d3 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -843,6 +843,7 @@ static bool have_dif_prot;
 static bool write_since_sync;
 static bool sdebug_statistics = DEF_STATISTICS;
 static bool sdebug_wp;
+static bool sdebug_allow_restart;
 /* Following enum: 0: no zbc, def; 1: host aware; 2: host managed */
 static enum blk_zoned_model sdeb_zbc_model = BLK_ZONED_NONE;
 static char *sdeb_zbc_model_s;
@@ -5469,6 +5470,9 @@ static int scsi_debug_slave_configure(struct scsi_device *sdp)
 		sdp->no_uld_attach = 1;
 	config_cdb_len(sdp);
 
+	if (sdebug_allow_restart)
+		sdp->allow_restart = 1;
+
 	devip->debugfs_entry = debugfs_create_dir(dev_name(&sdp->sdev_dev),
 				sdebug_debugfs_root);
 	debugfs_create_file("error", 0600, devip->debugfs_entry, sdp,
@@ -6186,6 +6190,7 @@ module_param_named(zone_cap_mb, sdeb_zbc_zone_cap_mb, int, S_IRUGO);
 module_param_named(zone_max_open, sdeb_zbc_max_open, int, S_IRUGO);
 module_param_named(zone_nr_conv, sdeb_zbc_nr_conv, int, S_IRUGO);
 module_param_named(zone_size_mb, sdeb_zbc_zone_size_mb, int, S_IRUGO);
+module_param_named(allow_restart, sdebug_allow_restart, bool, S_IRUGO | S_IWUSR);
 
 MODULE_AUTHOR("Eric Youngdale + Douglas Gilbert");
 MODULE_DESCRIPTION("SCSI debug adapter driver");
@@ -6258,6 +6263,7 @@ MODULE_PARM_DESC(zone_cap_mb, "Zone capacity in MiB (def=zone size)");
 MODULE_PARM_DESC(zone_max_open, "Maximum number of open zones; [0] for no limit (def=auto)");
 MODULE_PARM_DESC(zone_nr_conv, "Number of conventional zones (def=1)");
 MODULE_PARM_DESC(zone_size_mb, "Zone size in MiB (def=auto)");
+MODULE_PARM_DESC(allow_restart, "Set scsi_device's allow_restart flag(def=0)");
 
 #define SDEBUG_INFO_LEN 256
 static char sdebug_info[SDEBUG_INFO_LEN];
-- 
2.32.0

Re: [PATCH v5 10/10] scsi: scsi_debug: Add param to control sdev's allow_restart

[Douglas Gilbert]

On 2023-09-22 05:29, Wenchao Hao wrote:
> Add new module param "allow_restart" to control if setup
> scsi_device's allow_restart flag. This is used to test scsi
> command finished with sense_key 0x6, asc 0x4 and ascq 0x2
> 
> Signed-off-by: Wenchao Hao <haowenchao2@huawei.com>

Hi,
Looked at this and verified that the allow_restart flag of scsi_debug
devices (disks ?) is usually 0 and when the scsi_debug module is
started with allow_restart=1 then the allow_restart flag does indeed
change to 1. For example:
    # cat /sys/class/scsi_disk/1\:0\:0\:0/allow_restart
    1

That ASC/ASCQ code means: "Logical unit not ready, initializing command
required" according to my library. Played around with sg_start but didn't
see any change in how it reacts. According to scsi_device.h that flag's
description is: "issue START_UNIT in error handler" which implies it
changes how the EH handler reacts.

Perhaps the 3 line patch description could say a little more about how
to use this new parameter...

Tested-by: Douglas Gilbert <dgilbert@interlog.com>

<snip>

Re: [PATCH v5 10/10] scsi: scsi_debug: Add param to control sdev's allow_restart

[Wenchao Hao]

On 2023/10/9 7:17, Douglas Gilbert wrote:
> On 2023-09-22 05:29, Wenchao Hao wrote:
>> Add new module param "allow_restart" to control if setup
>> scsi_device's allow_restart flag. This is used to test scsi
>> command finished with sense_key 0x6, asc 0x4 and ascq 0x2
>>
>> Signed-off-by: Wenchao Hao <haowenchao2@huawei.com>
> 
> Hi,
> Looked at this and verified that the allow_restart flag of scsi_debug
> devices (disks ?) is usually 0 and when the scsi_debug module is
> started with allow_restart=1 then the allow_restart flag does indeed
> change to 1. For example:
>     # cat /sys/class/scsi_disk/1\:0\:0\:0/allow_restart
>     1
> 
> That ASC/ASCQ code means: "Logical unit not ready, initializing command
> required" according to my library. Played around with sg_start but didn't
> see any change in how it reacts. According to scsi_device.h that flag's
> description is: "issue START_UNIT in error handler" which implies it
> changes how the EH handler reacts.
> 
> Perhaps the 3 line patch description could say a little more about how
> to use this new parameter...

Sorry I did not write in detail. As you mentioned above, this is to
determine if to trigger error. I would update the commit message to
following lines:

Add new module param "allow_restart" to control if setup scsi_device's
allow_restart flag, this flag determines if trigger EH after command
finished with sense_key 0x6, asc 0x4 and ascq 0x2, EH would be triggered
if allow_restart=1 in this condition.

The new param can be used with error inject added in patch6 to test how
commands finished with sense_key 0x6, asc 0x4 and ascq 0x2 are handled.

> 
> Tested-by: Douglas Gilbert <dgilbert@interlog.com>
> 
> <snip>
> 
> 
> 
> 
> 

Re: [PATCH v5 00/10] scsi:scsi_debug: Add error injection for single device

[Martin K. Petersen]

Doug,

> The original error injection mechanism was based on scsi_host which
> could not inject fault for a single SCSI device.
>
> This patchset provides the ability to inject errors for a single SCSI
> device. Now we supports inject timeout errors, queuecommand errors,
> and hostbyte, driverbyte, statusbyte, and sense data for specific SCSI
> Command. Two new error injection is defined to make abort command or
> reset LUN failed.

Please review patches 7 through 10. Thank you!

-- 
Martin K. Petersen	Oracle Linux Engineering