diff mbox

[v4,10/10] scsi: Implement blk_mq_ops.show_rq()

Message ID 20170421234026.18970-11-bart.vanassche@sandisk.com (mailing list archive)
State New, archived
Headers show

Commit Message

Bart Van Assche April 21, 2017, 11:40 p.m. UTC
Show the SCSI CDB, .eh_eflags and .result for pending SCSI commands
in /sys/kernel/debug/block/*/mq/*/dispatch and */rq_list.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Omar Sandoval <osandov@fb.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: <linux-scsi@vger.kernel.org>
---
 drivers/scsi/scsi_lib.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Hannes Reinecke April 24, 2017, 7:35 a.m. UTC | #1
On 04/22/2017 01:40 AM, Bart Van Assche wrote:
> Show the SCSI CDB, .eh_eflags and .result for pending SCSI commands
> in /sys/kernel/debug/block/*/mq/*/dispatch and */rq_list.
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> Cc: Omar Sandoval <osandov@fb.com>
> Cc: Hannes Reinecke <hare@suse.com>
> Cc: <linux-scsi@vger.kernel.org>
> ---
>  drivers/scsi/scsi_lib.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 4a20e6098f7c..90bb269042df 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -2125,6 +2125,16 @@ static void scsi_exit_rq(struct request_queue *q, struct request *rq)
>  	scsi_free_sense_buffer(shost, cmd->sense_buffer);
>  }
>  
> +static void scsi_show_rq(struct seq_file *m, struct request *rq)
> +{
> +	struct scsi_cmnd *cmd = container_of(scsi_req(rq), typeof(*cmd), req);
> +	unsigned int i;
> +
> +	seq_puts(m, ", .cmd =");
> +	for (i = 0; i < cmd->cmd_len; i++)
> +		seq_printf(m, " %02x", cmd->cmnd[i]);
> +}
> +
>  struct request_queue *scsi_alloc_queue(struct scsi_device *sdev)
>  {
>  	struct Scsi_Host *shost = sdev->host;
> @@ -2157,6 +2167,7 @@ static const struct blk_mq_ops scsi_mq_ops = {
>  	.queue_rq	= scsi_queue_rq,
>  	.complete	= scsi_softirq_done,
>  	.timeout	= scsi_timeout,
> +	.show_rq	= scsi_show_rq,
>  	.init_request	= scsi_init_request,
>  	.exit_request	= scsi_exit_request,
>  	.map_queues	= scsi_map_queues,
> 
Hmm. Can't say I'm happy with this callback.
And I really would like to see a similar implementation for NVMe.

But if others agree:

Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
Martin K. Petersen April 24, 2017, 9:35 p.m. UTC | #2
Bart,

> Show the SCSI CDB, .eh_eflags and .result for pending SCSI commands
> in /sys/kernel/debug/block/*/mq/*/dispatch and */rq_list.

Why not use SCSI tracing if you are interested in these?
Bart Van Assche April 24, 2017, 9:49 p.m. UTC | #3
On Mon, 2017-04-24 at 17:35 -0400, Martin K. Petersen wrote:
> > Show the SCSI CDB, .eh_eflags and .result for pending SCSI commands
> > in /sys/kernel/debug/block/*/mq/*/dispatch and */rq_list.
> 
> Why not use SCSI tracing if you are interested in these?

Hello Martin,

SCSI tracing has to be enabled before a test is started, produces a huge amount
of data, and deriving state information from a huge trace is far from easy. The
information in debugfs provides an easy to read overview of the current state
without having to analyze megabytes of traces, without introducing any slowdown
and without having to enable any tracing mechanism from beforehand.

Bart.
Martin K. Petersen April 24, 2017, 11:19 p.m. UTC | #4
Bart,

> SCSI tracing has to be enabled before a test is started, produces a
> huge amount of data, and deriving state information from a huge trace
> is far from easy. The information in debugfs provides an easy to read
> overview of the current state without having to analyze megabytes of
> traces, without introducing any slowdown and without having to enable
> any tracing mechanism from beforehand.

Fair enough. Just seems like there's an obvious overlap in plumbing.
Don't know if that can be leveraged instead of introducing something
completely new?
Omar Sandoval April 24, 2017, 11:23 p.m. UTC | #5
On Mon, Apr 24, 2017 at 07:19:50PM -0400, Martin K. Petersen wrote:
> 
> Bart,
> 
> > SCSI tracing has to be enabled before a test is started, produces a
> > huge amount of data, and deriving state information from a huge trace
> > is far from easy. The information in debugfs provides an easy to read
> > overview of the current state without having to analyze megabytes of
> > traces, without introducing any slowdown and without having to enable
> > any tracing mechanism from beforehand.
> 
> Fair enough. Just seems like there's an obvious overlap in plumbing.
> Don't know if that can be leveraged instead of introducing something
> completely new?

The debugfs infrastructure is already there, and it already supports
showing requests. Bart is just exposing the SCSI information.
Martin K. Petersen April 24, 2017, 11:33 p.m. UTC | #6
Omar,

> The debugfs infrastructure is already there, and it already supports
> showing requests. Bart is just exposing the SCSI information.

That's fine.

I was merely objecting to the fact that we already have umpteen existing
interfaces for displaying SCSI command information.
Bart Van Assche April 24, 2017, 11:46 p.m. UTC | #7
On Mon, 2017-04-24 at 19:33 -0400, Martin K. Petersen wrote:
> > The debugfs infrastructure is already there, and it already supports
> > showing requests. Bart is just exposing the SCSI information.
> 
> That's fine.
> 
> I was merely objecting to the fact that we already have umpteen existing
> interfaces for displaying SCSI command information.

Hello Martin,

Do you perhaps want me to change the for-loop into a call to
__scsi_format_command()?

Thanks,

Bart.
Martin K. Petersen April 25, 2017, 4:40 p.m. UTC | #8
Bart,

>> I was merely objecting to the fact that we already have umpteen existing
>> interfaces for displaying SCSI command information.

> Do you perhaps want me to change the for-loop into a call to
> __scsi_format_command()?

If possible, I would love to see some commonality in the per-command
information regardless of whether it is displayed due to SCSI logging,
SCSI tracing or an error condition.
diff mbox

Patch

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 4a20e6098f7c..90bb269042df 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2125,6 +2125,16 @@  static void scsi_exit_rq(struct request_queue *q, struct request *rq)
 	scsi_free_sense_buffer(shost, cmd->sense_buffer);
 }
 
+static void scsi_show_rq(struct seq_file *m, struct request *rq)
+{
+	struct scsi_cmnd *cmd = container_of(scsi_req(rq), typeof(*cmd), req);
+	unsigned int i;
+
+	seq_puts(m, ", .cmd =");
+	for (i = 0; i < cmd->cmd_len; i++)
+		seq_printf(m, " %02x", cmd->cmnd[i]);
+}
+
 struct request_queue *scsi_alloc_queue(struct scsi_device *sdev)
 {
 	struct Scsi_Host *shost = sdev->host;
@@ -2157,6 +2167,7 @@  static const struct blk_mq_ops scsi_mq_ops = {
 	.queue_rq	= scsi_queue_rq,
 	.complete	= scsi_softirq_done,
 	.timeout	= scsi_timeout,
+	.show_rq	= scsi_show_rq,
 	.init_request	= scsi_init_request,
 	.exit_request	= scsi_exit_request,
 	.map_queues	= scsi_map_queues,