Re: [PATCH v5] scsi: add debugfs directories

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09/01/2019 09:17, John Garry wrote:
On 08/01/2019 04:00, Douglas Gilbert wrote:
Add a top level "scsi" directory in debugfs (usually at
/sys/kernel/debug/scsi) with two subdirectories: "uld" and "lld".
The idea is to place mid-level related 'knobs' in the "scsi"
directory, and for the ULDs to make subdirectories like
"scsi/uld/sd" and "scsi/uld/st" as required. LLDs could follow a
similar pattern.

Changes since v4:
  - remove export for /sys/kernel/debug/scsi directory but leave
    the exports for the uld and lld subdirectories
  - put externs for scsi_debugfs_uld and scsi_debugfs_lld in
    include/scsi/scsi_dbg.h . So ULDs and LLDs now need to
    include <scsi/scsi_dbg.h> to use this facility
  - clean-up whitespaces and redundant code [John Garry]

Changes since v3:
  - re-arrange as per scsi netlink interface [James Bottomley]
  - make all debugfs code conditional on CONFIG_BLK_DEBUG_FS

Changes since v2:
  - export symbols so other driver can use them [John Garry]
  - make prior code conditional on CONFIG_BLK_DEBUG_FS

Changes since v1:
  - tweak Makefile to keep kbuild test robot happier

Signed-off-by: Douglas Gilbert <dgilbert@xxxxxxxxxxxx>

I think that this gives somewhat better organisation to the debugfs
root, so, apart from comments below, FWIW:
Reviewed-by: John Garry <john.garry@xxxxxxxxxx>

---

My intention is to add a /sys/kernel/debug/scsi/uld/sg
directory containing at least a pseudo file called debug to have
the same actions as /proc/scsi/sg/debug , as part of my sg v4
patchset.

John Garry has indicated that he is prepared to modify debugfs
patches to the hisi_sas driver to use the proposed
/sys/kernel/debug/scsi/lld directory.

I can send a patch for this (even if kbuild robot will complain),  which
you can pick up. But I think that we still need to see the user of uld.


 drivers/scsi/scsi.c         |  3 +++
 drivers/scsi/scsi_debugfs.c | 32 +++++++++++++++++++++++++++++++-
 drivers/scsi/scsi_priv.h    | 12 ++++++++++++
 include/scsi/scsi_dbg.h     |  5 +++++
 4 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index fc1356d101b0..e8676a19ba6e 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -812,6 +812,8 @@ static int __init init_scsi(void)

     scsi_netlink_init();

+    scsi_debugfs_init();
+
     printk(KERN_NOTICE "SCSI subsystem initialized\n");
     return 0;

@@ -832,6 +834,7 @@ static int __init init_scsi(void)

 static void __exit exit_scsi(void)
 {
+    scsi_debugfs_exit();
     scsi_netlink_exit();
     scsi_sysfs_unregister();
     scsi_exit_sysctl();
diff --git a/drivers/scsi/scsi_debugfs.c b/drivers/scsi/scsi_debugfs.c
index c5a8756384bc..5b0c001c150f 100644
--- a/drivers/scsi/scsi_debugfs.c
+++ b/drivers/scsi/scsi_debugfs.c
@@ -1,8 +1,18 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <linux/seq_file.h>
+#include <linux/debugfs.h>
 #include <scsi/scsi_cmnd.h>
 #include <scsi/scsi_dbg.h>
-#include "scsi_debugfs.h"
+#include "scsi_priv.h"
+
+struct dentry *scsi_debugfs_root;

I'm going to sound like a broken record here (and maybe I did not
understand the response to v4 review comment): For now, this only seems
to be accessed from this file, so better to make static until referenced
from elsewhere. As I see, only scsi_debugfs_uld and scsi_debugfs_lld
would be referenced externally.

+
+struct dentry *scsi_debugfs_uld;
+EXPORT_SYMBOL(scsi_debugfs_uld);
+
+struct dentry *scsi_debugfs_lld;
+EXPORT_SYMBOL(scsi_debugfs_lld);
+

 #define SCSI_CMD_FLAG_NAME(name)[const_ilog2(SCMD_##name)] = #name
 static const char *const scsi_cmd_flags[] = {
@@ -50,3 +60,23 @@ void scsi_show_rq(struct seq_file *m, struct
request *rq)
            timeout_ms / 1000, timeout_ms % 1000,
            alloc_ms / 1000, alloc_ms % 1000);
 }
+
+void scsi_debugfs_init(void)
+{
+    scsi_debugfs_root = debugfs_create_dir("scsi", NULL);
+    if (!scsi_debugfs_root)
+        return;
+    scsi_debugfs_uld = debugfs_create_dir("uld", scsi_debugfs_root);
+    if (!scsi_debugfs_uld) {
+        scsi_debugfs_exit();
+        return;
+    }
+    scsi_debugfs_lld = debugfs_create_dir("lld", scsi_debugfs_root);
+    if (!scsi_debugfs_lld)
+        scsi_debugfs_exit();
+}
+
+void scsi_debugfs_exit(void)
+{
+    debugfs_remove_recursive(scsi_debugfs_root);
+}
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index 99f1db5e467e..e24835e8fa4f 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -160,6 +160,18 @@ static inline void scsi_netlink_init(void) {}
 static inline void scsi_netlink_exit(void) {}
 #endif

+/* scsi_debugfs.c */
+#ifdef CONFIG_BLK_DEBUG_FS
+extern void scsi_debugfs_init(void);
+extern void scsi_debugfs_exit(void);
+extern struct dentry *scsi_debugfs_root;
+extern struct dentry *scsi_debugfs_uld;
+extern struct dentry *scsi_debugfs_lld;

And I am not sure why we have these defined as extern twice.

John

+#else
+static inline void scsi_debugfs_init(void) {}
+static inline void scsi_debugfs_exit(void) {}
+#endif
+
 /* scsi_pm.c */
 #ifdef CONFIG_PM
 extern const struct dev_pm_ops scsi_bus_pm_ops;
diff --git a/include/scsi/scsi_dbg.h b/include/scsi/scsi_dbg.h
index e03bd9d41fa8..00d121bf5eb2 100644
--- a/include/scsi/scsi_dbg.h
+++ b/include/scsi/scsi_dbg.h
@@ -2,6 +2,11 @@
 #ifndef _SCSI_SCSI_DBG_H
 #define _SCSI_SCSI_DBG_H

+#ifdef CONFIG_BLK_DEBUG_FS
+extern struct dentry *scsi_debugfs_uld;
+extern struct dentry *scsi_debugfs_lld;
+#endif
+
 struct scsi_cmnd;
 struct scsi_device;
 struct scsi_sense_hdr;




.






[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux