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

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

[Bart Van Assche]

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,
-- 
2.12.2

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

[Hannes Reinecke]

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
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.com			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

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

[Martin K. Petersen]

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?

-- 
Martin K. Petersen	Oracle Linux Engineering

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

[Bart Van Assche]

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.

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

[Martin K. Petersen]

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?

-- 
Martin K. Petersen	Oracle Linux Engineering

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

[Omar Sandoval]

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.

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

[Martin K. Petersen]

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.

-- 
Martin K. Petersen	Oracle Linux Engineering

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

[Bart Van Assche]

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.

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

[Martin K. Petersen]

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.

-- 
Martin K. Petersen	Oracle Linux Engineering