RE: [EXT] [PATCH] scsi: bnx2i: fix indentation issue, remove a tab

2019-01-07 Thread Manish Rangankar 
> -Original Message-
> From: Colin King 
> Sent: Friday, January 4, 2019 10:32 PM
> To: qlogic-storage-upstr...@qlogic.com; James E . J . Bottomley
> ; Martin K . Petersen ;
> linux-scsi@vger.kernel.org
> Cc: kernel-janit...@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: [EXT] [PATCH] scsi: bnx2i: fix indentation issue, remove a tab
> 
> External Email
> 
> --
> External Email
> 
> From: Colin Ian King 
> 
> The return statement is indented incorrectly, fix this by removing a single 
> tab.
> 
> Signed-off-by: Colin Ian King 
> ---
>  drivers/scsi/bnx2i/bnx2i_iscsi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c 
> b/drivers/scsi/bnx2i/bnx2i_iscsi.c
> index 69c75426c5eb..c5fa5f3b00e9 100644
> --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c
> +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c
> @@ -577,7 +577,7 @@ static void bnx2i_free_mp_bdt(struct bnx2i_hba *hba)
>   hba->dummy_buffer, hba->dummy_buf_dma);
> hba->dummy_buffer = NULL;
> }
> -   return;
> +   return;
>  }
> 
>  /**
> --
> 2.19.1

Thanks
Acked-by: Manish Rangankar

Re: [PATCH] scsi/mvsas/mv_init.c: Use dma_zalloc_coherent

2019-01-07 Thread John Garry 

On 04/01/2019 19:49, Souptick Joarder wrote:

On Fri, Jan 4, 2019 at 9:34 PM John Garry  wrote:


On 04/01/2019 15:11, Sabyasachi Gupta wrote:

On Fri, Jan 4, 2019 at 6:43 PM John Garry  wrote:


On 04/01/2019 12:48, Sabyasachi Gupta wrote:

On Wed, Dec 19, 2018 at 6:49 PM Sabyasachi Gupta
 wrote:


On Sat, Dec 1, 2018 at 6:40 PM Sabyasachi Gupta
 wrote:


On Wed, Nov 21, 2018 at 7:18 PM Sabyasachi Gupta
 wrote:


Replace dma_alloc_coherent + memset with dma_zalloc_coherent



If you're going to make this change, then how about change these to the
managed version, so that we can avoid the explicit free'ing at driver
removal?


I can't get it


Please see dmam_alloc_coherent(). You can set __GFP_ZERO in the gfp
argument to memset(0).


and dma_zalloc_coherent() did the same. So both are same right ? No ?



Firstly I would to say that I am not so keen on changes like this since 
(I assume) it will not be tested at all.


However, having said that, the reason to use dmam_alloc_coherent() is 
that you can also drop the dma_free_coherent() in mvs_free(). But 
changes like that would actually require some code analysis effort...


John



I would say that this is a more useful change.






Signed-off-by: Sabyasachi Gupta 


Any comment on this patch?


Any comment on this patch?


Any comment on this patch?






---
 drivers/scsi/mvsas/mv_init.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
index 3ac3437..495bddb 100644
--- a/drivers/scsi/mvsas/mv_init.c
+++ b/drivers/scsi/mvsas/mv_init.c
@@ -253,33 +253,29 @@ static int mvs_alloc(struct mvs_info *mvi, struct 
Scsi_Host *shost)
/*
 * alloc and init our DMA areas
 */
-   mvi->tx = dma_alloc_coherent(mvi->dev,
+   mvi->tx = dma_zalloc_coherent(mvi->dev,
 sizeof(*mvi->tx) * MVS_CHIP_SLOT_SZ,
 &mvi->tx_dma, GFP_KERNEL);


I'm guessing that this does not pass checkpatch with --strict option.

Thanks,
John


I have not not checked with --strict option


It may warn that you're not maintaining alignment.






if (!mvi->tx)
goto err_out;
-   memset(mvi->tx, 0, sizeof(*mvi->tx) * MVS_CHIP_SLOT_SZ);
-   mvi->rx_fis = dma_alloc_coherent(mvi->dev, MVS_RX_FISL_SZ,
+   mvi->rx_fis = dma_zalloc_coherent(mvi->dev, MVS_RX_FISL_SZ,
 &mvi->rx_fis_dma, GFP_KERNEL);
if (!mvi->rx_fis)
goto err_out;
-   memset(mvi->rx_fis, 0, MVS_RX_FISL_SZ);

-   mvi->rx = dma_alloc_coherent(mvi->dev,
+   mvi->rx = dma_zalloc_coherent(mvi->dev,
 sizeof(*mvi->rx) * (MVS_RX_RING_SZ + 1),
 &mvi->rx_dma, GFP_KERNEL);
if (!mvi->rx)
goto err_out;
-   memset(mvi->rx, 0, sizeof(*mvi->rx) * (MVS_RX_RING_SZ + 1));
mvi->rx[0] = cpu_to_le32(0xfff);
mvi->rx_cons = 0xfff;

-   mvi->slot = dma_alloc_coherent(mvi->dev,
+   mvi->slot = dma_zalloc_coherent(mvi->dev,
   sizeof(*mvi->slot) * slot_nr,
   &mvi->slot_dma, GFP_KERNEL);
if (!mvi->slot)
goto err_out;
-   memset(mvi->slot, 0, sizeof(*mvi->slot) * slot_nr);

mvi->bulk_buffer = dma_alloc_coherent(mvi->dev,
   TRASH_BUCKET_SIZE,
--
2.7.4



.






.






.

[PATCH v1 1/1] scsi: ufs: Fix system suspend status

2019-01-07 Thread stanley.chu 
From: Stanley Chu 

hba->is_sys_suspended is set after successful system suspend but
not clear after successful system resume.

According to current behavior, hba->is_sys_suspended will not
be set if host is runtime-suspended but not system-suspended. Thus
we shall aligh the same policy: clear this flag even if host remains
runtime-suspended after ufshcd_system_resume is successfully returned.

Simply fix this flag to correct host status logs.

Signed-off-by: Stanley Chu 
---
 drivers/scsi/ufs/ufshcd.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 23d7cca..17536bd 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -7916,6 +7916,8 @@ int ufshcd_system_resume(struct ufs_hba *hba)
trace_ufshcd_system_resume(dev_name(hba->dev), ret,
ktime_to_us(ktime_sub(ktime_get(), start)),
hba->curr_dev_pwr_mode, hba->uic_link_state);
+   if (!ret)
+   hba->is_sys_suspended = false;
return ret;
 }
 EXPORT_SYMBOL(ufshcd_system_resume);
-- 
1.9.1

[PATCH v1 0/1] scsi: ufs: Fix system suspend status

2019-01-07 Thread stanley.chu 
From: Stanley Chu 

Stanley Chu (1):
  scsi: ufs: Fix system suspend status

 drivers/scsi/ufs/ufshcd.c | 2 ++
 1 file changed, 2 insertions(+)

-- 
1.9.1

[PATCH v1] scsi: ufs: Fix system suspend status

2019-01-07 Thread stanley.chu 
hba->is_sys_suspended is set after successful system suspend but
not clear after successful system resume.

Simply fix this flag to correct host status logs.

[PATCH] scsi: sd: Make protection lookup tables static

2019-01-07 Thread John Garry 
Currently the protection lookup tables in sd_prot_flag_mask() and
sd_prot_op() are declared non-static. As such, they will be rebuilt for
each respective function call.

Optimise by making them static.

This saves ~100B object code for sd.c:

Before:
   textdata bss dec hex filename
  254031024  16   26443674b drivers/scsi/sd.o

After:
  text data bss dec hex filename
  252991024  16   2633966e3 drivers/scsi/sd.o

Signed-off-by: John Garry 

diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 1d63f3a..89e6d42 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -195,15 +195,15 @@ static inline sector_t sectors_to_logical(struct 
scsi_device *sdev, sector_t sec
 static inline unsigned int sd_prot_op(bool write, bool dix, bool dif)
 {
/* Lookup table: bit 2 (write), bit 1 (dix), bit 0 (dif) */
-   const unsigned int ops[] = {/* wrt dix dif */
-   SCSI_PROT_NORMAL,   /*  0   0   0  */
-   SCSI_PROT_READ_STRIP,   /*  0   0   1  */
-   SCSI_PROT_READ_INSERT,  /*  0   1   0  */
-   SCSI_PROT_READ_PASS,/*  0   1   1  */
-   SCSI_PROT_NORMAL,   /*  1   0   0  */
-   SCSI_PROT_WRITE_INSERT, /*  1   0   1  */
-   SCSI_PROT_WRITE_STRIP,  /*  1   1   0  */
-   SCSI_PROT_WRITE_PASS,   /*  1   1   1  */
+   static const unsigned int ops[] = { /* wrt dix dif */
+   SCSI_PROT_NORMAL,   /*  0   0   0  */
+   SCSI_PROT_READ_STRIP,   /*  0   0   1  */
+   SCSI_PROT_READ_INSERT,  /*  0   1   0  */
+   SCSI_PROT_READ_PASS,/*  0   1   1  */
+   SCSI_PROT_NORMAL,   /*  1   0   0  */
+   SCSI_PROT_WRITE_INSERT, /*  1   0   1  */
+   SCSI_PROT_WRITE_STRIP,  /*  1   1   0  */
+   SCSI_PROT_WRITE_PASS,   /*  1   1   1  */
};
 
return ops[write << 2 | dix << 1 | dif];
@@ -215,7 +215,7 @@ static inline unsigned int sd_prot_op(bool write, bool dix, 
bool dif)
  */
 static inline unsigned int sd_prot_flag_mask(unsigned int prot_op)
 {
-   const unsigned int flag_mask[] = {
+   static const unsigned int flag_mask[] = {
[SCSI_PROT_NORMAL]  = 0,
 
[SCSI_PROT_READ_STRIP]  = SCSI_PROT_TRANSFER_PI |
-- 
1.9.1

Re: [PATCH v4] scsi: add debugfs directories

2019-01-07 Thread John Garry 

On 05/01/2019 01:52, Douglas Gilbert wrote:

Add a top level "scsi" directory in debugfs (usually at
/sys/kernel/debugfs/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 subsirectories like
"scsi/uld/sd" and "scsi/uld/st" as required. LLDs could follow a
similar pattern.



Hi Doug,

Some comments, below:


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

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 v4
patchset.

 drivers/scsi/scsi.c |  3 +++
 drivers/scsi/scsi_debugfs.c | 33 -
 drivers/scsi/scsi_priv.h| 12 
 3 files changed, 47 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..20d4cb0fa58b 100644
--- a/drivers/scsi/scsi_debugfs.c
+++ b/drivers/scsi/scsi_debugfs.c
@@ -1,8 +1,18 @@
 // SPDX-License-Identifier: GPL-2.0
 #include 
+#include 
 #include 
 #include 
-#include "scsi_debugfs.h"
+#include "scsi_priv.h"
+
+struct dentry *scsi_debugfs_root;


Can this be static, i.e. not exported?


+struct dentry *scsi_debugfs_uld;
+struct dentry *scsi_debugfs_lld;
+
+EXPORT_SYMBOL(scsi_debugfs_root);
+EXPORT_SYMBOL(scsi_debugfs_uld);
+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,24 @@ 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;


This seems to be indented with whitespaces.


+scsi_debugfs_uld = debugfs_create_dir("uld", scsi_debugfs_root);
+if (!scsi_debugfs_uld) {
+   scsi_debugfs_exit();
+   return;
+   }


Strange indentation.


+scsi_debugfs_lld = debugfs_create_dir("lld", scsi_debugfs_root);
+if (!scsi_debugfs_lld)
+scsi_debugfs_exit();
+}
+
+void scsi_debugfs_exit(void)
+{
+   if (scsi_debugfs_root)


I think debugfs_remove_recursive() can safely handle NULL as an argument.


+   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


I am suspicious that this header should not be included outside 
driver/scsi, where some scsi LLDs exist.



@@ -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;
+#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;



You can include this change as a user of the lld folder if you want:

diff --git a/drivers/scsi/hisi_sas/hisi_sas.h 
b/drivers/scsi/hisi_sas/hisi_sas.h

index 6a1a5ad..964d34d8 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -28,6 +28,8 @@
 #include 
 #include 

+#include "../scsi_priv.h"
+
 #define HISI_SAS_MAX_PHYS  9
 #define HISI_SAS_MAX_QUEUES32
 #define HISI_SAS_QUEUE_SLOTS 512
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c 
b/drivers/scsi/hisi_sas/hisi_sas_main.c

index 5fe13e9..bc8f014 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -3018,7 +3018,8 @@ static __init int hisi_sas_init(void)
return -ENOMEM;

if (hisi

Re: [PATCH] scsi: sd: Make protection lookup tables static

2019-01-07 Thread Bart Van Assche 
On Mon, 2019-01-07 at 22:41 +0800, John Garry wrote:
> Currently the protection lookup tables in sd_prot_flag_mask() and
> sd_prot_op() are declared non-static. As such, they will be rebuilt for
> each respective function call.
> 
> Optimise by making them static.
> 
> This saves ~100B object code for sd.c:
> 
> Before:
>text  data bss dec hex filename
>   25403  1024  16   26443674b drivers/scsi/sd.o
> 
> After:
>   text   data bss dec hex filename
>   25299  1024  16   2633966e3 drivers/scsi/sd.o

Since the sd_prot_op() function only has a single caller, please move it from 
sd.h
into sd.c.

Thanks,

Bart.

Re: [PATCH] scsi: sd: Make protection lookup tables static

2019-01-07 Thread John Garry 

On 07/01/2019 16:57, Bart Van Assche wrote:

On Mon, 2019-01-07 at 22:41 +0800, John Garry wrote:

Currently the protection lookup tables in sd_prot_flag_mask() and
sd_prot_op() are declared non-static. As such, they will be rebuilt for
each respective function call.

Optimise by making them static.

This saves ~100B object code for sd.c:

Before:
   textdata bss dec hex filename
  254031024  16   26443674b drivers/scsi/sd.o

After:
  text data bss dec hex filename
  252991024  16   2633966e3 drivers/scsi/sd.o


Since the sd_prot_op() function only has a single caller, please move it from 
sd.h
into sd.c.


Can do, as long as no one has objection.

Function sd_prot_flag_mask() also has a single caller AFAICS, so this 
could also be relocated.


Cheers,
John



Thanks,

Bart.

Re: [PATCH] scsi: sd: Make protection lookup tables static

2019-01-07 Thread Bart Van Assche 
On Mon, 2019-01-07 at 17:27 +, John Garry wrote:
> On 07/01/2019 16:57, Bart Van Assche wrote:
> > On Mon, 2019-01-07 at 22:41 +0800, John Garry wrote:
> > > Currently the protection lookup tables in sd_prot_flag_mask() and
> > > sd_prot_op() are declared non-static. As such, they will be rebuilt for
> > > each respective function call.
> > > 
> > > Optimise by making them static.
> > > 
> > > This saves ~100B object code for sd.c:
> > > 
> > > Before:
> > >text  data bss dec hex filename
> > >   25403  1024  16   26443674b drivers/scsi/sd.o
> > > 
> > > After:
> > >   text   data bss dec hex filename
> > >   25299  1024  16   2633966e3 drivers/scsi/sd.o
> > 
> > Since the sd_prot_op() function only has a single caller, please move it 
> > from sd.h
> > into sd.c.
> 
> Can do, as long as no one has objection.
> 
> Function sd_prot_flag_mask() also has a single caller AFAICS, so this 
> could also be relocated.

Please move that function too from sd.h into sd.c.

Thanks,

Bart.

Re: [PATCH v4] scsi: add debugfs directories

2019-01-07 Thread Douglas Gilbert 

John,
See response below.

On 2019-01-07 10:38 a.m., John Garry wrote:

On 05/01/2019 01:52, Douglas Gilbert wrote:

Add a top level "scsi" directory in debugfs (usually at
/sys/kernel/debugfs/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 subsirectories like
"scsi/uld/sd" and "scsi/uld/st" as required. LLDs could follow a
similar pattern.



Hi Doug,

Some comments, below:


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

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 v4
patchset.

 drivers/scsi/scsi.c |  3 +++
 drivers/scsi/scsi_debugfs.c | 33 -
 drivers/scsi/scsi_priv.h    | 12 
 3 files changed, 47 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..20d4cb0fa58b 100644
--- a/drivers/scsi/scsi_debugfs.c
+++ b/drivers/scsi/scsi_debugfs.c
@@ -1,8 +1,18 @@
 // SPDX-License-Identifier: GPL-2.0
 #include 
+#include 
 #include 
 #include 
-#include "scsi_debugfs.h"
+#include "scsi_priv.h"
+
+struct dentry *scsi_debugfs_root;


Can this be static, i.e. not exported?


Yes. I'll make in global, non-exported so the mid-level (and
transports ?) can add stuff in the /sys/kernel/debug/scsi
directory.


+struct dentry *scsi_debugfs_uld;
+struct dentry *scsi_debugfs_lld;
+
+EXPORT_SYMBOL(scsi_debugfs_root);
+EXPORT_SYMBOL(scsi_debugfs_uld);
+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,24 @@ 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;


This seems to be indented with whitespaces.


My bad. How could I have forgotten to use my favourite whitespace checker?


+    scsi_debugfs_uld = debugfs_create_dir("uld", scsi_debugfs_root);
+    if (!scsi_debugfs_uld) {
+    scsi_debugfs_exit();
+    return;
+    }


Strange indentation.


+    scsi_debugfs_lld = debugfs_create_dir("lld", scsi_debugfs_root);
+    if (!scsi_debugfs_lld)
+    scsi_debugfs_exit();
+}
+
+void scsi_debugfs_exit(void)
+{
+    if (scsi_debugfs_root)


I think debugfs_remove_recursive() can safely handle NULL as an argument.


checkpatch.pl picked that one up as well.


+    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


I am suspicious that this header should not be included outside driver/scsi, 
where some scsi LLDs exist.


Yes, it only seems to be included in mid-level and transport function files;
not in ULDs or LLDs. So I add this:

  #ifdef CONFIG_BLK_DEBUG_FS
  extern struct dentry *scsi_debugfs_uld;
  extern struct dentry *scsi_debugfs_lld;
  #endif

in include/scsi/scsi_dbg.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;
+#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;



You can include this change as a user of the lld folder if you want:

diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index 6a1a5ad..964d34d8 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/dri

[PATCH v5] scsi: add debugfs directories

2019-01-07 Thread Douglas Gilbert 
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  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 
---

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.

 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 
+#include 
 #include 
 #include 
-#include "scsi_debugfs.h"
+#include "scsi_priv.h"
+
+struct dentry *scsi_debugfs_root;
+
+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;
+#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;
-- 
2.17.1