Linux-SCSI Archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Improve the code for showing commands in debugfs
@ 2024-03-25 22:47 Bart Van Assche
  2024-03-25 22:47 ` [PATCH 1/2] scsi: core: Introduce scsi_cmd_list_info() Bart Van Assche
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Bart Van Assche @ 2024-03-25 22:47 UTC (permalink / raw
  To: Martin K . Petersen; +Cc: linux-scsi, Bart Van Assche

Hi Martin,

The SCSI debugfs code may show information in debugfs that is invalid.
Hence this patch series that makes sure only valid information is shown
in debugfs. Please consider this patch series for the next merge window.

Thanks,

Bart.

Bart Van Assche (2):
  scsi: core: Introduce scsi_cmd_list_info()
  scsi: core: Improve the code for showing commands in debugfs

 drivers/scsi/scsi_debugfs.c | 56 ++++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 25 deletions(-)


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] scsi: core: Introduce scsi_cmd_list_info()
  2024-03-25 22:47 [PATCH 0/2] Improve the code for showing commands in debugfs Bart Van Assche
@ 2024-03-25 22:47 ` Bart Van Assche
  2024-03-25 22:47 ` [PATCH 2/2] scsi: core: Improve the code for showing commands in debugfs Bart Van Assche
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Bart Van Assche @ 2024-03-25 22:47 UTC (permalink / raw
  To: Martin K . Petersen; +Cc: linux-scsi, Bart Van Assche, James E.J. Bottomley

Slightly improve code readability by introducing a helper function for
deriving the list information and by using guard() + return instead of
goto + explicit unlock + return.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/scsi_debugfs.c | 40 +++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/drivers/scsi/scsi_debugfs.c b/drivers/scsi/scsi_debugfs.c
index f795848b316c..d4eaca7cc952 100644
--- a/drivers/scsi/scsi_debugfs.c
+++ b/drivers/scsi/scsi_debugfs.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <linux/bitops.h>
+#include <linux/cleanup.h>
 #include <linux/seq_file.h>
 #include <scsi/scsi_cmnd.h>
 #include <scsi/scsi_dbg.h>
@@ -32,31 +33,32 @@ static int scsi_flags_show(struct seq_file *m, const unsigned long flags,
 	return 0;
 }
 
-void scsi_show_rq(struct seq_file *m, struct request *rq)
+static const char *scsi_cmd_list_info(struct scsi_cmnd *cmd)
 {
-	struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq), *cmd2;
 	struct Scsi_Host *shost = cmd->device->host;
+	struct scsi_cmnd *cmd2;
+
+	guard(spinlock_irq)(shost->host_lock);
+
+	list_for_each_entry(cmd2, &shost->eh_abort_list, eh_entry)
+		if (cmd == cmd2)
+			return "on eh_abort_list";
+
+	list_for_each_entry(cmd2, &shost->eh_cmd_q, eh_entry)
+		if (cmd == cmd2)
+			return "on eh_cmd_q";
+
+	return NULL;
+}
+
+void scsi_show_rq(struct seq_file *m, struct request *rq)
+{
+	struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
 	int alloc_ms = jiffies_to_msecs(jiffies - cmd->jiffies_at_alloc);
 	int timeout_ms = jiffies_to_msecs(rq->timeout);
-	const char *list_info = NULL;
+	const char *list_info = scsi_cmd_list_info(cmd);
 	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";
-			goto unlock;
-		}
-	}
-	list_for_each_entry(cmd2, &shost->eh_cmd_q, eh_entry) {
-		if (cmd == cmd2) {
-			list_info = "on eh_cmd_q";
-			goto unlock;
-		}
-	}
-unlock:
-	spin_unlock_irq(shost->host_lock);
-
 	__scsi_format_command(buf, sizeof(buf), cmd->cmnd, cmd->cmd_len);
 	seq_printf(m, ", .cmd=%s, .retries=%d, .allowed=%d, .result = %#x, %s%s.flags=",
 		   buf, cmd->retries, cmd->allowed, cmd->result,

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] scsi: core: Improve the code for showing commands in debugfs
  2024-03-25 22:47 [PATCH 0/2] Improve the code for showing commands in debugfs Bart Van Assche
  2024-03-25 22:47 ` [PATCH 1/2] scsi: core: Introduce scsi_cmd_list_info() Bart Van Assche
@ 2024-03-25 22:47 ` Bart Van Assche
  2024-04-05 19:47 ` [PATCH 0/2] " Bart Van Assche
  2024-04-12  2:05 ` Martin K. Petersen
  3 siblings, 0 replies; 5+ messages in thread
From: Bart Van Assche @ 2024-03-25 22:47 UTC (permalink / raw
  To: Martin K . Petersen; +Cc: linux-scsi, Bart Van Assche, James E.J. Bottomley

Some but not all command information is cleared by scsi_end_request().
As an example, if scsi_show_rq() is called after a SCSI command has been
allocated and before SCMD_INITIALIZED is set, .cmnd holds the CDB
of a previous command. Showing that information in debugfs is confusing.
Hence this patch that restricts the information shown in debugfs to
valid information.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/scsi_debugfs.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/scsi_debugfs.c b/drivers/scsi/scsi_debugfs.c
index d4eaca7cc952..eb52e39f37c9 100644
--- a/drivers/scsi/scsi_debugfs.c
+++ b/drivers/scsi/scsi_debugfs.c
@@ -56,16 +56,20 @@ void scsi_show_rq(struct seq_file *m, struct request *rq)
 	struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
 	int alloc_ms = jiffies_to_msecs(jiffies - cmd->jiffies_at_alloc);
 	int timeout_ms = jiffies_to_msecs(rq->timeout);
-	const char *list_info = scsi_cmd_list_info(cmd);
 	char buf[80] = "(?)";
 
-	__scsi_format_command(buf, sizeof(buf), cmd->cmnd, cmd->cmd_len);
-	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 ? ", " : "");
+	if (cmd->flags & SCMD_INITIALIZED) {
+		const char *list_info = scsi_cmd_list_info(cmd);
+
+		__scsi_format_command(buf, sizeof(buf), cmd->cmnd, cmd->cmd_len);
+		seq_printf(m, ", .cmd=%s, .retries=%d, .allowed=%d, .result = %#x%s%s",
+			   buf, cmd->retries, cmd->allowed, cmd->result,
+			   list_info ? ", " : "", list_info ? : "");
+		seq_printf(m, ", .timeout=%d.%03d, allocated %d.%03d s ago",
+			   timeout_ms / 1000, timeout_ms % 1000,
+			   alloc_ms / 1000, alloc_ms % 1000);
+	}
+	seq_printf(m, ", .flags=");
 	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",
-		   timeout_ms / 1000, timeout_ms % 1000,
-		   alloc_ms / 1000, alloc_ms % 1000);
 }

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] Improve the code for showing commands in debugfs
  2024-03-25 22:47 [PATCH 0/2] Improve the code for showing commands in debugfs Bart Van Assche
  2024-03-25 22:47 ` [PATCH 1/2] scsi: core: Introduce scsi_cmd_list_info() Bart Van Assche
  2024-03-25 22:47 ` [PATCH 2/2] scsi: core: Improve the code for showing commands in debugfs Bart Van Assche
@ 2024-04-05 19:47 ` Bart Van Assche
  2024-04-12  2:05 ` Martin K. Petersen
  3 siblings, 0 replies; 5+ messages in thread
From: Bart Van Assche @ 2024-04-05 19:47 UTC (permalink / raw
  To: Martin K . Petersen; +Cc: linux-scsi

On 3/25/24 15:47, Bart Van Assche wrote:
> The SCSI debugfs code may show information in debugfs that is invalid.
> Hence this patch series that makes sure only valid information is shown
> in debugfs. Please consider this patch series for the next merge window.

Can anyone help with reviewing this patch series?

Thanks,

Bart.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] Improve the code for showing commands in debugfs
  2024-03-25 22:47 [PATCH 0/2] Improve the code for showing commands in debugfs Bart Van Assche
                   ` (2 preceding siblings ...)
  2024-04-05 19:47 ` [PATCH 0/2] " Bart Van Assche
@ 2024-04-12  2:05 ` Martin K. Petersen
  3 siblings, 0 replies; 5+ messages in thread
From: Martin K. Petersen @ 2024-04-12  2:05 UTC (permalink / raw
  To: Bart Van Assche; +Cc: Martin K . Petersen, linux-scsi

On Mon, 25 Mar 2024 15:47:52 -0700, Bart Van Assche wrote:

> The SCSI debugfs code may show information in debugfs that is invalid.
> Hence this patch series that makes sure only valid information is shown
> in debugfs. Please consider this patch series for the next merge window.
> 
> Thanks,
> 
> Bart.
> 
> [...]

Applied to 6.10/scsi-queue, thanks!

[1/2] scsi: core: Introduce scsi_cmd_list_info()
      https://git.kernel.org/mkp/scsi/c/9972c02a8067
[2/2] scsi: core: Improve the code for showing commands in debugfs
      https://git.kernel.org/mkp/scsi/c/ba0f09b0dbd8

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-04-12  2:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-25 22:47 [PATCH 0/2] Improve the code for showing commands in debugfs Bart Van Assche
2024-03-25 22:47 ` [PATCH 1/2] scsi: core: Introduce scsi_cmd_list_info() Bart Van Assche
2024-03-25 22:47 ` [PATCH 2/2] scsi: core: Improve the code for showing commands in debugfs Bart Van Assche
2024-04-05 19:47 ` [PATCH 0/2] " Bart Van Assche
2024-04-12  2:05 ` Martin K. Petersen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).