Re: [PATCH] scsi: core: Report error list information in debugfs

[John Garry <john.g.garry@oracle.com>]

On 21/08/2023 21:41, Bart Van Assche wrote:
> Provide information in debugfs about SCSI error handling to make it
> easier to debug the SCSI error handler. Additionally, report the maximum
> number of retries in debugfs (.allowed).
> 
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> Cc: Mike Christie <michael.christie@oracle.com>
> Cc: John Garry <john.g.garry@oracle.com>
> Cc: Ming Lei <ming.lei@redhat.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>   drivers/scsi/scsi_debugfs.c | 25 ++++++++++++++++++++++---
>   1 file changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_debugfs.c b/drivers/scsi/scsi_debugfs.c
> index 217b70c678c3..a9bc5f7ce745 100644
> --- a/drivers/scsi/scsi_debugfs.c
> +++ b/drivers/scsi/scsi_debugfs.c
> @@ -3,6 +3,7 @@
>   #include <linux/seq_file.h>
>   #include <scsi/scsi_cmnd.h>
>   #include <scsi/scsi_dbg.h>
> +#include <scsi/scsi_host.h>
>   #include "scsi_debugfs.h"
>   
>   #define SCSI_CMD_FLAG_NAME(name)[const_ilog2(SCMD_##name)] = #name
> @@ -33,14 +34,32 @@ static int scsi_flags_show(struct seq_file *m, const unsigned long flags,
>   
>   void scsi_show_rq(struct seq_file *m, struct request *rq)
>   {
> -	struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
> +	struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq), *cmd2;
> +	struct Scsi_Host *shost = cmd->device->host;
>   	int alloc_ms = jiffies_to_msecs(jiffies - cmd->jiffies_at_alloc);
>   	int timeout_ms = jiffies_to_msecs(rq->timeout);
> +	const char *list_info = NULL;
>   	char buf[80] = "(?)";
>   
> +	spin_lock_irq(shost->host_lock);
> +	list_for_each_entry(cmd2, &shost->eh_abort_list, eh_entry) {
> +		if (cmd == cmd2) {
> +			list_info = "on eh_abort_list";
> +			break;
> +		}
> +	}
> +	list_for_each_entry(cmd2, &shost->eh_cmd_q, eh_entry) {

If it's on the first list, then there is not much point in checking this 
list. It might be even worth checking list_empty(&cmd->eh_entry) 
initially also to save the bother.

Having said all this, adding those checks will add lots of unpleasant 
indentation...

> +		if (cmd == cmd2) {
> +			list_info = "on eh_cmd_q";
> +			break;
> +		}
> +	}
> +	spin_unlock_irq(shost->host_lock);
> +
>   	__scsi_format_command(buf, sizeof(buf), cmd->cmnd, cmd->cmd_len);
> -	seq_printf(m, ", .cmd=%s, .retries=%d, .result = %#x, .flags=", buf,
> -		   cmd->retries, cmd->result);
> +	seq_printf(m, ", .cmd=%s, .retries=%d, .allowed=%d, .result = %#x, %s%s.flags=",
> +		   buf, cmd->retries, cmd->allowed, cmd->result,
> +		   list_info ? : "", list_info ? ", " : "");
>   	scsi_flags_show(m, cmd->flags, scsi_cmd_flags,
>   			ARRAY_SIZE(scsi_cmd_flags));
>   	seq_printf(m, ", .timeout=%d.%03d, allocated %d.%03d s ago",