[PATCH] lpfc: properly factor out debugfs code

[PATCH] lpfc: properly factor out debugfs code

[Jörn Engel]

Hello James!

I'm currently preparing a janitorial patch series and noticed that
this one already collides with some of your changes.  Can I leave it
to you to fix up the collision and send on?

Jörn

-- 
All art is but imitation of nature.
-- Lucius Annaeus Seneca


Saves literally hundreds of calls to empty functions.  Surprisingly,
this also saves a few bytes in the debug-case.  Gcc had an imo valid
warning about reg_val being used uninitialized.  Fixed that up as
well.

Before, CONFIG_SCSI_LPFC_DEBUG_FS=n:
   text    data     bss     dec     hex filename
 382377    7024    1604  391005   5f75d drivers/scsi/lpfc/built-in.o
After, CONFIG_SCSI_LPFC_DEBUG_FS=n:
   text    data     bss     dec     hex filename
 374325    7024    1604  382953   5d7e9 drivers/scsi/lpfc/built-in.o

Before, CONFIG_SCSI_LPFC_DEBUG_FS=y:
   text    data     bss     dec     hex filename
 401763    7032    1712  410507   6438b drivers/scsi/lpfc/built-in.o
After, CONFIG_SCSI_LPFC_DEBUG_FS=y:
   text    data     bss     dec     hex filename
 401749    7032    1712  410493   6437d drivers/scsi/lpfc/built-in.o

Signed-off-by: Joern Engel <joern@logfs.org>
---
 drivers/scsi/lpfc/Makefile       |    5 +++--
 drivers/scsi/lpfc/lpfc_crtn.h    |   25 +++++++++++++++++++++++--
 drivers/scsi/lpfc/lpfc_debugfs.c |   20 +++++---------------
 3 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/drivers/scsi/lpfc/Makefile b/drivers/scsi/lpfc/Makefile
index 88928f0..c0f6b90 100644
--- a/drivers/scsi/lpfc/Makefile
+++ b/drivers/scsi/lpfc/Makefile
@@ -24,6 +24,7 @@ ccflags-$(GCOV) += -O0
 
 obj-$(CONFIG_SCSI_LPFC) := lpfc.o
 
-lpfc-objs := lpfc_mem.o lpfc_sli.o lpfc_ct.o lpfc_els.o lpfc_hbadisc.o	\
+lpfc-y := lpfc_mem.o lpfc_sli.o lpfc_ct.o lpfc_els.o lpfc_hbadisc.o	\
 	lpfc_init.o lpfc_mbox.o lpfc_nportdisc.o lpfc_scsi.o lpfc_attr.o \
-	lpfc_vport.o lpfc_debugfs.o lpfc_bsg.o
+	lpfc_vport.o lpfc_bsg.o
+lpfc-$(CONFIG_SCSI_LPFC_DEBUG_FS) += lpfc_debugfs.o
diff --git a/drivers/scsi/lpfc/lpfc_crtn.h b/drivers/scsi/lpfc/lpfc_crtn.h
index fc20c24..2bbec9f 100644
--- a/drivers/scsi/lpfc/lpfc_crtn.h
+++ b/drivers/scsi/lpfc/lpfc_crtn.h
@@ -360,12 +360,33 @@ void destroy_port(struct lpfc_vport *);
 int lpfc_get_instance(void);
 void lpfc_host_attrib_init(struct Scsi_Host *);
 
-extern void lpfc_debugfs_initialize(struct lpfc_vport *);
-extern void lpfc_debugfs_terminate(struct lpfc_vport *);
+#ifdef CONFIG_SCSI_LPFC_DEBUG_FS
 extern void lpfc_debugfs_disc_trc(struct lpfc_vport *, int, char *, uint32_t,
 	uint32_t, uint32_t);
 extern void lpfc_debugfs_slow_ring_trc(struct lpfc_hba *, char *, uint32_t,
 	uint32_t, uint32_t);
+extern void lpfc_debugfs_initialize(struct lpfc_vport *);
+extern void lpfc_debugfs_terminate(struct lpfc_vport *);
+#else
+static inline void lpfc_debugfs_disc_trc(struct lpfc_vport *vport, int mask, char *fmt,
+	uint32_t data1, uint32_t data2, uint32_t data3)
+{
+}
+
+static inline void lpfc_debugfs_slow_ring_trc(struct lpfc_hba *phba, char *fmt,
+	uint32_t data1, uint32_t data2, uint32_t data3)
+{
+}
+
+static inline void lpfc_debugfs_initialize(struct lpfc_vport *vport)
+{
+}
+
+static inline void lpfc_debugfs_terminate(struct lpfc_vport *vport)
+{
+}
+#endif
+
 extern struct lpfc_hbq_init *lpfc_hbq_defs[];
 
 /* SLI4 if_type 2 externs. */
diff --git a/drivers/scsi/lpfc/lpfc_debugfs.c b/drivers/scsi/lpfc/lpfc_debugfs.c
index ffe82d1..b6faecb 100644
--- a/drivers/scsi/lpfc/lpfc_debugfs.c
+++ b/drivers/scsi/lpfc/lpfc_debugfs.c
@@ -18,6 +18,7 @@
  * included with this package.                                     *
  *******************************************************************/
 
+#ifdef CONFIG_SCSI_LPFC_DEBUG_FS
 #include <linux/blkdev.h>
 #include <linux/delay.h>
 #include <linux/dma-mapping.h>
@@ -49,7 +50,6 @@
 #include "lpfc_compat.h"
 #include "lpfc_debugfs.h"
 
-#ifdef CONFIG_SCSI_LPFC_DEBUG_FS
 /*
  * debugfs interface
  *
@@ -584,7 +584,6 @@ lpfc_debugfs_nodelist_data(struct lpfc_vport *vport, char *buf, int size)
 	spin_unlock_irq(shost->host_lock);
 	return len;
 }
-#endif
 
 /**
  * lpfc_debugfs_disc_trc - Store discovery trace log
@@ -602,11 +601,10 @@ lpfc_debugfs_nodelist_data(struct lpfc_vport *vport, char *buf, int size)
  * match will be thrown away. @fmt, @data1, @data2, and @data3 are used like
  * printf when displaying the log.
  **/
-inline void
+void
 lpfc_debugfs_disc_trc(struct lpfc_vport *vport, int mask, char *fmt,
 	uint32_t data1, uint32_t data2, uint32_t data3)
 {
-#ifdef CONFIG_SCSI_LPFC_DEBUG_FS
 	struct lpfc_debugfs_trc *dtp;
 	int index;
 
@@ -626,7 +624,6 @@ lpfc_debugfs_disc_trc(struct lpfc_vport *vport, int mask, char *fmt,
 	dtp->data3 = data3;
 	dtp->seq_cnt = atomic_inc_return(&lpfc_debugfs_seq_trc_cnt);
 	dtp->jif = jiffies;
-#endif
 	return;
 }
 
@@ -643,11 +640,10 @@ lpfc_debugfs_disc_trc(struct lpfc_vport *vport, int mask, char *fmt,
  * discovery trace buffer associated with @vport. @fmt, @data1, @data2, and
  * @data3 are used like printf when displaying the log.
  **/
-inline void
+void
 lpfc_debugfs_slow_ring_trc(struct lpfc_hba *phba, char *fmt,
 	uint32_t data1, uint32_t data2, uint32_t data3)
 {
-#ifdef CONFIG_SCSI_LPFC_DEBUG_FS
 	struct lpfc_debugfs_trc *dtp;
 	int index;
 
@@ -664,11 +660,9 @@ lpfc_debugfs_slow_ring_trc(struct lpfc_hba *phba, char *fmt,
 	dtp->data3 = data3;
 	dtp->seq_cnt = atomic_inc_return(&lpfc_debugfs_seq_trc_cnt);
 	dtp->jif = jiffies;
-#endif
 	return;
 }
 
-#ifdef CONFIG_SCSI_LPFC_DEBUG_FS
 /**
  * lpfc_debugfs_disc_trc_open - Open the discovery trace log
  * @inode: The inode pointer that contains a vport pointer.
@@ -2256,7 +2250,7 @@ lpfc_idiag_drbacc_write(struct file *file, const char __user *buf,
 {
 	struct lpfc_debug *debug = file->private_data;
 	struct lpfc_hba *phba = (struct lpfc_hba *)debug->i_private;
-	uint32_t drb_reg_id, value, reg_val;
+	uint32_t drb_reg_id, value, reg_val = 0;
 	void __iomem *drb_reg;
 	int rc;
 
@@ -2447,7 +2441,6 @@ static const struct file_operations lpfc_idiag_op_drbAcc = {
 	.release =      lpfc_idiag_cmd_release,
 };
 
-#endif
 
 /**
  * lpfc_debugfs_initialize - Initialize debugfs for a vport
@@ -2462,7 +2455,6 @@ static const struct file_operations lpfc_idiag_op_drbAcc = {
 inline void
 lpfc_debugfs_initialize(struct lpfc_vport *vport)
 {
-#ifdef CONFIG_SCSI_LPFC_DEBUG_FS
 	struct lpfc_hba   *phba = vport->phba;
 	char name[64];
 	uint32_t num, i;
@@ -2750,7 +2742,6 @@ lpfc_debugfs_initialize(struct lpfc_vport *vport)
 
 debug_failed:
 	return;
-#endif
 }
 
 /**
@@ -2767,7 +2758,6 @@ debug_failed:
 inline void
 lpfc_debugfs_terminate(struct lpfc_vport *vport)
 {
-#ifdef CONFIG_SCSI_LPFC_DEBUG_FS
 	struct lpfc_hba   *phba = vport->phba;
 
 	if (vport->disc_trc) {
@@ -2866,6 +2856,6 @@ lpfc_debugfs_terminate(struct lpfc_vport *vport)
 			lpfc_debugfs_root = NULL;
 		}
 	}
-#endif
 	return;
 }
+#endif
-- 
1.7.2.3

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] lpfc: properly factor out debugfs code

[Jörn Engel]

On Fri, 26 August 2011 11:37:37 +0200, Jörn Engel wrote:
> 
> I'm currently preparing a janitorial patch series and noticed that
> this one already collides with some of your changes.  Can I leave it
> to you to fix up the collision and send on?

Actually here is a fixed version - or at least one that applies to
Linus' current tree.  I'm rather impressed that this reduces your
object code by 6%.  Wouldn't be surprised if it even shows itself in
some benchmarks.

Jörn

-- 
Fantasy is more important than knowledge. Knowledge is limited,
while fantasy embraces the whole world.
-- Albert Einstein

Before:
   text    data     bss     dec     hex filename
 414641    7080    1712  423433   67609 drivers/scsi/lpfc/built-in.o
After:
   text    data     bss     dec     hex filename
 387057    7072    1604  395733   609d5 drivers/scsi/lpfc/built-in.o

Signed-off-by: Joern Engel <joern@logfs.org>
---
 drivers/scsi/lpfc/Makefile       |    3 ++-
 drivers/scsi/lpfc/lpfc_crtn.h    |   20 ++++++++++++++++++++
 drivers/scsi/lpfc/lpfc_debugfs.c |   25 ++++---------------------
 drivers/scsi/lpfc/lpfc_vport.c   |    2 +-
 4 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/drivers/scsi/lpfc/Makefile b/drivers/scsi/lpfc/Makefile
index 88928f0..065e332 100644
--- a/drivers/scsi/lpfc/Makefile
+++ b/drivers/scsi/lpfc/Makefile
@@ -26,4 +26,5 @@ obj-$(CONFIG_SCSI_LPFC) := lpfc.o
 
 lpfc-objs := lpfc_mem.o lpfc_sli.o lpfc_ct.o lpfc_els.o lpfc_hbadisc.o	\
 	lpfc_init.o lpfc_mbox.o lpfc_nportdisc.o lpfc_scsi.o lpfc_attr.o \
-	lpfc_vport.o lpfc_debugfs.o lpfc_bsg.o
+	lpfc_vport.o lpfc_bsg.o
+lpfc-$(CONFIG_SCSI_LPFC_DEBUG_FS) += lpfc_debugfs.o
diff --git a/drivers/scsi/lpfc/lpfc_crtn.h b/drivers/scsi/lpfc/lpfc_crtn.h
index a6db6ae..c9a1472 100644
--- a/drivers/scsi/lpfc/lpfc_crtn.h
+++ b/drivers/scsi/lpfc/lpfc_crtn.h
@@ -362,12 +362,32 @@ void destroy_port(struct lpfc_vport *);
 int lpfc_get_instance(void);
 void lpfc_host_attrib_init(struct Scsi_Host *);
 
+#ifdef CONFIG_SCSI_LPFC_DEBUG_FS
 extern void lpfc_debugfs_initialize(struct lpfc_vport *);
 extern void lpfc_debugfs_terminate(struct lpfc_vport *);
 extern void lpfc_debugfs_disc_trc(struct lpfc_vport *, int, char *, uint32_t,
 	uint32_t, uint32_t);
 extern void lpfc_debugfs_slow_ring_trc(struct lpfc_hba *, char *, uint32_t,
 	uint32_t, uint32_t);
+#else
+static inline void lpfc_debugfs_disc_trc(struct lpfc_vport *vport, int mask, char *fmt,
+	uint32_t data1, uint32_t data2, uint32_t data3)
+{
+}
+
+static inline void lpfc_debugfs_slow_ring_trc(struct lpfc_hba *phba, char *fmt,
+	uint32_t data1, uint32_t data2, uint32_t data3)
+{
+}
+
+static inline void lpfc_debugfs_initialize(struct lpfc_vport *vport)
+{
+}
+
+static inline void lpfc_debugfs_terminate(struct lpfc_vport *vport)
+{
+}
+#endif
 extern struct lpfc_hbq_init *lpfc_hbq_defs[];
 
 /* SLI4 if_type 2 externs. */
diff --git a/drivers/scsi/lpfc/lpfc_debugfs.c b/drivers/scsi/lpfc/lpfc_debugfs.c
index a0424dd..2ce9cd6 100644
--- a/drivers/scsi/lpfc/lpfc_debugfs.c
+++ b/drivers/scsi/lpfc/lpfc_debugfs.c
@@ -50,7 +50,6 @@
 #include "lpfc_debugfs.h"
 #include "lpfc_bsg.h"
 
-#ifdef CONFIG_SCSI_LPFC_DEBUG_FS
 /*
  * debugfs interface
  *
@@ -602,7 +601,6 @@ lpfc_debugfs_nodelist_data(struct lpfc_vport *vport, char *buf, int size)
 	spin_unlock_irq(shost->host_lock);
 	return len;
 }
-#endif
 
 /**
  * lpfc_debugfs_disc_trc - Store discovery trace log
@@ -620,11 +618,10 @@ lpfc_debugfs_nodelist_data(struct lpfc_vport *vport, char *buf, int size)
  * match will be thrown away. @fmt, @data1, @data2, and @data3 are used like
  * printf when displaying the log.
  **/
-inline void
+void
 lpfc_debugfs_disc_trc(struct lpfc_vport *vport, int mask, char *fmt,
 	uint32_t data1, uint32_t data2, uint32_t data3)
 {
-#ifdef CONFIG_SCSI_LPFC_DEBUG_FS
 	struct lpfc_debugfs_trc *dtp;
 	int index;
 
@@ -644,7 +641,6 @@ lpfc_debugfs_disc_trc(struct lpfc_vport *vport, int mask, char *fmt,
 	dtp->data3 = data3;
 	dtp->seq_cnt = atomic_inc_return(&lpfc_debugfs_seq_trc_cnt);
 	dtp->jif = jiffies;
-#endif
 	return;
 }
 
@@ -661,11 +657,10 @@ lpfc_debugfs_disc_trc(struct lpfc_vport *vport, int mask, char *fmt,
  * discovery trace buffer associated with @vport. @fmt, @data1, @data2, and
  * @data3 are used like printf when displaying the log.
  **/
-inline void
+void
 lpfc_debugfs_slow_ring_trc(struct lpfc_hba *phba, char *fmt,
 	uint32_t data1, uint32_t data2, uint32_t data3)
 {
-#ifdef CONFIG_SCSI_LPFC_DEBUG_FS
 	struct lpfc_debugfs_trc *dtp;
 	int index;
 
@@ -682,11 +677,9 @@ lpfc_debugfs_slow_ring_trc(struct lpfc_hba *phba, char *fmt,
 	dtp->data3 = data3;
 	dtp->seq_cnt = atomic_inc_return(&lpfc_debugfs_seq_trc_cnt);
 	dtp->jif = jiffies;
-#endif
 	return;
 }
 
-#ifdef CONFIG_SCSI_LPFC_DEBUG_FS
 /**
  * lpfc_debugfs_disc_trc_open - Open the discovery trace log
  * @inode: The inode pointer that contains a vport pointer.
@@ -3473,8 +3466,6 @@ static const struct file_operations lpfc_idiag_op_extAcc = {
 	.release =      lpfc_idiag_cmd_release,
 };
 
-#endif
-
 /* lpfc_idiag_mbxacc_dump_bsg_mbox - idiag debugfs dump bsg mailbox command
  * @phba: Pointer to HBA context object.
  * @dmabuf: Pointer to a DMA buffer descriptor.
@@ -3489,7 +3480,6 @@ lpfc_idiag_mbxacc_dump_bsg_mbox(struct lpfc_hba *phba, enum nemb_type nemb_tp,
 				enum sta_type sta_tp,
 				struct lpfc_dmabuf *dmabuf, uint32_t ext_buf)
 {
-#ifdef CONFIG_SCSI_LPFC_DEBUG_FS
 	uint32_t *mbx_mbox_cmd, *mbx_dump_map, *mbx_dump_cnt, *mbx_word_cnt;
 	char line_buf[LPFC_MBX_ACC_LBUF_SZ];
 	int len = 0;
@@ -3571,7 +3561,6 @@ lpfc_idiag_mbxacc_dump_bsg_mbox(struct lpfc_hba *phba, enum nemb_type nemb_tp,
 	if (*mbx_dump_cnt == 0)
 		memset(&idiag, 0, sizeof(idiag));
 	return;
-#endif
 }
 
 /* lpfc_idiag_mbxacc_dump_issue_mbox - idiag debugfs dump issue mailbox command
@@ -3585,7 +3574,6 @@ lpfc_idiag_mbxacc_dump_bsg_mbox(struct lpfc_hba *phba, enum nemb_type nemb_tp,
 void
 lpfc_idiag_mbxacc_dump_issue_mbox(struct lpfc_hba *phba, MAILBOX_t *pmbox)
 {
-#ifdef CONFIG_SCSI_LPFC_DEBUG_FS
 	uint32_t *mbx_dump_map, *mbx_dump_cnt, *mbx_word_cnt, *mbx_mbox_cmd;
 	char line_buf[LPFC_MBX_ACC_LBUF_SZ];
 	int len = 0;
@@ -3668,7 +3656,6 @@ lpfc_idiag_mbxacc_dump_issue_mbox(struct lpfc_hba *phba, MAILBOX_t *pmbox)
 	if (*mbx_dump_cnt == 0)
 		memset(&idiag, 0, sizeof(idiag));
 	return;
-#endif
 }
 
 /**
@@ -3681,10 +3668,9 @@ lpfc_idiag_mbxacc_dump_issue_mbox(struct lpfc_hba *phba, MAILBOX_t *pmbox)
  * lpfcX directory (for this HBA), and vportX directory for this vport. It will
  * also create each file used to access lpfc specific debugfs information.
  **/
-inline void
+void
 lpfc_debugfs_initialize(struct lpfc_vport *vport)
 {
-#ifdef CONFIG_SCSI_LPFC_DEBUG_FS
 	struct lpfc_hba   *phba = vport->phba;
 	char name[64];
 	uint32_t num, i;
@@ -4030,7 +4016,6 @@ lpfc_debugfs_initialize(struct lpfc_vport *vport)
 
 debug_failed:
 	return;
-#endif
 }
 
 /**
@@ -4044,10 +4029,9 @@ debug_failed:
  * this is the last user of the HBA directory or driver directory then it will
  * remove those from the debugfs infrastructure as well.
  **/
-inline void
+void
 lpfc_debugfs_terminate(struct lpfc_vport *vport)
 {
-#ifdef CONFIG_SCSI_LPFC_DEBUG_FS
 	struct lpfc_hba   *phba = vport->phba;
 
 	if (vport->disc_trc) {
@@ -4165,6 +4149,5 @@ lpfc_debugfs_terminate(struct lpfc_vport *vport)
 			lpfc_debugfs_root = NULL;
 		}
 	}
-#endif
 	return;
 }
diff --git a/drivers/scsi/lpfc/lpfc_vport.c b/drivers/scsi/lpfc/lpfc_vport.c
index 1feb551..e0a58be 100644
--- a/drivers/scsi/lpfc/lpfc_vport.c
+++ b/drivers/scsi/lpfc/lpfc_vport.c
@@ -46,7 +46,7 @@
 #include "lpfc_version.h"
 #include "lpfc_vport.h"
 
-inline void lpfc_vport_set_state(struct lpfc_vport *vport,
+void lpfc_vport_set_state(struct lpfc_vport *vport,
 				 enum fc_vport_state new_state)
 {
 	struct fc_vport *fc_vport = vport->fc_vport;
-- 
1.7.2.3

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] lpfc: properly factor out debugfs code

[James Smart]

Yes, I'll look into it.   The reg_val warning was a lapse in the compiler, 
which can't detect setting of fields as the macros accessed the element by 
address.  We did submit a patch to quiet the compiler.

-- james s



On 8/26/2011 5:37 AM, Jörn Engel wrote:
> Hello James!
>
> I'm currently preparing a janitorial patch series and noticed that
> this one already collides with some of your changes.  Can I leave it
> to you to fix up the collision and send on?
>
> Jörn
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html