* [PATCH] scsi: core: Report error list information in debugfs
@ 2023-08-21 20:41 Bart Van Assche
2023-08-22 5:52 ` Damien Le Moal
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Bart Van Assche @ 2023-08-21 20:41 UTC (permalink / raw)
To: Martin K . Petersen
Cc: linux-scsi, Bart Van Assche, Hannes Reinecke, Damien Le Moal,
Mike Christie, John Garry, Ming Lei, James E.J. Bottomley
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 (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",
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] scsi: core: Report error list information in debugfs
2023-08-21 20:41 [PATCH] scsi: core: Report error list information in debugfs Bart Van Assche
@ 2023-08-22 5:52 ` Damien Le Moal
2023-08-22 9:04 ` John Garry
2023-10-09 7:12 ` Hannes Reinecke
2 siblings, 0 replies; 6+ messages in thread
From: Damien Le Moal @ 2023-08-22 5:52 UTC (permalink / raw)
To: Bart Van Assche, Martin K . Petersen
Cc: linux-scsi, Hannes Reinecke, Damien Le Moal, Mike Christie,
John Garry, Ming Lei, James E.J. Bottomley
On 8/22/23 05: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>
Looks OK to me.
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] scsi: core: Report error list information in debugfs
2023-08-21 20:41 [PATCH] scsi: core: Report error list information in debugfs Bart Van Assche
2023-08-22 5:52 ` Damien Le Moal
@ 2023-08-22 9:04 ` John Garry
2023-08-22 15:25 ` Bart Van Assche
2023-10-09 7:12 ` Hannes Reinecke
2 siblings, 1 reply; 6+ messages in thread
From: John Garry @ 2023-08-22 9:04 UTC (permalink / raw)
To: Bart Van Assche, Martin K . Petersen
Cc: linux-scsi, Hannes Reinecke, Damien Le Moal, Mike Christie,
Ming Lei, James E.J. Bottomley
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",
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] scsi: core: Report error list information in debugfs
2023-08-22 9:04 ` John Garry
@ 2023-08-22 15:25 ` Bart Van Assche
2023-08-22 16:24 ` John Garry
0 siblings, 1 reply; 6+ messages in thread
From: Bart Van Assche @ 2023-08-22 15:25 UTC (permalink / raw)
To: John Garry, Martin K . Petersen
Cc: linux-scsi, Hannes Reinecke, Damien Le Moal, Mike Christie,
Ming Lei, James E.J. Bottomley
On 8/22/23 02:04, John Garry wrote:
> 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...
Since the above code is only used to export information through debugfs I don't
think that it should be optimized heavily? Anyway, how about combining the
(untested) patch below with the above patch?
diff --git a/drivers/scsi/scsi_debugfs.c b/drivers/scsi/scsi_debugfs.c
index a9bc5f7ce745..f795848b316c 100644
--- a/drivers/scsi/scsi_debugfs.c
+++ b/drivers/scsi/scsi_debugfs.c
@@ -45,15 +45,16 @@ void scsi_show_rq(struct seq_file *m, struct request *rq)
list_for_each_entry(cmd2, &shost->eh_abort_list, eh_entry) {
if (cmd == cmd2) {
list_info = "on eh_abort_list";
- break;
+ goto unlock;
}
}
list_for_each_entry(cmd2, &shost->eh_cmd_q, eh_entry) {
if (cmd == cmd2) {
list_info = "on eh_cmd_q";
- break;
+ goto unlock;
}
}
+unlock:
spin_unlock_irq(shost->host_lock);
__scsi_format_command(buf, sizeof(buf), cmd->cmnd, cmd->cmd_len);
Thanks,
Bart.
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] scsi: core: Report error list information in debugfs
2023-08-22 15:25 ` Bart Van Assche
@ 2023-08-22 16:24 ` John Garry
0 siblings, 0 replies; 6+ messages in thread
From: John Garry @ 2023-08-22 16:24 UTC (permalink / raw)
To: Bart Van Assche, Martin K . Petersen
Cc: linux-scsi, Hannes Reinecke, Damien Le Moal, Mike Christie,
Ming Lei, James E.J. Bottomley
On 22/08/2023 16:25, Bart Van Assche wrote:
> On 8/22/23 02:04, John Garry wrote:
>> 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...
>
> Since the above code is only used to export information through debugfs
> I don't
> think that it should be optimized heavily?
To me it's not really a matter of having the code optimized, but more
about having clear and efficient code.
> Anyway, how about combining the
> (untested) patch below with the above patch?
>
> diff --git a/drivers/scsi/scsi_debugfs.c b/drivers/scsi/scsi_debugfs.c
> index a9bc5f7ce745..f795848b316c 100644
> --- a/drivers/scsi/scsi_debugfs.c
> +++ b/drivers/scsi/scsi_debugfs.c
> @@ -45,15 +45,16 @@ void scsi_show_rq(struct seq_file *m, struct request
> *rq)
> list_for_each_entry(cmd2, &shost->eh_abort_list, eh_entry) {
> if (cmd == cmd2) {
> list_info = "on eh_abort_list";
> - break;
> + goto unlock;
> }
> }
> list_for_each_entry(cmd2, &shost->eh_cmd_q, eh_entry) {
> if (cmd == cmd2) {
> list_info = "on eh_cmd_q";
> - break;
> + goto unlock;
> }
> }
I think that list_info could be set to NULL here, rather than when declared.
> +unlock:
> spin_unlock_irq(shost->host_lock);
>
> __scsi_format_command(buf, sizeof(buf), cmd->cmnd, cmd->cmd_len);
Regardless of comment, above:
Reviewed-by: John Garry <john.g.garry@oracle.com>
thanks
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] scsi: core: Report error list information in debugfs
2023-08-21 20:41 [PATCH] scsi: core: Report error list information in debugfs Bart Van Assche
2023-08-22 5:52 ` Damien Le Moal
2023-08-22 9:04 ` John Garry
@ 2023-10-09 7:12 ` Hannes Reinecke
2 siblings, 0 replies; 6+ messages in thread
From: Hannes Reinecke @ 2023-10-09 7:12 UTC (permalink / raw)
To: Bart Van Assche, Martin K . Petersen
Cc: linux-scsi, Damien Le Moal, Mike Christie, John Garry, Ming Lei,
James E.J. Bottomley
On 8/21/23 22: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(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-10-09 7:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-21 20:41 [PATCH] scsi: core: Report error list information in debugfs Bart Van Assche
2023-08-22 5:52 ` Damien Le Moal
2023-08-22 9:04 ` John Garry
2023-08-22 15:25 ` Bart Van Assche
2023-08-22 16:24 ` John Garry
2023-10-09 7:12 ` Hannes Reinecke
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox