[PATCH 0/6] qla2xxx driver features

[PATCH 0/6] qla2xxx driver features

[Nilesh Javali]

Martin,

Please apply the qla2xxx driver features to the scsi tree
at your earliest convenience.

Thanks,
Nilesh

Anil Gurumurthy (1):
  qla2xxx: Add NVMe parameters support in Auxiliary Image Status

Arun Easi (4):
  qla2xxx: Add debugfs create/delete helpers
  qla2xxx: Add a generic tracing framework
  qla2xxx: Add driver console messages tracing
  qla2xxx: Add srb tracing

Nilesh Javali (1):
  qla2xxx: Update version to 10.02.07.900-k

 drivers/scsi/qla2xxx/qla_bsg.c     |   8 +-
 drivers/scsi/qla2xxx/qla_bsg.h     |   3 +-
 drivers/scsi/qla2xxx/qla_dbg.c     |  26 ++-
 drivers/scsi/qla2xxx/qla_dbg.h     | 286 +++++++++++++++++++++++++++++
 drivers/scsi/qla2xxx/qla_def.h     |  55 +++++-
 drivers/scsi/qla2xxx/qla_dfs.c     | 239 ++++++++++++++++++++++++
 drivers/scsi/qla2xxx/qla_fw.h      |   3 +
 drivers/scsi/qla2xxx/qla_gbl.h     |   1 +
 drivers/scsi/qla2xxx/qla_init.c    |   8 +-
 drivers/scsi/qla2xxx/qla_inline.h  |  12 ++
 drivers/scsi/qla2xxx/qla_iocb.c    |   4 +
 drivers/scsi/qla2xxx/qla_isr.c     |   5 +
 drivers/scsi/qla2xxx/qla_os.c      |  35 ++++
 drivers/scsi/qla2xxx/qla_version.h |   4 +-
 14 files changed, 675 insertions(+), 14 deletions(-)


base-commit: f095c3cd1b694a73a5de276dae919f05a8dd1811
-- 
2.19.0.rc0

[PATCH 1/6] qla2xxx: Add debugfs create/delete helpers

[Nilesh Javali]

From: Arun Easi <aeasi@marvell.com>

Define a few helpful macros for creating debugfs files.

Signed-off-by: Arun Easi <aeasi@marvell.com>
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_def.h |  5 ++
 drivers/scsi/qla2xxx/qla_dfs.c | 90 ++++++++++++++++++++++++++++++++++
 2 files changed, 95 insertions(+)

diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index 3ec6a200942e..22274b405d01 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -35,6 +35,11 @@
 
 #include <uapi/scsi/fc/fc_els.h>
 
+#define QLA_DFS_DEFINE_DENTRY(_debugfs_file_name) \
+	struct dentry *dfs_##_debugfs_file_name
+#define QLA_DFS_ROOT_DEFINE_DENTRY(_debugfs_file_name) \
+	struct dentry *qla_dfs_##_debugfs_file_name
+
 /* Big endian Fibre Channel S_ID (source ID) or D_ID (destination ID). */
 typedef struct {
 	uint8_t domain;
diff --git a/drivers/scsi/qla2xxx/qla_dfs.c b/drivers/scsi/qla2xxx/qla_dfs.c
index 85bd0e468d43..c3c8b9536ef6 100644
--- a/drivers/scsi/qla2xxx/qla_dfs.c
+++ b/drivers/scsi/qla2xxx/qla_dfs.c
@@ -489,6 +489,96 @@ qla_dfs_naqp_show(struct seq_file *s, void *unused)
 	return 0;
 }
 
+/*
+ * Helper macros for setting up debugfs entries.
+ * _name: The name of the debugfs entry
+ * _ctx_struct: The context that was passed when creating the debugfs file
+ *
+ * QLA_DFS_SETUP_RD could be used when there is only a show function.
+ * - show function take the name qla_dfs_<sysfs-name>_show
+ *
+ * QLA_DFS_SETUP_RW could be used when there are both show and write functions.
+ * - show function take the name  qla_dfs_<sysfs-name>_show
+ * - write function take the name qla_dfs_<sysfs-name>_write
+ *
+ * To have a new debugfs entry, do:
+ * 1. Create a "struct dentry *" in the appropriate structure in the format
+ * dfs_<sysfs-name>
+ * 2. Setup debugfs entries using QLA_DFS_SETUP_RD / QLA_DFS_SETUP_RW
+ * 3. Create debugfs file in qla2x00_dfs_setup() using QLA_DFS_CREATE_FILE
+ * or QLA_DFS_ROOT_CREATE_FILE
+ * 4. Remove debugfs file in qla2x00_dfs_remove() using QLA_DFS_REMOVE_FILE
+ * or QLA_DFS_ROOT_REMOVE_FILE
+ *
+ * Example for creating "TEST" sysfs file:
+ * 1. struct qla_hw_data { ... struct dentry *dfs_TEST; }
+ * 2. QLA_DFS_SETUP_RD(TEST, scsi_qla_host_t);
+ * 3. In qla2x00_dfs_setup():
+ * QLA_DFS_CREATE_FILE(ha, TEST, 0600, ha->dfs_dir, vha);
+ * 4. In qla2x00_dfs_remove():
+ * QLA_DFS_REMOVE_FILE(ha, TEST);
+ */
+#define QLA_DFS_SETUP_RD(_name, _ctx_struct)				\
+static int								\
+qla_dfs_##_name##_open(struct inode *inode, struct file *file)		\
+{									\
+	_ctx_struct *__ctx = inode->i_private;				\
+									\
+	return single_open(file, qla_dfs_##_name##_show, __ctx);	\
+}									\
+									\
+static const struct file_operations qla_dfs_##_name##_ops = {		\
+	.open           = qla_dfs_##_name##_open,			\
+	.read           = seq_read,					\
+	.llseek         = seq_lseek,					\
+	.release        = single_release,				\
+};
+
+#define QLA_DFS_SETUP_RW(_name, _ctx_struct)		 		\
+static int								\
+qla_dfs_##_name##_open(struct inode *inode, struct file *file)		\
+{									\
+	_ctx_struct *__ctx = inode->i_private;				\
+									\
+	return single_open(file, qla_dfs_##_name##_show, __ctx);	\
+}									\
+									\
+static const struct file_operations qla_dfs_##_name##_ops = {		\
+	.open           = qla_dfs_##_name##_open,			\
+	.read           = seq_read,					\
+	.llseek         = seq_lseek,					\
+	.release        = single_release,				\
+	.write		= qla_dfs_##_name##_write,			\
+};
+
+#define QLA_DFS_ROOT_CREATE_FILE(_name, _perm, _ctx)			\
+	do {								\
+		if (!qla_dfs_##_name)					\
+			qla_dfs_##_name = debugfs_create_file(#_name,	\
+					_perm, qla2x00_dfs_root, _ctx,	\
+					&qla_dfs_##_name##_ops);	\
+	} while (0)
+
+#define QLA_DFS_ROOT_REMOVE_FILE(_name)					\
+	do {								\
+		if (qla_dfs_##_name) {					\
+			debugfs_remove(qla_dfs_##_name);		\
+			qla_dfs_##_name = NULL;				\
+		}							\
+	} while (0)
+
+#define QLA_DFS_CREATE_FILE(_struct, _name, _perm, _parent, _ctx)	\
+	(_struct)->dfs_##_name = debugfs_create_file(#_name, _perm,	\
+				_parent, _ctx, &qla_dfs_##_name##_ops);
+
+#define QLA_DFS_REMOVE_FILE(_struct, _name)				\
+	do {								\
+		if ((_struct)->dfs_##_name) {				\
+			debugfs_remove((_struct)->dfs_##_name);		\
+			(_struct)->dfs_##_name = NULL;			\
+		}							\
+	} while (0)
+
 static int
 qla_dfs_naqp_open(struct inode *inode, struct file *file)
 {
-- 
2.19.0.rc0

[PATCH 2/6] qla2xxx: Add a generic tracing framework

[Nilesh Javali]

From: Arun Easi <aeasi@marvell.com>

Adding a message based tracing mechanism where
a rotating number of messages are captured in
a trace structure. Disable/enable/resize
operations are allowed via debugfs interfaces.

Signed-off-by: Arun Easi <aeasi@marvell.com>
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_dbg.c |  12 +++
 drivers/scsi/qla2xxx/qla_dbg.h | 140 +++++++++++++++++++++++++++++++++
 drivers/scsi/qla2xxx/qla_def.h |  31 ++++++++
 drivers/scsi/qla2xxx/qla_dfs.c | 102 ++++++++++++++++++++++++
 drivers/scsi/qla2xxx/qla_os.c  |   5 ++
 5 files changed, 290 insertions(+)

diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c
index 7cf1f78cbaee..c4ba8ac51d27 100644
--- a/drivers/scsi/qla2xxx/qla_dbg.c
+++ b/drivers/scsi/qla2xxx/qla_dbg.c
@@ -2777,3 +2777,15 @@ ql_dbg_qp(uint32_t level, struct qla_qpair *qpair, int32_t id,
 	va_end(va);
 
 }
+
+#ifdef QLA_TRACING
+void qla_tracing_init(void)
+{
+	if (is_kdump_kernel())
+		return;
+}
+
+void qla_tracing_exit(void)
+{
+}
+#endif /* QLA_TRACING */
diff --git a/drivers/scsi/qla2xxx/qla_dbg.h b/drivers/scsi/qla2xxx/qla_dbg.h
index feeb1666227f..e0d91a1f81c0 100644
--- a/drivers/scsi/qla2xxx/qla_dbg.h
+++ b/drivers/scsi/qla2xxx/qla_dbg.h
@@ -5,6 +5,7 @@
  */
 
 #include "qla_def.h"
+#include <linux/delay.h>
 
 /*
  * Firmware Dump structure definition
@@ -321,6 +322,145 @@ struct qla2xxx_fw_dump {
 
 extern uint ql_errlev;
 
+#ifdef QLA_TRACING
+#include <linux/crash_dump.h>
+
+extern void qla_tracing_init(void);
+extern void qla_tracing_exit(void);
+
+static inline int
+ql_mask_match_ext(uint level, int *log_tunable)
+{
+	if (*log_tunable == 1)
+		*log_tunable = QL_DBG_DEFAULT1_MASK;
+
+	return (level & *log_tunable) == level;
+}
+
+static inline int
+__qla_trace_get(struct qla_trace *trc)
+{
+	if (test_bit(QLA_TRACE_QUIESCE, &trc->flags))
+		return -EIO;
+	atomic_inc(&trc->ref_count);
+	return 0;
+}
+
+static inline int
+qla_trace_get(struct qla_trace *trc)
+{
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&trc->trc_lock, flags);
+	ret = __qla_trace_get(trc);
+	spin_unlock_irqrestore(&trc->trc_lock, flags);
+
+	return ret;
+}
+
+static inline void
+qla_trace_put(struct qla_trace *trc)
+{
+	wmb();
+	atomic_dec(&trc->ref_count);
+}
+
+static inline char *
+qla_get_trace_next(struct qla_trace *trc)
+{
+	uint32_t t_ind;
+	char *buf;
+	unsigned long flags;
+
+	spin_lock_irqsave(&trc->trc_lock, flags);
+	if (!test_bit(QLA_TRACE_ENABLED, &trc->flags) ||
+	    __qla_trace_get(trc)) {
+		spin_unlock_irqrestore(&trc->trc_lock, flags);
+		return NULL;
+	}
+	t_ind = trc->trace_ind = qla_trace_ind_norm(trc, trc->trace_ind + 1);
+	spin_unlock_irqrestore(&trc->trc_lock, flags);
+
+	if (!t_ind)
+		set_bit(QLA_TRACE_WRAPPED, &trc->flags);
+
+	buf = qla_trace_record(trc, t_ind);
+	/* Put an end marker '>' for the next record. */
+	qla_trace_record(trc, qla_trace_ind_norm(trc, t_ind + 1))[0] = '>';
+
+	return buf;
+}
+
+static inline int
+qla_trace_quiesce(struct qla_trace *trc)
+{
+	unsigned long flags;
+	u32 cnt = 0;
+	int ret = 0;
+
+	set_bit(QLA_TRACE_QUIESCE, &trc->flags);
+
+	spin_lock_irqsave(&trc->trc_lock, flags);
+	while (atomic_read(&trc->ref_count)) {
+		spin_unlock_irqrestore(&trc->trc_lock, flags);
+
+		msleep(1);
+
+		spin_lock_irqsave(&trc->trc_lock, flags);
+		cnt++;
+		if (cnt > 10 * 1000) {
+			pr_info("qla2xxx: Trace could not be quiesced now (count=%d).",
+				atomic_read(&trc->ref_count));
+			/* Leave trace enabled */
+			clear_bit(QLA_TRACE_QUIESCE, &trc->flags);
+			ret = -EIO;
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&trc->trc_lock, flags);
+	return ret;
+}
+
+static inline void
+qla_trace_init(struct qla_trace *trc, char *name, u32 num_entries)
+{
+	if (trc->recs)
+		return;
+
+	memset(trc, 0, sizeof(*trc));
+
+	trc->name = name;
+	spin_lock_init(&trc->trc_lock);
+	if (!num_entries)
+		return;
+	trc->num_entries = num_entries;
+	trc->recs = vzalloc(trc->num_entries *
+				sizeof(struct qla_trace_rec));
+	if (!trc->recs)
+		return;
+
+	set_bit(QLA_TRACE_ENABLED, &trc->flags);
+}
+
+static inline void
+qla_trace_uninit(struct qla_trace *trc)
+{
+	if (!trc->recs)
+		return;
+
+	vfree(trc->recs);
+	trc->recs = NULL;
+	clear_bit(QLA_TRACE_ENABLED, &trc->flags);
+}
+
+#else /* QLA_TRACING */
+#define qla_trace_init(trc, name, num)
+#define qla_trace_uninit(trc)
+#define qla_tracing_init()
+#define qla_tracing_exit()
+#endif /* QLA_TRACING */
+
 void __attribute__((format (printf, 4, 5)))
 ql_dbg(uint, scsi_qla_host_t *vha, uint, const char *fmt, ...);
 void __attribute__((format (printf, 4, 5)))
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index 22274b405d01..39322105e7be 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -35,6 +35,37 @@
 
 #include <uapi/scsi/fc/fc_els.h>
 
+#define QLA_TRACING /* Captures driver messages to buffer */
+
+#ifdef QLA_TRACING
+#define QLA_TRACE_LINE_SIZE		256	/* Biggest so far is ~215 */
+#define qla_trace_ind_norm(_trc, _ind)	((_ind) >= (_trc)->num_entries ? \
+							0 : (_ind))
+#define qla_trace_record(_trc, __ind)	((_trc)->recs[__ind].buf)
+#define qla_trace_record_len		(sizeof(struct qla_trace_rec))
+#define qla_trace_start(_trc)		qla_trace_record(_trc, 0)
+#define qla_trace_len(_trc)		((_trc)->num_entries)
+#define qla_trace_size(_trc)		(qla_trace_record_len * \
+						(_trc)->num_entries)
+#define qla_trace_cur_ind(_trc)		((_trc)->trace_ind)
+struct qla_trace_rec {
+	char buf[QLA_TRACE_LINE_SIZE];
+};
+
+struct qla_trace {
+#define QLA_TRACE_ENABLED	0 /* allow trace writes or not */
+#define QLA_TRACE_WRAPPED	1
+#define QLA_TRACE_QUIESCE	2
+	unsigned long	flags;
+	atomic_t	ref_count;
+	u32		num_entries;
+	u32		trace_ind;
+	spinlock_t	trc_lock;
+	char		*name;
+	struct qla_trace_rec *recs;
+};
+#endif /* QLA_TRACING */
+
 #define QLA_DFS_DEFINE_DENTRY(_debugfs_file_name) \
 	struct dentry *dfs_##_debugfs_file_name
 #define QLA_DFS_ROOT_DEFINE_DENTRY(_debugfs_file_name) \
diff --git a/drivers/scsi/qla2xxx/qla_dfs.c b/drivers/scsi/qla2xxx/qla_dfs.c
index c3c8b9536ef6..98c6390ad1f1 100644
--- a/drivers/scsi/qla2xxx/qla_dfs.c
+++ b/drivers/scsi/qla2xxx/qla_dfs.c
@@ -489,6 +489,108 @@ qla_dfs_naqp_show(struct seq_file *s, void *unused)
 	return 0;
 }
 
+#ifdef QLA_TRACING
+static char *trace_help = "\
+# Format:\n\
+#   <msec> <cpu#> <message>\n\
+#\n\
+# Trace control by writing:\n\
+#   'enable' - to enable this trace\n\
+#   'disable' - to disable this trace\n\
+#   'resize=<nlines>' - to resize this trace to <nlines>\n\
+#\n";
+
+static int
+qla_dfs_trace_show(struct seq_file *s, void *unused)
+{
+	struct qla_trace *trc = s->private;
+	char *buf;
+	u32 t_ind = 0, i;
+
+	seq_puts(s, trace_help);
+
+	if (qla_trace_get(trc))
+		return 0;
+
+	seq_printf(s, "# Trace max lines = %d, writes = %s\n#\n",
+		   trc->num_entries, test_bit(QLA_TRACE_ENABLED,
+		   &trc->flags) ? "enabled" : "disabled");
+
+	if (test_bit(QLA_TRACE_WRAPPED, &trc->flags))
+		t_ind = qla_trace_cur_ind(trc) + 1;
+
+	for (i = 0; i < qla_trace_len(trc); i++, t_ind++) {
+		t_ind = qla_trace_ind_norm(trc, t_ind);
+		buf = qla_trace_record(trc, t_ind);
+		if (!buf[0])
+			continue;
+		seq_puts(s, buf);
+	}
+
+	mb();
+	qla_trace_put(trc);
+	return 0;
+}
+
+#define string_is(_buf, _str_val) \
+	(strncmp(_str_val, _buf, strlen(_str_val)) == 0)
+
+static ssize_t
+qla_dfs_trace_write(struct file *file, const char __user *buffer,
+		    size_t count, loff_t *pos)
+{
+	struct seq_file *s = file->private_data;
+	struct qla_trace *trc = s->private;
+	char buf[32];
+	ssize_t ret = count;
+
+	memset(buf, 0, sizeof(buf));
+	if (copy_from_user(buf, buffer, min(sizeof(buf), count)))
+		return -EFAULT;
+
+	if (string_is(buf, "enable")) {
+		if (!trc->recs) {
+			pr_warn("qla2xxx: '%s' is empty, resize before enabling.\n",
+					trc->name);
+			return -EINVAL;
+		}
+		pr_info("qla2xxx: Enabling trace '%s'\n", trc->name);
+		set_bit(QLA_TRACE_ENABLED, &trc->flags);
+	} else if (string_is(buf, "disable")) {
+		pr_info("qla2xxx: Disabling trace '%s'\n", trc->name);
+		clear_bit(QLA_TRACE_ENABLED, &trc->flags);
+	} else if (string_is(buf, "resize")) {
+		u32 new_len;
+
+		if (sscanf(buf, "resize=%d", &new_len) != 1)
+			return -EINVAL;
+		if (new_len == trc->num_entries) {
+			pr_info("qla2xxx: New trace size is same as old.\n");
+			return count;
+		}
+		pr_info("qla2xxx: Changing trace '%s' size to %d\n",
+			trc->name, new_len);
+		if (qla_trace_quiesce(trc)) {
+			ret = -EBUSY;
+			goto done;
+		}
+		qla_trace_uninit(trc);
+		/*
+		 * Go through init once again to start creating traces
+		 * based on the respective tunable.
+		 */
+		qla_trace_init(trc, trc->name, new_len);
+		if (!trc->recs) {
+			pr_warn("qla2xxx: Trace allocation failed for '%s'\n",
+				trc->name);
+			ret = -ENOMEM;
+		}
+	}
+done:
+	return ret;
+}
+#endif /* QLA_TRACING */
+
 /*
  * Helper macros for setting up debugfs entries.
  * _name: The name of the debugfs entry
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 0bd0fd1042df..0d2397069cac 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -8191,6 +8191,8 @@ qla2x00_module_init(void)
 	BUILD_BUG_ON(sizeof(sw_info_t) != 32);
 	BUILD_BUG_ON(sizeof(target_id_t) != 2);
 
+	qla_tracing_init();
+
 	/* Allocate cache for SRBs. */
 	srb_cachep = kmem_cache_create("qla2xxx_srbs", sizeof(srb_t), 0,
 	    SLAB_HWCACHE_ALIGN, NULL);
@@ -8269,6 +8271,7 @@ qla2x00_module_init(void)
 
 destroy_cache:
 	kmem_cache_destroy(srb_cachep);
+	qla_tracing_exit();
 	return ret;
 }
 
@@ -8287,6 +8290,8 @@ qla2x00_module_exit(void)
 	fc_release_transport(qla2xxx_transport_template);
 	qlt_exit();
 	kmem_cache_destroy(srb_cachep);
+
+	qla_tracing_exit();
 }
 
 module_init(qla2x00_module_init);
-- 
2.19.0.rc0

[PATCH 3/6] qla2xxx: Add driver console messages tracing

[Nilesh Javali]

From: Arun Easi <aeasi@marvell.com>

Driver messages, using the default log settings of 1 are captured
by default in an internal trace buffer.
Traces can be read from:
	/sys/kernel/debug/qla2xxx/message_trace

Enable/disable/resize operations are possible by writing
enable/disable/resize=<nlines> to the debugfs file.

Signed-off-by: Arun Easi <aeasi@marvell.com>
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_dbg.c | 14 +++++-
 drivers/scsi/qla2xxx/qla_dbg.h | 79 ++++++++++++++++++++++++++++++++++
 drivers/scsi/qla2xxx/qla_dfs.c | 30 +++++++++++++
 drivers/scsi/qla2xxx/qla_gbl.h |  1 +
 drivers/scsi/qla2xxx/qla_os.c  |  2 +
 5 files changed, 125 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c
index c4ba8ac51d27..3d12a5bf54b9 100644
--- a/drivers/scsi/qla2xxx/qla_dbg.c
+++ b/drivers/scsi/qla2xxx/qla_dbg.c
@@ -68,7 +68,7 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/qla.h>
 
-static uint32_t ql_dbg_offset = 0x800;
+uint32_t ql_dbg_offset = 0x800;
 
 static inline void
 qla2xxx_prep_dump(struct qla_hw_data *ha, struct qla2xxx_fw_dump *fw_dump)
@@ -2491,6 +2491,8 @@ ql_dbg(uint level, scsi_qla_host_t *vha, uint id, const char *fmt, ...)
 	struct va_format vaf;
 	char pbuf[64];
 
+	ql_msg_trace(1, level, vha, NULL, id, fmt);
+
 	if (!ql_mask_match(level) && !trace_ql_dbg_log_enabled())
 		return;
 
@@ -2533,6 +2535,9 @@ ql_dbg_pci(uint level, struct pci_dev *pdev, uint id, const char *fmt, ...)
 
 	if (pdev == NULL)
 		return;
+
+	ql_msg_trace(1, level, NULL, pdev, id, fmt);
+
 	if (!ql_mask_match(level))
 		return;
 
@@ -2570,6 +2575,8 @@ ql_log(uint level, scsi_qla_host_t *vha, uint id, const char *fmt, ...)
 	if (level > ql_errlev)
 		return;
 
+	ql_msg_trace(0, level, vha, NULL, id, fmt);
+
 	ql_dbg_prefix(pbuf, ARRAY_SIZE(pbuf), vha, id);
 
 	va_start(va, fmt);
@@ -2621,6 +2628,8 @@ ql_log_pci(uint level, struct pci_dev *pdev, uint id, const char *fmt, ...)
 	if (level > ql_errlev)
 		return;
 
+	ql_msg_trace(0, level, NULL, pdev, id, fmt);
+
 	ql_dbg_prefix(pbuf, ARRAY_SIZE(pbuf), NULL, id);
 
 	va_start(va, fmt);
@@ -2783,9 +2792,12 @@ void qla_tracing_init(void)
 {
 	if (is_kdump_kernel())
 		return;
+
+	qla_trace_init(&qla_message_trace, "message_trace", ql2xnum_msg_trace);
 }
 
 void qla_tracing_exit(void)
 {
+	qla_trace_uninit(&qla_message_trace);
 }
 #endif /* QLA_TRACING */
diff --git a/drivers/scsi/qla2xxx/qla_dbg.h b/drivers/scsi/qla2xxx/qla_dbg.h
index e0d91a1f81c0..9704cb13aacf 100644
--- a/drivers/scsi/qla2xxx/qla_dbg.h
+++ b/drivers/scsi/qla2xxx/qla_dbg.h
@@ -325,6 +325,24 @@ extern uint ql_errlev;
 #ifdef QLA_TRACING
 #include <linux/crash_dump.h>
 
+#define QLA_MTRC_DEF_NUM_REC		(4*1024) /* Has to be power of 2 */
+#define QLA_MESSAGE_TRACE_DEFINES \
+	struct qla_trace qla_message_trace;	\
+	int ql2xextended_error_logging_msg_trace = 1; \
+	module_param(ql2xextended_error_logging_msg_trace, int, 0600); \
+	MODULE_PARM_DESC(ql2xextended_error_logging_msg_trace, \
+		"Option to log console messages to buffer; uses same " \
+			"ql2xextended_error_logging masks."); \
+	\
+	int ql2xnum_msg_trace = QLA_MTRC_DEF_NUM_REC;	\
+	module_param(ql2xnum_msg_trace, int, 0600);	\
+	MODULE_PARM_DESC(ql2xnum_msg_trace, \
+		"Number of trace entries in power of 2. (default 4k)");
+
+extern int ql2xnum_msg_trace;
+extern int ql2xextended_error_logging_msg_trace;
+
+extern struct qla_trace qla_message_trace;
 extern void qla_tracing_init(void);
 extern void qla_tracing_exit(void);
 
@@ -422,6 +440,61 @@ qla_trace_quiesce(struct qla_trace *trc)
 	return ret;
 }
 
+#define ql_msg_trace(dbg_msg, level, vha, pdev, id, fmt) do {		\
+	struct va_format _vaf;						\
+	va_list _va;							\
+	u32 dbg_off = dbg_msg ? ql_dbg_offset : 0;			\
+									\
+	if (!test_bit(QLA_TRACE_ENABLED, &qla_message_trace.flags))	\
+		break;							\
+									\
+	if (dbg_msg && !ql_mask_match_ext(level,			\
+				&ql2xextended_error_logging_msg_trace))	\
+		break;							\
+									\
+	va_start(_va, fmt);						\
+									\
+	_vaf.fmt = fmt;							\
+	_vaf.va = &_va;							\
+	__ql_msg_trace(&qla_message_trace, vha, pdev,			\
+						id + dbg_off, &_vaf);	\
+									\
+	va_end(_va);							\
+} while (0)
+
+/* Messages beyond QLA_TRACE_LINE_SIZE characters are not printed */
+static inline void
+__ql_msg_trace(struct qla_trace *trc, scsi_qla_host_t *vha,
+		struct pci_dev *pdev, uint id, struct va_format *vaf)
+{
+	int tl;
+	char *buf;
+	u64 t_us = ktime_to_us(ktime_get());
+	uint cpu = raw_smp_processor_id();
+
+	buf = qla_get_trace_next(trc);
+	if (!buf)
+		return;
+
+	if (vha) {
+		const struct pci_dev *_pdev = vha->hw->pdev;
+		tl = snprintf(buf, QLA_TRACE_LINE_SIZE,
+			"%12llu %03u %s [%s]-%04x:%ld: %pV", t_us, cpu,
+			QL_MSGHDR, dev_name(&(_pdev->dev)), id,
+			vha->host_no, vaf);
+	} else {
+		tl = snprintf(buf, QLA_TRACE_LINE_SIZE,
+			"%12llu %03u %s [%s]-%04x: : %pV", t_us, cpu, QL_MSGHDR,
+			pdev ? dev_name(&(pdev->dev)) : "0000:00:00.0",
+			id, vaf);
+	}
+
+	tl = min(tl, QLA_TRACE_LINE_SIZE - 1);
+	buf[tl] = '\0';
+
+	qla_trace_put(trc);
+}
+
 static inline void
 qla_trace_init(struct qla_trace *trc, char *name, u32 num_entries)
 {
@@ -455,10 +528,16 @@ qla_trace_uninit(struct qla_trace *trc)
 }
 
 #else /* QLA_TRACING */
+#define ql_msg_trace(dbg_msg, level, vha, pdev, id, fmt) do { } while (0)
 #define qla_trace_init(trc, name, num)
 #define qla_trace_uninit(trc)
 #define qla_tracing_init()
 #define qla_tracing_exit()
+#define QLA_MESSAGE_TRACE_DEFINES
+
+#define ql_srb_trace_ext(_level, _vha, _fcport, _fmt, _args...) do { } while (0)
+#define ql_srb_trace(_level, _vha, _fmt, _args...) do { } while (0)
+#define QLA_SRB_TRACE_DEFINES
 #endif /* QLA_TRACING */
 
 void __attribute__((format (printf, 4, 5)))
diff --git a/drivers/scsi/qla2xxx/qla_dfs.c b/drivers/scsi/qla2xxx/qla_dfs.c
index 98c6390ad1f1..d3f9f6af43f2 100644
--- a/drivers/scsi/qla2xxx/qla_dfs.c
+++ b/drivers/scsi/qla2xxx/qla_dfs.c
@@ -11,6 +11,10 @@
 static struct dentry *qla2x00_dfs_root;
 static atomic_t qla2x00_dfs_root_count;
 
+#ifdef QLA_TRACING
+static QLA_DFS_ROOT_DEFINE_DENTRY(message_trace); /* qla_dfs_message_trace */
+#endif /* QLA_TRACING */
+
 #define QLA_DFS_RPORT_DEVLOSS_TMO	1
 
 static int
@@ -589,6 +593,18 @@ qla_dfs_trace_write(struct file *file, const char __user *buffer,
 done:
 	return ret;
 }
+
+static int
+qla_dfs_message_trace_show(struct seq_file *s, void *unused)
+{
+	return qla_dfs_trace_show(s, unused);
+}
+static ssize_t
+qla_dfs_message_trace_write(struct file *file, const char __user *buffer,
+	size_t count, loff_t *pos)
+{
+	return qla_dfs_trace_write(file, buffer, count, pos);
+}
 #endif /* QLA_TRACING */
 
 /*
@@ -681,6 +697,10 @@ static const struct file_operations qla_dfs_##_name##_ops = {		\
 		}							\
 	} while (0)
 
+#ifdef QLA_TRACING
+QLA_DFS_SETUP_RW(message_trace, struct qla_trace *);
+#endif /* QLA_TRACING */
+
 static int
 qla_dfs_naqp_open(struct inode *inode, struct file *file)
 {
@@ -788,6 +808,11 @@ qla2x00_dfs_setup(scsi_qla_host_t *vha)
 	ha->tgt.dfs_tgt_sess = debugfs_create_file("tgt_sess",
 		S_IRUSR, ha->dfs_dir, vha, &qla2x00_dfs_tgt_sess_fops);
 
+#ifdef QLA_TRACING
+	QLA_DFS_ROOT_CREATE_FILE(message_trace, 0600, &qla_message_trace);
+
+#endif /* QLA_TRACING */
+
 	if (IS_QLA27XX(ha) || IS_QLA83XX(ha) || IS_QLA28XX(ha)) {
 		ha->tgt.dfs_naqp = debugfs_create_file("naqp",
 		    0400, ha->dfs_dir, vha, &dfs_naqp_ops);
@@ -847,6 +872,11 @@ qla2x00_dfs_remove(scsi_qla_host_t *vha)
 		vha->dfs_rport_root = NULL;
 	}
 
+#ifdef QLA_TRACING
+	QLA_DFS_ROOT_REMOVE_FILE(message_trace);
+
+#endif /* QLA_TRACING */
+
 	if (ha->dfs_dir) {
 		debugfs_remove(ha->dfs_dir);
 		ha->dfs_dir = NULL;
diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h
index 5dd2932382ee..8c149a3482cb 100644
--- a/drivers/scsi/qla2xxx/qla_gbl.h
+++ b/drivers/scsi/qla2xxx/qla_gbl.h
@@ -163,6 +163,7 @@ extern int ql2xrdpenable;
 extern int ql2xsmartsan;
 extern int ql2xallocfwdump;
 extern int ql2xextended_error_logging;
+extern uint32_t ql_dbg_offset;
 extern int ql2xiidmaenable;
 extern int ql2xmqsupport;
 extern int ql2xfwloadbin;
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 0d2397069cac..b937283f5e53 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -117,6 +117,8 @@ MODULE_PARM_DESC(ql2xextended_error_logging,
 		"ql2xextended_error_logging=1).\n"
 		"\t\tDo LOGICAL OR of the value to enable more than one level");
 
+QLA_MESSAGE_TRACE_DEFINES;
+
 int ql2xshiftctondsd = 6;
 module_param(ql2xshiftctondsd, int, S_IRUGO);
 MODULE_PARM_DESC(ql2xshiftctondsd,
-- 
2.19.0.rc0

[PATCH 4/6] qla2xxx: Add srb tracing

[Nilesh Javali]

From: Arun Easi <aeasi@marvell.com>

Add a separate driver internal trace to capture srb
related info. The debugfs file is:
	/sys/kernel/debug/qla2xxx/qla2xxx_<host#>/srb_trace

Like message trace, enable/disable/resize operations are possible
by writing:
            enable/disable/resize=<nlines>
..to the debugfs file.

Signed-off-by: Arun Easi <aeasi@marvell.com>
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_dbg.h    | 67 +++++++++++++++++++++++++++++++
 drivers/scsi/qla2xxx/qla_def.h    | 17 +++++---
 drivers/scsi/qla2xxx/qla_dfs.c    | 17 ++++++++
 drivers/scsi/qla2xxx/qla_inline.h | 12 ++++++
 drivers/scsi/qla2xxx/qla_iocb.c   |  4 ++
 drivers/scsi/qla2xxx/qla_isr.c    |  5 +++
 drivers/scsi/qla2xxx/qla_os.c     | 28 +++++++++++++
 7 files changed, 144 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_dbg.h b/drivers/scsi/qla2xxx/qla_dbg.h
index 9704cb13aacf..056fbdbc6794 100644
--- a/drivers/scsi/qla2xxx/qla_dbg.h
+++ b/drivers/scsi/qla2xxx/qla_dbg.h
@@ -342,6 +342,22 @@ extern uint ql_errlev;
 extern int ql2xnum_msg_trace;
 extern int ql2xextended_error_logging_msg_trace;
 
+#define QLA_SRB_TRACE_DEFINES \
+	int ql2xextended_error_logging_srb_trace = 1; \
+	module_param(ql2xextended_error_logging_srb_trace, int, \
+						S_IRUGO|S_IWUSR); \
+	MODULE_PARM_DESC(ql2xextended_error_logging_srb_trace, \
+		"Option to log srb messages to buffer; uses same " \
+			"ql2xextended_error_logging masks."); \
+	\
+	int ql2xnum_srb_trace = 0;	\
+	module_param(ql2xnum_srb_trace, int, S_IRUGO);	\
+	MODULE_PARM_DESC(ql2xnum_srb_trace, \
+		"Number of srb trace entries in power of 2. (default 0)");
+
+extern int ql2xnum_srb_trace;
+extern int ql2xextended_error_logging_srb_trace;
+
 extern struct qla_trace qla_message_trace;
 extern void qla_tracing_init(void);
 extern void qla_tracing_exit(void);
@@ -495,6 +511,57 @@ __ql_msg_trace(struct qla_trace *trc, scsi_qla_host_t *vha,
 	qla_trace_put(trc);
 }
 
+#define ql_srb_trace_ext(_level, _vha, _fcport, _fmt, _args...) do {	\
+	if (_fcport) {							\
+		__ql_srb_trace(_level, _vha,				\
+			DBG_FCPORT_PRFMT(_fcport, _fmt, ##_args));	\
+	} else {							\
+		__ql_srb_trace(_level, _vha,				\
+			"%s: " _fmt "\n", __func__, ##_args);		\
+	}								\
+} while (0)
+
+#define ql_srb_trace(_level, _vha, _fmt, _args...) \
+	__ql_srb_trace(_level, _vha, _fmt, ##_args)
+
+static inline void
+__ql_srb_trace(int level, scsi_qla_host_t *vha, const char *fmt, ...)
+{
+	int tl;
+	char *buf;
+	u64 t_us;
+	uint cpu;
+	struct va_format vaf;
+	va_list va;
+
+	if (!test_bit(QLA_TRACE_ENABLED, &vha->hw->srb_trace.flags))
+		return;
+
+	if (!ql_mask_match_ext(level, &ql2xextended_error_logging_srb_trace))
+		return;
+
+	t_us = ktime_to_us(ktime_get());
+	cpu = raw_smp_processor_id();
+	buf = qla_get_trace_next(&vha->hw->srb_trace);
+	if (!buf)
+		return;
+
+	va_start(va, fmt);
+
+	vaf.fmt = fmt;
+	vaf.va = &va;
+
+	tl = snprintf(buf, QLA_TRACE_LINE_SIZE, "%12llu %03u %pV",
+			t_us, cpu, &vaf);
+
+	tl = min(tl, QLA_TRACE_LINE_SIZE - 1);
+	buf[tl] = '\0';
+
+	qla_trace_put(&vha->hw->srb_trace);
+
+	va_end(va);
+}
+
 static inline void
 qla_trace_init(struct qla_trace *trc, char *name, u32 num_entries)
 {
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index 39322105e7be..e1528a4e6ee4 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -4793,6 +4793,11 @@ struct qla_hw_data {
 	spinlock_t sadb_lock;	/* protects list */
 	struct els_reject elsrej;
 	u8 edif_post_stop_cnt_down;
+
+#ifdef QLA_TRACING
+	QLA_DFS_DEFINE_DENTRY(srb_trace);
+	struct qla_trace srb_trace;
+#endif /* QLA_TRACING */
 };
 
 #define RX_ELS_SIZE (roundup(sizeof(struct enode) + ELS_MAX_PAYLOAD, SMP_CACHE_BYTES))
@@ -5493,6 +5498,12 @@ struct ql_vnd_tgt_stats_resp {
 	struct ql_vnd_stats stats;
 } __packed;
 
+#define DBG_FCPORT_PRFMT(_fp, _fmt, _args...) \
+	"%s: %8phC: " _fmt " (state=%d disc_state=%d scan_state=%d loopid=0x%x deleted=%d flags=0x%x)\n", \
+	__func__, _fp->port_name, ##_args, atomic_read(&_fp->state), \
+	_fp->disc_state, _fp->scan_state, _fp->loop_id, _fp->deleted, \
+	_fp->flags
+
 #include "qla_target.h"
 #include "qla_gbl.h"
 #include "qla_dbg.h"
@@ -5501,10 +5512,4 @@ struct ql_vnd_tgt_stats_resp {
 #define IS_SESSION_DELETED(_fcport) (_fcport->disc_state == DSC_DELETE_PEND || \
 				      _fcport->disc_state == DSC_DELETED)
 
-#define DBG_FCPORT_PRFMT(_fp, _fmt, _args...) \
-	"%s: %8phC: " _fmt " (state=%d disc_state=%d scan_state=%d loopid=0x%x deleted=%d flags=0x%x)\n", \
-	__func__, _fp->port_name, ##_args, atomic_read(&_fp->state), \
-	_fp->disc_state, _fp->scan_state, _fp->loop_id, _fp->deleted, \
-	_fp->flags
-
 #endif
diff --git a/drivers/scsi/qla2xxx/qla_dfs.c b/drivers/scsi/qla2xxx/qla_dfs.c
index d3f9f6af43f2..806cb4afcb30 100644
--- a/drivers/scsi/qla2xxx/qla_dfs.c
+++ b/drivers/scsi/qla2xxx/qla_dfs.c
@@ -599,12 +599,26 @@ qla_dfs_message_trace_show(struct seq_file *s, void *unused)
 {
 	return qla_dfs_trace_show(s, unused);
 }
+
 static ssize_t
 qla_dfs_message_trace_write(struct file *file, const char __user *buffer,
 	size_t count, loff_t *pos)
 {
 	return qla_dfs_trace_write(file, buffer, count, pos);
 }
+
+static int
+qla_dfs_srb_trace_show(struct seq_file *s, void *unused)
+{
+	return qla_dfs_trace_show(s, unused);
+}
+
+static ssize_t
+qla_dfs_srb_trace_write(struct file *file, const char __user *buffer,
+	size_t count, loff_t *pos)
+{
+	return qla_dfs_trace_write(file, buffer, count, pos);
+}
 #endif /* QLA_TRACING */
 
 /*
@@ -699,6 +713,7 @@ static const struct file_operations qla_dfs_##_name##_ops = {		\
 
 #ifdef QLA_TRACING
 QLA_DFS_SETUP_RW(message_trace, struct qla_trace *);
+QLA_DFS_SETUP_RW(srb_trace, struct qla_trace *);
 #endif /* QLA_TRACING */
 
 static int
@@ -811,6 +826,7 @@ qla2x00_dfs_setup(scsi_qla_host_t *vha)
 #ifdef QLA_TRACING
 	QLA_DFS_ROOT_CREATE_FILE(message_trace, 0600, &qla_message_trace);
 
+	QLA_DFS_CREATE_FILE(ha, srb_trace, 0600, ha->dfs_dir, &ha->srb_trace);
 #endif /* QLA_TRACING */
 
 	if (IS_QLA27XX(ha) || IS_QLA83XX(ha) || IS_QLA28XX(ha)) {
@@ -875,6 +891,7 @@ qla2x00_dfs_remove(scsi_qla_host_t *vha)
 #ifdef QLA_TRACING
 	QLA_DFS_ROOT_REMOVE_FILE(message_trace);
 
+	QLA_DFS_REMOVE_FILE(ha, srb_trace);
 #endif /* QLA_TRACING */
 
 	if (ha->dfs_dir) {
diff --git a/drivers/scsi/qla2xxx/qla_inline.h b/drivers/scsi/qla2xxx/qla_inline.h
index db17f7f410cd..19f334693a38 100644
--- a/drivers/scsi/qla2xxx/qla_inline.h
+++ b/drivers/scsi/qla2xxx/qla_inline.h
@@ -201,6 +201,10 @@ qla2xxx_get_qpair_sp(scsi_qla_host_t *vha, struct qla_qpair *qpair,
 		return NULL;
 
 	sp = mempool_alloc(qpair->srb_mempool, flag);
+	/* Avoid trace for calls from qla2x00_get_sp */
+	if (vha->hw->base_qpair != qpair)
+		ql_srb_trace_ext(ql_dbg_io, vha, fcport,
+			"sp=%px", sp);
 	if (sp)
 		qla2xxx_init_sp(sp, vha, qpair, fcport);
 	else
@@ -214,6 +218,10 @@ void qla2xxx_rel_free_warning(srb_t *sp);
 static inline void
 qla2xxx_rel_qpair_sp(struct qla_qpair *qpair, srb_t *sp)
 {
+	/* Avoid trace for calls from qla2x00_get_sp */
+	if (qpair->vha->hw->base_qpair != qpair)
+		ql_srb_trace_ext(ql_dbg_io, sp->vha, sp->fcport,
+			"sp=%px type=%d", sp, sp->type);
 	sp->qpair = NULL;
 	sp->done = qla2xxx_rel_done_warning;
 	sp->free = qla2xxx_rel_free_warning;
@@ -234,6 +242,7 @@ qla2x00_get_sp(scsi_qla_host_t *vha, fc_port_t *fcport, gfp_t flag)
 
 	qpair = vha->hw->base_qpair;
 	sp = qla2xxx_get_qpair_sp(vha, qpair, fcport, flag);
+	ql_srb_trace_ext(ql_dbg_disc, vha, fcport, "sp=%px", sp);
 	if (!sp)
 		goto done;
 
@@ -247,6 +256,9 @@ qla2x00_get_sp(scsi_qla_host_t *vha, fc_port_t *fcport, gfp_t flag)
 static inline void
 qla2x00_rel_sp(srb_t *sp)
 {
+	ql_srb_trace_ext(ql_dbg_disc, sp->vha, sp->fcport,
+		"sp=%px type=%d", sp, sp->type);
+
 	QLA_VHA_MARK_NOT_BUSY(sp->vha);
 	qla2xxx_rel_qpair_sp(sp->qpair, sp);
 }
diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
index 42ce4e1fe744..4fe569ef27ae 100644
--- a/drivers/scsi/qla2xxx/qla_iocb.c
+++ b/drivers/scsi/qla2xxx/qla_iocb.c
@@ -3826,6 +3826,10 @@ qla2x00_start_sp(srb_t *sp)
 	if (vha->hw->flags.eeh_busy)
 		return -EIO;
 
+	ql_srb_trace_ext(ql_dbg_disc, vha, sp->fcport,
+			"caller=%ps sp=%px type=%d",
+			__builtin_return_address(0), sp, sp->type);
+
 	spin_lock_irqsave(qp->qp_lock_ptr, flags);
 	pkt = __qla2x00_alloc_iocbs(sp->qpair, sp);
 	if (!pkt) {
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index 76e79f350a22..af368d3beeab 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -3197,6 +3197,11 @@ qla2x00_status_entry(scsi_qla_host_t *vha, struct rsp_que *rsp, void *pkt)
 		}
 		return;
 	}
+
+	ql_srb_trace_ext(ql_dbg_io, vha, sp->fcport,
+			 "sp=%px handle=0x%x type=%d done=%ps",
+			 sp, sp->handle, sp->type, sp->done);
+
 	qla_put_iocbs(sp->qpair, &sp->iores);
 
 	if (sp->cmd_type != TYPE_SRB) {
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index b937283f5e53..4f9d2c1680c1 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -119,6 +119,8 @@ MODULE_PARM_DESC(ql2xextended_error_logging,
 
 QLA_MESSAGE_TRACE_DEFINES;
 
+QLA_SRB_TRACE_DEFINES;
+
 int ql2xshiftctondsd = 6;
 module_param(ql2xshiftctondsd, int, S_IRUGO);
 MODULE_PARM_DESC(ql2xshiftctondsd,
@@ -752,6 +754,10 @@ void qla2x00_sp_compl(srb_t *sp, int res)
 	struct scsi_cmnd *cmd = GET_CMD_SP(sp);
 	struct completion *comp = sp->comp;
 
+	ql_srb_trace_ext(ql_dbg_io, sp->vha, sp->fcport,
+			 "sp=%px handle=0x%x cmd=%px res=%x",
+			 sp, sp->handle, cmd, res);
+
 	/* kref: INIT */
 	kref_put(&sp->cmd_kref, qla2x00_sp_release);
 	cmd->result = res;
@@ -844,6 +850,10 @@ void qla2xxx_qpair_sp_compl(srb_t *sp, int res)
 	struct scsi_cmnd *cmd = GET_CMD_SP(sp);
 	struct completion *comp = sp->comp;
 
+	ql_srb_trace_ext(ql_dbg_io, sp->vha, sp->fcport,
+			 "sp=%px handle=0x%x cmd=%px res=%x",
+			 sp, sp->handle, cmd, res);
+
 	/* ref: INIT */
 	kref_put(&sp->cmd_kref, qla2x00_sp_release);
 	cmd->result = res;
@@ -870,6 +880,10 @@ qla2xxx_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
 		goto qc24_fail_command;
 	}
 
+	ql_srb_trace_ext(ql_dbg_io, fcport->vha, fcport,
+		"cmd=%px mq=%d remchk=0x%x",
+		cmd, ha->mqenable, fc_remote_port_chkready(rport));
+
 	if (ha->mqenable) {
 		uint32_t tag;
 		uint16_t hwq;
@@ -877,6 +891,10 @@ qla2xxx_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
 
 		tag = blk_mq_unique_tag(scsi_cmd_to_rq(cmd));
 		hwq = blk_mq_unique_tag_to_hwq(tag);
+
+		ql_srb_trace_ext(ql_dbg_io, fcport->vha, fcport,
+				 "tag=%px hwq=%d", tag, hwq);
+
 		qpair = ha->queue_pair_map[hwq];
 
 		if (qpair)
@@ -1298,6 +1316,10 @@ qla2xxx_eh_abort(struct scsi_cmnd *cmd)
 
 	vha->cmd_timeout_cnt++;
 
+	ql_srb_trace_ext(ql_dbg_io, sp->vha, sp->fcport,
+			 "sp=%px cmd=%px fast_fail_sts=0x%x",
+			 sp, cmd, fast_fail_status);
+
 	if ((sp->fcport && sp->fcport->deleted) || !qpair)
 		return fast_fail_status != SUCCESS ? fast_fail_status : FAILED;
 
@@ -3528,6 +3550,8 @@ qla2x00_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	ql_dbg(ql_dbg_init, base_vha, 0x00f2,
 	    "Init done and hba is online.\n");
 
+	qla_trace_init(&ha->srb_trace, "srb_trace", ql2xnum_srb_trace);
+
 	if (qla_ini_mode_enabled(base_vha) ||
 		qla_dual_mode_enabled(base_vha))
 		scsi_scan_host(host);
@@ -3600,6 +3624,8 @@ qla2x00_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	rsp = NULL;
 
 probe_hw_failed:
+	qla_trace_uninit(&ha->srb_trace);
+
 	qla2x00_mem_free(ha);
 	qla2x00_free_req_que(ha, req);
 	qla2x00_free_rsp_que(ha, rsp);
@@ -3883,6 +3909,8 @@ qla2x00_remove_one(struct pci_dev *pdev)
 
 	qla2x00_delete_all_vps(ha, base_vha);
 
+	qla_trace_uninit(&ha->srb_trace);
+
 	qla2x00_dfs_remove(base_vha);
 
 	qla84xx_put_chip(base_vha);
-- 
2.19.0.rc0

[PATCH 5/6] qla2xxx: Add NVMe parameters support in Auxiliary Image Status

[Nilesh Javali]

From: Anil Gurumurthy <agurumurthy@marvell.com>

Add new API to obtain the NVMe Parameters region status from the
Auxiliary Image Status bitmap.

Signed-off-by: Anil Gurumurthy <agurumurthy@marvell.com>
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_bsg.c  | 8 ++++++--
 drivers/scsi/qla2xxx/qla_bsg.h  | 3 ++-
 drivers/scsi/qla2xxx/qla_def.h  | 2 ++
 drivers/scsi/qla2xxx/qla_fw.h   | 3 +++
 drivers/scsi/qla2xxx/qla_init.c | 8 ++++++--
 5 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
index 5db9bf69dcff..cd75b179410d 100644
--- a/drivers/scsi/qla2xxx/qla_bsg.c
+++ b/drivers/scsi/qla2xxx/qla_bsg.c
@@ -2519,19 +2519,23 @@ qla2x00_get_flash_image_status(struct bsg_job *bsg_job)
 	qla27xx_get_active_image(vha, &active_regions);
 	regions.global_image = active_regions.global;
 
+	if (IS_QLA27XX(ha))
+		regions.nvme_params = QLA27XX_PRIMARY_IMAGE;
+
 	if (IS_QLA28XX(ha)) {
 		qla28xx_get_aux_images(vha, &active_regions);
 		regions.board_config = active_regions.aux.board_config;
 		regions.vpd_nvram = active_regions.aux.vpd_nvram;
 		regions.npiv_config_0_1 = active_regions.aux.npiv_config_0_1;
 		regions.npiv_config_2_3 = active_regions.aux.npiv_config_2_3;
+		regions.nvme_params = active_regions.aux.nvme_params;
 	}
 
 	ql_dbg(ql_dbg_user, vha, 0x70e1,
-	    "%s(%lu): FW=%u BCFG=%u VPDNVR=%u NPIV01=%u NPIV02=%u\n",
+	    "%s(%lu): FW=%u BCFG=%u VPDNVR=%u NPIV01=%u NPIV02=%u NVME_PARAMS=%u\n",
 	    __func__, vha->host_no, regions.global_image,
 	    regions.board_config, regions.vpd_nvram,
-	    regions.npiv_config_0_1, regions.npiv_config_2_3);
+	    regions.npiv_config_0_1, regions.npiv_config_2_3, regions.nvme_params);
 
 	sg_copy_from_buffer(bsg_job->reply_payload.sg_list,
 	    bsg_job->reply_payload.sg_cnt, &regions, sizeof(regions));
diff --git a/drivers/scsi/qla2xxx/qla_bsg.h b/drivers/scsi/qla2xxx/qla_bsg.h
index bb64b9c5a74b..d38dab0a07e8 100644
--- a/drivers/scsi/qla2xxx/qla_bsg.h
+++ b/drivers/scsi/qla2xxx/qla_bsg.h
@@ -314,7 +314,8 @@ struct qla_active_regions {
 	uint8_t vpd_nvram;
 	uint8_t npiv_config_0_1;
 	uint8_t npiv_config_2_3;
-	uint8_t reserved[32];
+	uint8_t nvme_params;
+	uint8_t reserved[31];
 } __packed;
 
 #include "qla_edif_bsg.h"
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index e1528a4e6ee4..994b2378e24a 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -4809,6 +4809,7 @@ struct active_regions {
 		uint8_t vpd_nvram;
 		uint8_t npiv_config_0_1;
 		uint8_t npiv_config_2_3;
+		uint8_t nvme_params;
 	} aux;
 };
 
@@ -5093,6 +5094,7 @@ struct qla27xx_image_status {
 #define QLA28XX_AUX_IMG_VPD_NVRAM		BIT_1
 #define QLA28XX_AUX_IMG_NPIV_CONFIG_0_1		BIT_2
 #define QLA28XX_AUX_IMG_NPIV_CONFIG_2_3		BIT_3
+#define QLA28XX_AUX_IMG_NVME_PARAMS		BIT_4
 
 #define SET_VP_IDX	1
 #define SET_AL_PA	2
diff --git a/drivers/scsi/qla2xxx/qla_fw.h b/drivers/scsi/qla2xxx/qla_fw.h
index 361015b5763e..f307beed9d29 100644
--- a/drivers/scsi/qla2xxx/qla_fw.h
+++ b/drivers/scsi/qla2xxx/qla_fw.h
@@ -1675,6 +1675,7 @@ struct qla_flt_location {
 #define FLT_REG_VPD_SEC_27XX_1	0x52
 #define FLT_REG_VPD_SEC_27XX_2	0xD8
 #define FLT_REG_VPD_SEC_27XX_3	0xDA
+#define FLT_REG_NVME_PARAMS_27XX	0x21
 
 /* 28xx */
 #define FLT_REG_AUX_IMG_PRI_28XX	0x125
@@ -1691,6 +1692,8 @@ struct qla_flt_location {
 #define FLT_REG_MPI_SEC_28XX		0xF0
 #define FLT_REG_PEP_PRI_28XX		0xD1
 #define FLT_REG_PEP_SEC_28XX		0xF1
+#define FLT_REG_NVME_PARAMS_PRI_28XX	0x14E
+#define FLT_REG_NVME_PARAMS_SEC_28XX	0x179
 
 struct qla_flt_region {
 	__le16	code;
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index e7fe0e52c11d..e12db95de688 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -7933,6 +7933,9 @@ qla28xx_component_status(
 
 	active_regions->aux.npiv_config_2_3 =
 	    qla28xx_component_bitmask(aux, QLA28XX_AUX_IMG_NPIV_CONFIG_2_3);
+
+	active_regions->aux.nvme_params =
+	    qla28xx_component_bitmask(aux, QLA28XX_AUX_IMG_NVME_PARAMS);
 }
 
 static int
@@ -8041,11 +8044,12 @@ qla28xx_get_aux_images(
 	}
 
 	ql_dbg(ql_dbg_init, vha, 0x018f,
-	    "aux images active: BCFG=%u VPD/NVR=%u NPIV0/1=%u NPIV2/3=%u\n",
+	    "aux images active: BCFG=%u VPD/NVR=%u NPIV0/1=%u NPIV2/3=%u, NVME=%u\n",
 	    active_regions->aux.board_config,
 	    active_regions->aux.vpd_nvram,
 	    active_regions->aux.npiv_config_0_1,
-	    active_regions->aux.npiv_config_2_3);
+	    active_regions->aux.npiv_config_2_3,
+	    active_regions->aux.nvme_params);
 }
 
 void
-- 
2.19.0.rc0

[PATCH 6/6] qla2xxx: Update version to 10.02.07.900-k

[Nilesh Javali]

Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_version.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_version.h b/drivers/scsi/qla2xxx/qla_version.h
index f3257d46b6d2..03f3e2cd62b5 100644
--- a/drivers/scsi/qla2xxx/qla_version.h
+++ b/drivers/scsi/qla2xxx/qla_version.h
@@ -6,9 +6,9 @@
 /*
  * Driver version
  */
-#define QLA2XXX_VERSION      "10.02.07.800-k"
+#define QLA2XXX_VERSION      "10.02.07.900-k"
 
 #define QLA_DRIVER_MAJOR_VER	10
 #define QLA_DRIVER_MINOR_VER	2
 #define QLA_DRIVER_PATCH_VER	7
-#define QLA_DRIVER_BETA_VER	800
+#define QLA_DRIVER_BETA_VER	900
-- 
2.19.0.rc0

Re: [PATCH 2/6] qla2xxx: Add a generic tracing framework

[Daniel Wagner]

Hi

[adding Steven Rostedt]

On Thu, Jul 14, 2022 at 11:02:23PM -0700, Nilesh Javali wrote:
> From: Arun Easi <aeasi@marvell.com>
> 
> Adding a message based tracing mechanism where
> a rotating number of messages are captured in
> a trace structure. Disable/enable/resize
> operations are allowed via debugfs interfaces.

I really wonder why you need to this kind of infrastructure to your
driver? As far I know the qla2xxx is already able to log into
ftrace. Why can't we just use this infrastructure?

Thanks,
Daniel

> 
> Signed-off-by: Arun Easi <aeasi@marvell.com>
> Signed-off-by: Nilesh Javali <njavali@marvell.com>
> ---
>  drivers/scsi/qla2xxx/qla_dbg.c |  12 +++
>  drivers/scsi/qla2xxx/qla_dbg.h | 140 +++++++++++++++++++++++++++++++++
>  drivers/scsi/qla2xxx/qla_def.h |  31 ++++++++
>  drivers/scsi/qla2xxx/qla_dfs.c | 102 ++++++++++++++++++++++++
>  drivers/scsi/qla2xxx/qla_os.c  |   5 ++
>  5 files changed, 290 insertions(+)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c
> index 7cf1f78cbaee..c4ba8ac51d27 100644
> --- a/drivers/scsi/qla2xxx/qla_dbg.c
> +++ b/drivers/scsi/qla2xxx/qla_dbg.c
> @@ -2777,3 +2777,15 @@ ql_dbg_qp(uint32_t level, struct qla_qpair *qpair, int32_t id,
>  	va_end(va);
>  
>  }
> +
> +#ifdef QLA_TRACING
> +void qla_tracing_init(void)
> +{
> +	if (is_kdump_kernel())
> +		return;
> +}
> +
> +void qla_tracing_exit(void)
> +{
> +}
> +#endif /* QLA_TRACING */
> diff --git a/drivers/scsi/qla2xxx/qla_dbg.h b/drivers/scsi/qla2xxx/qla_dbg.h
> index feeb1666227f..e0d91a1f81c0 100644
> --- a/drivers/scsi/qla2xxx/qla_dbg.h
> +++ b/drivers/scsi/qla2xxx/qla_dbg.h
> @@ -5,6 +5,7 @@
>   */
>  
>  #include "qla_def.h"
> +#include <linux/delay.h>
>  
>  /*
>   * Firmware Dump structure definition
> @@ -321,6 +322,145 @@ struct qla2xxx_fw_dump {
>  
>  extern uint ql_errlev;
>  
> +#ifdef QLA_TRACING
> +#include <linux/crash_dump.h>
> +
> +extern void qla_tracing_init(void);
> +extern void qla_tracing_exit(void);
> +
> +static inline int
> +ql_mask_match_ext(uint level, int *log_tunable)
> +{
> +	if (*log_tunable == 1)
> +		*log_tunable = QL_DBG_DEFAULT1_MASK;
> +
> +	return (level & *log_tunable) == level;
> +}
> +
> +static inline int
> +__qla_trace_get(struct qla_trace *trc)
> +{
> +	if (test_bit(QLA_TRACE_QUIESCE, &trc->flags))
> +		return -EIO;
> +	atomic_inc(&trc->ref_count);
> +	return 0;
> +}
> +
> +static inline int
> +qla_trace_get(struct qla_trace *trc)
> +{
> +	unsigned long flags;
> +	int ret;
> +
> +	spin_lock_irqsave(&trc->trc_lock, flags);
> +	ret = __qla_trace_get(trc);
> +	spin_unlock_irqrestore(&trc->trc_lock, flags);
> +
> +	return ret;
> +}
> +
> +static inline void
> +qla_trace_put(struct qla_trace *trc)
> +{
> +	wmb();
> +	atomic_dec(&trc->ref_count);
> +}
> +
> +static inline char *
> +qla_get_trace_next(struct qla_trace *trc)
> +{
> +	uint32_t t_ind;
> +	char *buf;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&trc->trc_lock, flags);
> +	if (!test_bit(QLA_TRACE_ENABLED, &trc->flags) ||
> +	    __qla_trace_get(trc)) {
> +		spin_unlock_irqrestore(&trc->trc_lock, flags);
> +		return NULL;
> +	}
> +	t_ind = trc->trace_ind = qla_trace_ind_norm(trc, trc->trace_ind + 1);
> +	spin_unlock_irqrestore(&trc->trc_lock, flags);
> +
> +	if (!t_ind)
> +		set_bit(QLA_TRACE_WRAPPED, &trc->flags);
> +
> +	buf = qla_trace_record(trc, t_ind);
> +	/* Put an end marker '>' for the next record. */
> +	qla_trace_record(trc, qla_trace_ind_norm(trc, t_ind + 1))[0] = '>';
> +
> +	return buf;
> +}
> +
> +static inline int
> +qla_trace_quiesce(struct qla_trace *trc)
> +{
> +	unsigned long flags;
> +	u32 cnt = 0;
> +	int ret = 0;
> +
> +	set_bit(QLA_TRACE_QUIESCE, &trc->flags);
> +
> +	spin_lock_irqsave(&trc->trc_lock, flags);
> +	while (atomic_read(&trc->ref_count)) {
> +		spin_unlock_irqrestore(&trc->trc_lock, flags);
> +
> +		msleep(1);
> +
> +		spin_lock_irqsave(&trc->trc_lock, flags);
> +		cnt++;
> +		if (cnt > 10 * 1000) {
> +			pr_info("qla2xxx: Trace could not be quiesced now (count=%d).",
> +				atomic_read(&trc->ref_count));
> +			/* Leave trace enabled */
> +			clear_bit(QLA_TRACE_QUIESCE, &trc->flags);
> +			ret = -EIO;
> +			break;
> +		}
> +	}
> +	spin_unlock_irqrestore(&trc->trc_lock, flags);
> +	return ret;
> +}
> +
> +static inline void
> +qla_trace_init(struct qla_trace *trc, char *name, u32 num_entries)
> +{
> +	if (trc->recs)
> +		return;
> +
> +	memset(trc, 0, sizeof(*trc));
> +
> +	trc->name = name;
> +	spin_lock_init(&trc->trc_lock);
> +	if (!num_entries)
> +		return;
> +	trc->num_entries = num_entries;
> +	trc->recs = vzalloc(trc->num_entries *
> +				sizeof(struct qla_trace_rec));
> +	if (!trc->recs)
> +		return;
> +
> +	set_bit(QLA_TRACE_ENABLED, &trc->flags);
> +}
> +
> +static inline void
> +qla_trace_uninit(struct qla_trace *trc)
> +{
> +	if (!trc->recs)
> +		return;
> +
> +	vfree(trc->recs);
> +	trc->recs = NULL;
> +	clear_bit(QLA_TRACE_ENABLED, &trc->flags);
> +}
> +
> +#else /* QLA_TRACING */
> +#define qla_trace_init(trc, name, num)
> +#define qla_trace_uninit(trc)
> +#define qla_tracing_init()
> +#define qla_tracing_exit()
> +#endif /* QLA_TRACING */
> +
>  void __attribute__((format (printf, 4, 5)))
>  ql_dbg(uint, scsi_qla_host_t *vha, uint, const char *fmt, ...);
>  void __attribute__((format (printf, 4, 5)))
> diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
> index 22274b405d01..39322105e7be 100644
> --- a/drivers/scsi/qla2xxx/qla_def.h
> +++ b/drivers/scsi/qla2xxx/qla_def.h
> @@ -35,6 +35,37 @@
>  
>  #include <uapi/scsi/fc/fc_els.h>
>  
> +#define QLA_TRACING /* Captures driver messages to buffer */
> +
> +#ifdef QLA_TRACING
> +#define QLA_TRACE_LINE_SIZE		256	/* Biggest so far is ~215 */
> +#define qla_trace_ind_norm(_trc, _ind)	((_ind) >= (_trc)->num_entries ? \
> +							0 : (_ind))
> +#define qla_trace_record(_trc, __ind)	((_trc)->recs[__ind].buf)
> +#define qla_trace_record_len		(sizeof(struct qla_trace_rec))
> +#define qla_trace_start(_trc)		qla_trace_record(_trc, 0)
> +#define qla_trace_len(_trc)		((_trc)->num_entries)
> +#define qla_trace_size(_trc)		(qla_trace_record_len * \
> +						(_trc)->num_entries)
> +#define qla_trace_cur_ind(_trc)		((_trc)->trace_ind)
> +struct qla_trace_rec {
> +	char buf[QLA_TRACE_LINE_SIZE];
> +};
> +
> +struct qla_trace {
> +#define QLA_TRACE_ENABLED	0 /* allow trace writes or not */
> +#define QLA_TRACE_WRAPPED	1
> +#define QLA_TRACE_QUIESCE	2
> +	unsigned long	flags;
> +	atomic_t	ref_count;
> +	u32		num_entries;
> +	u32		trace_ind;
> +	spinlock_t	trc_lock;
> +	char		*name;
> +	struct qla_trace_rec *recs;
> +};
> +#endif /* QLA_TRACING */
> +
>  #define QLA_DFS_DEFINE_DENTRY(_debugfs_file_name) \
>  	struct dentry *dfs_##_debugfs_file_name
>  #define QLA_DFS_ROOT_DEFINE_DENTRY(_debugfs_file_name) \
> diff --git a/drivers/scsi/qla2xxx/qla_dfs.c b/drivers/scsi/qla2xxx/qla_dfs.c
> index c3c8b9536ef6..98c6390ad1f1 100644
> --- a/drivers/scsi/qla2xxx/qla_dfs.c
> +++ b/drivers/scsi/qla2xxx/qla_dfs.c
> @@ -489,6 +489,108 @@ qla_dfs_naqp_show(struct seq_file *s, void *unused)
>  	return 0;
>  }
>  
> +#ifdef QLA_TRACING
> +static char *trace_help = "\
> +# Format:\n\
> +#   <msec> <cpu#> <message>\n\
> +#\n\
> +# Trace control by writing:\n\
> +#   'enable' - to enable this trace\n\
> +#   'disable' - to disable this trace\n\
> +#   'resize=<nlines>' - to resize this trace to <nlines>\n\
> +#\n";
> +
> +static int
> +qla_dfs_trace_show(struct seq_file *s, void *unused)
> +{
> +	struct qla_trace *trc = s->private;
> +	char *buf;
> +	u32 t_ind = 0, i;
> +
> +	seq_puts(s, trace_help);
> +
> +	if (qla_trace_get(trc))
> +		return 0;
> +
> +	seq_printf(s, "# Trace max lines = %d, writes = %s\n#\n",
> +		   trc->num_entries, test_bit(QLA_TRACE_ENABLED,
> +		   &trc->flags) ? "enabled" : "disabled");
> +
> +	if (test_bit(QLA_TRACE_WRAPPED, &trc->flags))
> +		t_ind = qla_trace_cur_ind(trc) + 1;
> +
> +	for (i = 0; i < qla_trace_len(trc); i++, t_ind++) {
> +		t_ind = qla_trace_ind_norm(trc, t_ind);
> +		buf = qla_trace_record(trc, t_ind);
> +		if (!buf[0])
> +			continue;
> +		seq_puts(s, buf);
> +	}
> +
> +	mb();
> +	qla_trace_put(trc);
> +	return 0;
> +}
> +
> +#define string_is(_buf, _str_val) \
> +	(strncmp(_str_val, _buf, strlen(_str_val)) == 0)
> +
> +static ssize_t
> +qla_dfs_trace_write(struct file *file, const char __user *buffer,
> +		    size_t count, loff_t *pos)
> +{
> +	struct seq_file *s = file->private_data;
> +	struct qla_trace *trc = s->private;
> +	char buf[32];
> +	ssize_t ret = count;
> +
> +	memset(buf, 0, sizeof(buf));
> +	if (copy_from_user(buf, buffer, min(sizeof(buf), count)))
> +		return -EFAULT;
> +
> +	if (string_is(buf, "enable")) {
> +		if (!trc->recs) {
> +			pr_warn("qla2xxx: '%s' is empty, resize before enabling.\n",
> +					trc->name);
> +			return -EINVAL;
> +		}
> +		pr_info("qla2xxx: Enabling trace '%s'\n", trc->name);
> +		set_bit(QLA_TRACE_ENABLED, &trc->flags);
> +	} else if (string_is(buf, "disable")) {
> +		pr_info("qla2xxx: Disabling trace '%s'\n", trc->name);
> +		clear_bit(QLA_TRACE_ENABLED, &trc->flags);
> +	} else if (string_is(buf, "resize")) {
> +		u32 new_len;
> +
> +		if (sscanf(buf, "resize=%d", &new_len) != 1)
> +			return -EINVAL;
> +		if (new_len == trc->num_entries) {
> +			pr_info("qla2xxx: New trace size is same as old.\n");
> +			return count;
> +		}
> +		pr_info("qla2xxx: Changing trace '%s' size to %d\n",
> +			trc->name, new_len);
> +		if (qla_trace_quiesce(trc)) {
> +			ret = -EBUSY;
> +			goto done;
> +		}
> +		qla_trace_uninit(trc);
> +		/*
> +		 * Go through init once again to start creating traces
> +		 * based on the respective tunable.
> +		 */
> +		qla_trace_init(trc, trc->name, new_len);
> +		if (!trc->recs) {
> +			pr_warn("qla2xxx: Trace allocation failed for '%s'\n",
> +				trc->name);
> +			ret = -ENOMEM;
> +		}
> +	}
> +done:
> +	return ret;
> +}
> +#endif /* QLA_TRACING */
> +
>  /*
>   * Helper macros for setting up debugfs entries.
>   * _name: The name of the debugfs entry
> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> index 0bd0fd1042df..0d2397069cac 100644
> --- a/drivers/scsi/qla2xxx/qla_os.c
> +++ b/drivers/scsi/qla2xxx/qla_os.c
> @@ -8191,6 +8191,8 @@ qla2x00_module_init(void)
>  	BUILD_BUG_ON(sizeof(sw_info_t) != 32);
>  	BUILD_BUG_ON(sizeof(target_id_t) != 2);
>  
> +	qla_tracing_init();
> +
>  	/* Allocate cache for SRBs. */
>  	srb_cachep = kmem_cache_create("qla2xxx_srbs", sizeof(srb_t), 0,
>  	    SLAB_HWCACHE_ALIGN, NULL);
> @@ -8269,6 +8271,7 @@ qla2x00_module_init(void)
>  
>  destroy_cache:
>  	kmem_cache_destroy(srb_cachep);
> +	qla_tracing_exit();
>  	return ret;
>  }
>  
> @@ -8287,6 +8290,8 @@ qla2x00_module_exit(void)
>  	fc_release_transport(qla2xxx_transport_template);
>  	qlt_exit();
>  	kmem_cache_destroy(srb_cachep);
> +
> +	qla_tracing_exit();
>  }
>  
>  module_init(qla2x00_module_init);
> -- 
> 2.19.0.rc0
> 

Re: [PATCH 2/6] qla2xxx: Add a generic tracing framework

[Arun Easi]

On Mon, 18 Jul 2022, 1:54am, Daniel Wagner wrote:

> Hi
> 
> [adding Steven Rostedt]
> 
> On Thu, Jul 14, 2022 at 11:02:23PM -0700, Nilesh Javali wrote:
> > From: Arun Easi <aeasi@marvell.com>
> > 
> > Adding a message based tracing mechanism where
> > a rotating number of messages are captured in
> > a trace structure. Disable/enable/resize
> > operations are allowed via debugfs interfaces.
> 
> I really wonder why you need to this kind of infrastructure to your
> driver? As far I know the qla2xxx is already able to log into
> ftrace. Why can't we just use this infrastructure?
> 

Many times when a problem was reported on the driver, we had to request 
for a repro with extended error logging (via driver module parameter) 
turned on. With this internal tracing in place, log messages that appear 
only with extended error logging are captured by default in the internal 
trace buffer.

AFAIK, kernel tracing requires a user initiated action to be turned on, 
like enabling individual tracepoints. Though a script (either startup or 
udev) can do this job, enabling tracepoints by default for a single 
driver, I think, may not be a preferred choice -- particularly when the 
trace buffer is shared across the kernel. That also brings the extra 
overhead of finding how this could be done across various distros.

For cases where the memory/driver size matters, there is an option to 
compile out this feature, plus choosing a lower default trace buffer size.

Regards,
-Arun

Re: [PATCH 2/6] qla2xxx: Add a generic tracing framework

[Steven Rostedt]

On Mon, 18 Jul 2022 12:02:26 -0700
Arun Easi <aeasi@marvell.com> wrote:

> Many times when a problem was reported on the driver, we had to request 
> for a repro with extended error logging (via driver module parameter) 
> turned on. With this internal tracing in place, log messages that appear 
> only with extended error logging are captured by default in the internal 
> trace buffer.
> 
> AFAIK, kernel tracing requires a user initiated action to be turned on, 
> like enabling individual tracepoints. Though a script (either startup or 
> udev) can do this job, enabling tracepoints by default for a single 
> driver, I think, may not be a preferred choice -- particularly when the 
> trace buffer is shared across the kernel. That also brings the extra 
> overhead of finding how this could be done across various distros.
> 
> For cases where the memory/driver size matters, there is an option to 
> compile out this feature, plus choosing a lower default trace buffer size.

You can enable an ftrace instance from inside the kernel, and make it a
compile time option.

	#include <linux/trace_events.h>
	#include <linux/trace.h>

	struct trace_array *tr;

	tr = trace_array_get_by_name("qla2xxx");
	trace_array_set_clr_event(tr, "qla", NULL, true);

And now you have trace events running:

 # cat /sys/kernel/tracing/instances/qla/trace

-- Steve

Re: [PATCH 2/6] qla2xxx: Add a generic tracing framework

[Daniel Wagner]

Hi Arun,

On Mon, Jul 18, 2022 at 03:22:43PM -0400, Steven Rostedt wrote:
> On Mon, 18 Jul 2022 12:02:26 -0700
> Arun Easi <aeasi@marvell.com> wrote:
> 
> > Many times when a problem was reported on the driver, we had to request 
> > for a repro with extended error logging (via driver module parameter) 
> > turned on. With this internal tracing in place, log messages that appear 
> > only with extended error logging are captured by default in the internal 
> > trace buffer.
> > 
> > AFAIK, kernel tracing requires a user initiated action to be turned on, 
> > like enabling individual tracepoints. Though a script (either startup or 
> > udev) can do this job, enabling tracepoints by default for a single 
> > driver, I think, may not be a preferred choice -- particularly when the 
> > trace buffer is shared across the kernel. That also brings the extra 
> > overhead of finding how this could be done across various distros.
> > 
> > For cases where the memory/driver size matters, there is an option to 
> > compile out this feature, plus choosing a lower default trace buffer size.

I am really asking the question why do we need to add special debugging
code to every single driver? Can't we try to make more use of generic
code and extend it if necessary?

Both FC drivers qla2xxx and lpfc are doing their own thing for
debugging/logging and I really fail to see why we can't not move towards
a more generic way. Dumping logs to the kernel log was the simplest way
but as this series shows, this is not something you can do in production
systems.

> You can enable an ftrace instance from inside the kernel, and make it a
> compile time option.
>
> 	#include <linux/trace_events.h>
> 	#include <linux/trace.h>
> 
> 	struct trace_array *tr;
> 
> 	tr = trace_array_get_by_name("qla2xxx");
> 	trace_array_set_clr_event(tr, "qla", NULL, true);
> 
> And now you have trace events running:
> 
>  # cat /sys/kernel/tracing/instances/qla/trace

Thanks Steve for the tip!

Daniel

Re: [PATCH 2/6] qla2xxx: Add a generic tracing framework

[Steven Rostedt]

On Tue, 19 Jul 2022 10:25:14 +0200
Daniel Wagner <dwagner@suse.de> wrote:

> > You can enable an ftrace instance from inside the kernel, and make it a
> > compile time option.
> >
> > 	#include <linux/trace_events.h>
> > 	#include <linux/trace.h>
> > 
> > 	struct trace_array *tr;
> > 
> > 	tr = trace_array_get_by_name("qla2xxx");
> > 	trace_array_set_clr_event(tr, "qla", NULL, true);
> > 
> > And now you have trace events running:
> > 
> >  # cat /sys/kernel/tracing/instances/qla/trace  

The above should be:

     # cat /sys/kernel/tracing/instances/qla2xxx/trace

as the instance name is the string sent to trace_array_get_by_name().

-- Steve

Re: [EXT] Re: [PATCH 2/6] qla2xxx: Add a generic tracing framework

[Arun Easi]

[-- Attachment #1: Type: text/plain, Size: 3074 bytes --]

On Mon, 18 Jul 2022, 12:22pm, Steven Rostedt wrote:

> External Email
> 
> ----------------------------------------------------------------------
> On Mon, 18 Jul 2022 12:02:26 -0700
> Arun Easi <aeasi@marvell.com> wrote:
> 
> > Many times when a problem was reported on the driver, we had to request 
> > for a repro with extended error logging (via driver module parameter) 
> > turned on. With this internal tracing in place, log messages that appear 
> > only with extended error logging are captured by default in the internal 
> > trace buffer.
> > 
> > AFAIK, kernel tracing requires a user initiated action to be turned on, 
> > like enabling individual tracepoints. Though a script (either startup or 
> > udev) can do this job, enabling tracepoints by default for a single 
> > driver, I think, may not be a preferred choice -- particularly when the 
> > trace buffer is shared across the kernel. That also brings the extra 
> > overhead of finding how this could be done across various distros.
> > 
> > For cases where the memory/driver size matters, there is an option to 
> > compile out this feature, plus choosing a lower default trace buffer size.
> 
> You can enable an ftrace instance from inside the kernel, and make it a
> compile time option.
> 
> 	#include <linux/trace_events.h>
> 	#include <linux/trace.h>
> 
> 	struct trace_array *tr;
> 
> 	tr = trace_array_get_by_name("qla2xxx");
> 	trace_array_set_clr_event(tr, "qla", NULL, true);
> 
> And now you have trace events running:
> 
>  # cat /sys/kernel/tracing/instances/qla/trace
> 

Thanks Steve. I was not aware of this relatively newer interface. This 
looks promising.

I have a question on the behavior of this interface.

It appears by calling the above two interfaces, I get a separate instance 
of "qla" only traces, but with only the "qla"-only instance being enabled, 
leaving the main (/sys/kernel/tracing/events/qla/enable) one disabled. No 
issues there, but when I enable both of them, I get garbage values on one.

/sys/kernel/tracing/instances/qla2xxx/trace:
             cat-56106   [012] ..... 2419873.470098: ql_dbg_log: qla2xxx [0000:05:00.0]-1054:14:  Entered (null).
             cat-56106   [012] ..... 2419873.470101: ql_dbg_log: qla2xxx [0000:05:00.0]-1000:14:  Entered ×+<96>²Ü<98>^H.
             cat-56106   [012] ..... 2419873.470102: ql_dbg_log: qla2xxx [0000:05:00.0]-1006:14:  Prepare to issue mbox cmd=0xde589000.

/sys/kernel/tracing/trace:
             cat-56106   [012] ..... 2419873.470097: ql_dbg_log: qla2xxx [0000:05:00.0]-1054:14:  Entered qla2x00_get_firmware_state.
             cat-56106   [012] ..... 2419873.470100: ql_dbg_log: qla2xxx [0000:05:00.0]-1000:14:  Entered qla2x00_mailbox_command.
             cat-56106   [012] ..... 2419873.470102: ql_dbg_log: qla2xxx [0000:05:00.0]-1006:14:  Prepare to issue mbox cmd=0x69.

It appears that only one should be enabled at a time. Per my read of 
Documentation/trace/ftrace.rst, the main directory and instances have 
separate trace buffers, so I am a bit confused with the above output.

Regards,
-Arun

Re: [EXT] Re: [PATCH 2/6] qla2xxx: Add a generic tracing framework

[Steven Rostedt]

On Tue, 19 Jul 2022 14:06:11 -0700
Arun Easi <aeasi@marvell.com> wrote:

> It appears by calling the above two interfaces, I get a separate instance 
> of "qla" only traces, but with only the "qla"-only instance being enabled, 
> leaving the main (/sys/kernel/tracing/events/qla/enable) one disabled. No 
> issues there, but when I enable both of them, I get garbage values on one.
> 
> /sys/kernel/tracing/instances/qla2xxx/trace:
>              cat-56106   [012] ..... 2419873.470098: ql_dbg_log: qla2xxx [0000:05:00.0]-1054:14:  Entered (null).
>              cat-56106   [012] ..... 2419873.470101: ql_dbg_log: qla2xxx [0000:05:00.0]-1000:14:  Entered ×+<96>²Ü<98>^H.
>              cat-56106   [012] ..... 2419873.470102: ql_dbg_log: qla2xxx [0000:05:00.0]-1006:14:  Prepare to issue mbox cmd=0xde589000.
> 
> /sys/kernel/tracing/trace:
>              cat-56106   [012] ..... 2419873.470097: ql_dbg_log: qla2xxx [0000:05:00.0]-1054:14:  Entered qla2x00_get_firmware_state.
>              cat-56106   [012] ..... 2419873.470100: ql_dbg_log: qla2xxx [0000:05:00.0]-1000:14:  Entered qla2x00_mailbox_command.
>              cat-56106   [012] ..... 2419873.470102: ql_dbg_log: qla2xxx [0000:05:00.0]-1006:14:  Prepare to issue mbox cmd=0x69.
> 
> It appears that only one should be enabled at a time. Per my read of 
> Documentation/trace/ftrace.rst, the main directory and instances have 
> separate trace buffers, so I am a bit confused with the above output.

That's because it uses va_list, and va_list can only be used once.

I just added helpers for va_list use cases:

   https://lore.kernel.org/all/20220705224453.120955146@goodmis.org/

But it will likely suffer the same issue, but I can easily fix that with
this on top:

Not even compiled tested:

-- Steve

diff --git a/include/trace/stages/stage6_event_callback.h b/include/trace/stages/stage6_event_callback.h
index 0f51f6b3ab70..3c554a585320 100644
--- a/include/trace/stages/stage6_event_callback.h
+++ b/include/trace/stages/stage6_event_callback.h
@@ -40,7 +40,12 @@
 
 #undef __assign_vstr
 #define __assign_vstr(dst, fmt, va)					\
-	vsnprintf(__get_str(dst), TRACE_EVENT_STR_MAX, fmt, *(va))
+	do {								\
+		va_list __cp_va;					\
+		va_copy(__cp_va, *(va));				\
+		vsnprintf(__get_str(dst), TRACE_EVENT_STR_MAX, fmt, __cp_va); \
+		va_end(__cp_va);					\
+	} while (0)
 
 #undef __bitmask
 #define __bitmask(item, nr_bits) __dynamic_array(unsigned long, item, -1)

Re: [EXT] Re: [PATCH 2/6] qla2xxx: Add a generic tracing framework

[Arun Easi]

[-- Attachment #1: Type: text/plain, Size: 2939 bytes --]

On Tue, 19 Jul 2022, 2:40pm, Steven Rostedt wrote:

> On Tue, 19 Jul 2022 14:06:11 -0700
> Arun Easi <aeasi@marvell.com> wrote:
> 
> > It appears by calling the above two interfaces, I get a separate instance 
> > of "qla" only traces, but with only the "qla"-only instance being enabled, 
> > leaving the main (/sys/kernel/tracing/events/qla/enable) one disabled. No 
> > issues there, but when I enable both of them, I get garbage values on one.
> > 
> > /sys/kernel/tracing/instances/qla2xxx/trace:
> >              cat-56106   [012] ..... 2419873.470098: ql_dbg_log: qla2xxx [0000:05:00.0]-1054:14:  Entered (null).
> >              cat-56106   [012] ..... 2419873.470101: ql_dbg_log: qla2xxx [0000:05:00.0]-1000:14:  Entered ×+<96>²Ü<98>^H.
> >              cat-56106   [012] ..... 2419873.470102: ql_dbg_log: qla2xxx [0000:05:00.0]-1006:14:  Prepare to issue mbox cmd=0xde589000.
> > 
> > /sys/kernel/tracing/trace:
> >              cat-56106   [012] ..... 2419873.470097: ql_dbg_log: qla2xxx [0000:05:00.0]-1054:14:  Entered qla2x00_get_firmware_state.
> >              cat-56106   [012] ..... 2419873.470100: ql_dbg_log: qla2xxx [0000:05:00.0]-1000:14:  Entered qla2x00_mailbox_command.
> >              cat-56106   [012] ..... 2419873.470102: ql_dbg_log: qla2xxx [0000:05:00.0]-1006:14:  Prepare to issue mbox cmd=0x69.
> > 
> > It appears that only one should be enabled at a time. Per my read of 
> > Documentation/trace/ftrace.rst, the main directory and instances have 
> > separate trace buffers, so I am a bit confused with the above output.
> 
> That's because it uses va_list, and va_list can only be used once.

Ah, that makes sense.

> 
> I just added helpers for va_list use cases:
> 
>    https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_all_20220705224453.120955146-40goodmis.org_&d=DwIFaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=P-q_Qkt75qFy33SvdD2nAxAyN87eO1d-mFO-lqNOomw&m=cDBXba_lNDGk7c1Qm4elDe1Co-RV8zR-c1A9xV7vc4nMcQO7iRle72qNxljHsDNA&s=74PzgMj6nmfRCLXd3oMIqt-_DZbQeukUl0nK7MzfUjs&e= 
> 
> But it will likely suffer the same issue, but I can easily fix that with
> this on top:
> 
> Not even compiled tested:
> 
> -- Steve
> 
> diff --git a/include/trace/stages/stage6_event_callback.h b/include/trace/stages/stage6_event_callback.h
> index 0f51f6b3ab70..3c554a585320 100644
> --- a/include/trace/stages/stage6_event_callback.h
> +++ b/include/trace/stages/stage6_event_callback.h
> @@ -40,7 +40,12 @@
>  
>  #undef __assign_vstr
>  #define __assign_vstr(dst, fmt, va)					\
> -	vsnprintf(__get_str(dst), TRACE_EVENT_STR_MAX, fmt, *(va))
> +	do {								\
> +		va_list __cp_va;					\
> +		va_copy(__cp_va, *(va));				\
> +		vsnprintf(__get_str(dst), TRACE_EVENT_STR_MAX, fmt, __cp_va); \
> +		va_end(__cp_va);					\
> +	} while (0)
>  
>  #undef __bitmask
>  #define __bitmask(item, nr_bits) __dynamic_array(unsigned long, item, -1)
> 

This will be nice.

Thanks for your help, Steve.

Regards,
-Arun

Re: [EXT] Re: [PATCH 2/6] qla2xxx: Add a generic tracing framework

[Arun Easi]

Hi Daniel,

On Tue, 19 Jul 2022, 1:25am, Daniel Wagner wrote:

> External Email
> 
> ----------------------------------------------------------------------
> Hi Arun,
> 
> On Mon, Jul 18, 2022 at 03:22:43PM -0400, Steven Rostedt wrote:
> > On Mon, 18 Jul 2022 12:02:26 -0700
> > Arun Easi <aeasi@marvell.com> wrote:
> > 
> > > Many times when a problem was reported on the driver, we had to request 
> > > for a repro with extended error logging (via driver module parameter) 
> > > turned on. With this internal tracing in place, log messages that appear 
> > > only with extended error logging are captured by default in the internal 
> > > trace buffer.
> > > 
> > > AFAIK, kernel tracing requires a user initiated action to be turned on, 
> > > like enabling individual tracepoints. Though a script (either startup or 
> > > udev) can do this job, enabling tracepoints by default for a single 
> > > driver, I think, may not be a preferred choice -- particularly when the 
> > > trace buffer is shared across the kernel. That also brings the extra 
> > > overhead of finding how this could be done across various distros.
> > > 
> > > For cases where the memory/driver size matters, there is an option to 
> > > compile out this feature, plus choosing a lower default trace buffer size.
> 
> I am really asking the question why do we need to add special debugging
> code to every single driver? Can't we try to make more use of generic
> code and extend it if necessary?
> 
> Both FC drivers qla2xxx and lpfc are doing their own thing for
> debugging/logging and I really fail to see why we can't not move towards
> a more generic way. Dumping logs to the kernel log was the simplest way
> but as this series shows, this is not something you can do in production
> systems.

No contention here on a generic way. The per instance trace creation and 
enabling from within the kernel looks like such a one. Let me revisit the 
trace patches with this new info.

Regards,
-Arun

> 
> > You can enable an ftrace instance from inside the kernel, and make it a
> > compile time option.
> >
> > 	#include <linux/trace_events.h>
> > 	#include <linux/trace.h>
> > 
> > 	struct trace_array *tr;
> > 
> > 	tr = trace_array_get_by_name("qla2xxx");
> > 	trace_array_set_clr_event(tr, "qla", NULL, true);
> > 
> > And now you have trace events running:
> > 
> >  # cat /sys/kernel/tracing/instances/qla/trace
> 
> Thanks Steve for the tip!
> 
> Daniel
> 

Re: [EXT] Re: [PATCH 2/6] qla2xxx: Add a generic tracing framework

[Steven Rostedt]

On Tue, 19 Jul 2022 15:09:30 -0700
Arun Easi <aeasi@marvell.com> wrote:

> This will be nice.
> 
> Thanks for your help, Steve.

No problem.

What a coincidence that I just finished this work earlier this month.

-- Steve

Re: [PATCH 2/6] qla2xxx: Add a generic tracing framework

[kernel test robot]

Hi Nilesh,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on f095c3cd1b694a73a5de276dae919f05a8dd1811]

url:    https://github.com/intel-lab-lkp/linux/commits/Nilesh-Javali/qla2xxx-driver-features/20220715-140608
base:   f095c3cd1b694a73a5de276dae919f05a8dd1811
config: mips-allyesconfig (https://download.01.org/0day-ci/archive/20220720/202207201230.esVUm6qp-lkp@intel.com/config)
compiler: mips-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/d5f3010956e8a08bd2742acc3715478d1b961178
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Nilesh-Javali/qla2xxx-driver-features/20220715-140608
        git checkout d5f3010956e8a08bd2742acc3715478d1b961178
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/scsi/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

Note: the linux-review/Nilesh-Javali/qla2xxx-driver-features/20220715-140608 HEAD d044ae5809919c7fbaa3ca3c374d1dfd00403990 builds fine.
      It only hurts bisectability.

All error/warnings (new ones prefixed by >>):

   In file included from drivers/scsi/qla2xxx/qla_def.h:5498,
                    from drivers/scsi/qla2xxx/qla_os.c:6:
   drivers/scsi/qla2xxx/qla_dbg.h: In function 'qla_trace_init':
>> drivers/scsi/qla2xxx/qla_dbg.h:438:21: error: implicit declaration of function 'vzalloc'; did you mean 'kvzalloc'? [-Werror=implicit-function-declaration]
     438 |         trc->recs = vzalloc(trc->num_entries *
         |                     ^~~~~~~
         |                     kvzalloc
>> drivers/scsi/qla2xxx/qla_dbg.h:438:19: warning: assignment to 'struct qla_trace_rec *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     438 |         trc->recs = vzalloc(trc->num_entries *
         |                   ^
   drivers/scsi/qla2xxx/qla_dbg.h: In function 'qla_trace_uninit':
>> drivers/scsi/qla2xxx/qla_dbg.h:452:9: error: implicit declaration of function 'vfree'; did you mean 'kvfree'? [-Werror=implicit-function-declaration]
     452 |         vfree(trc->recs);
         |         ^~~~~
         |         kvfree
   In file included from drivers/scsi/qla2xxx/qla_os.c:9:
   include/linux/vmalloc.h: At top level:
>> include/linux/vmalloc.h:143:14: error: conflicting types for 'vzalloc'; have 'void *(long unsigned int)'
     143 | extern void *vzalloc(unsigned long size) __alloc_size(1);
         |              ^~~~~~~
   drivers/scsi/qla2xxx/qla_dbg.h:438:21: note: previous implicit declaration of 'vzalloc' with type 'int()'
     438 |         trc->recs = vzalloc(trc->num_entries *
         |                     ^~~~~~~
>> include/linux/vmalloc.h:163:13: warning: conflicting types for 'vfree'; have 'void(const void *)'
     163 | extern void vfree(const void *addr);
         |             ^~~~~
   drivers/scsi/qla2xxx/qla_dbg.h:452:9: note: previous implicit declaration of 'vfree' with type 'void(const void *)'
     452 |         vfree(trc->recs);
         |         ^~~~~
   cc1: some warnings being treated as errors
--
   In file included from drivers/scsi/qla2xxx/qla_def.h:5498,
                    from drivers/scsi/qla2xxx/qla_init.c:6:
   drivers/scsi/qla2xxx/qla_dbg.h: In function 'qla_trace_init':
>> drivers/scsi/qla2xxx/qla_dbg.h:438:21: error: implicit declaration of function 'vzalloc'; did you mean 'kvzalloc'? [-Werror=implicit-function-declaration]
     438 |         trc->recs = vzalloc(trc->num_entries *
         |                     ^~~~~~~
         |                     kvzalloc
>> drivers/scsi/qla2xxx/qla_dbg.h:438:19: warning: assignment to 'struct qla_trace_rec *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     438 |         trc->recs = vzalloc(trc->num_entries *
         |                   ^
   drivers/scsi/qla2xxx/qla_dbg.h: In function 'qla_trace_uninit':
>> drivers/scsi/qla2xxx/qla_dbg.h:452:9: error: implicit declaration of function 'vfree'; did you mean 'kvfree'? [-Werror=implicit-function-declaration]
     452 |         vfree(trc->recs);
         |         ^~~~~
         |         kvfree
   In file included from drivers/scsi/qla2xxx/qla_init.c:11:
   include/linux/vmalloc.h: At top level:
>> include/linux/vmalloc.h:143:14: error: conflicting types for 'vzalloc'; have 'void *(long unsigned int)'
     143 | extern void *vzalloc(unsigned long size) __alloc_size(1);
         |              ^~~~~~~
   drivers/scsi/qla2xxx/qla_dbg.h:438:21: note: previous implicit declaration of 'vzalloc' with type 'int()'
     438 |         trc->recs = vzalloc(trc->num_entries *
         |                     ^~~~~~~
>> include/linux/vmalloc.h:163:13: warning: conflicting types for 'vfree'; have 'void(const void *)'
     163 | extern void vfree(const void *addr);
         |             ^~~~~
   drivers/scsi/qla2xxx/qla_dbg.h:452:9: note: previous implicit declaration of 'vfree' with type 'void(const void *)'
     452 |         vfree(trc->recs);
         |         ^~~~~
   drivers/scsi/qla2xxx/qla_init.c: In function 'qla24xx_async_abort_cmd':
   drivers/scsi/qla2xxx/qla_init.c:171:17: warning: variable 'bail' set but not used [-Wunused-but-set-variable]
     171 |         uint8_t bail;
         |                 ^~~~
   drivers/scsi/qla2xxx/qla_init.c: In function 'qla2x00_async_tm_cmd':
   drivers/scsi/qla2xxx/qla_init.c:2023:17: warning: variable 'bail' set but not used [-Wunused-but-set-variable]
    2023 |         uint8_t bail;
         |                 ^~~~
   cc1: some warnings being treated as errors
--
   In file included from drivers/scsi/qla2xxx/qla_def.h:5498,
                    from drivers/scsi/qla2xxx/qla_mbx.c:6:
   drivers/scsi/qla2xxx/qla_dbg.h: In function 'qla_trace_init':
>> drivers/scsi/qla2xxx/qla_dbg.h:438:21: error: implicit declaration of function 'vzalloc'; did you mean 'kvzalloc'? [-Werror=implicit-function-declaration]
     438 |         trc->recs = vzalloc(trc->num_entries *
         |                     ^~~~~~~
         |                     kvzalloc
>> drivers/scsi/qla2xxx/qla_dbg.h:438:19: warning: assignment to 'struct qla_trace_rec *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     438 |         trc->recs = vzalloc(trc->num_entries *
         |                   ^
   drivers/scsi/qla2xxx/qla_dbg.h: In function 'qla_trace_uninit':
>> drivers/scsi/qla2xxx/qla_dbg.h:452:9: error: implicit declaration of function 'vfree'; did you mean 'kvfree'? [-Werror=implicit-function-declaration]
     452 |         vfree(trc->recs);
         |         ^~~~~
         |         kvfree
   cc1: some warnings being treated as errors
--
   In file included from drivers/scsi/qla2xxx/qla_def.h:5498,
                    from drivers/scsi/qla2xxx/qla_iocb.c:6:
   drivers/scsi/qla2xxx/qla_dbg.h: In function 'qla_trace_init':
>> drivers/scsi/qla2xxx/qla_dbg.h:438:21: error: implicit declaration of function 'vzalloc'; did you mean 'kvzalloc'? [-Werror=implicit-function-declaration]
     438 |         trc->recs = vzalloc(trc->num_entries *
         |                     ^~~~~~~
         |                     kvzalloc
>> drivers/scsi/qla2xxx/qla_dbg.h:438:19: warning: assignment to 'struct qla_trace_rec *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     438 |         trc->recs = vzalloc(trc->num_entries *
         |                   ^
   drivers/scsi/qla2xxx/qla_dbg.h: In function 'qla_trace_uninit':
>> drivers/scsi/qla2xxx/qla_dbg.h:452:9: error: implicit declaration of function 'vfree'; did you mean 'kvfree'? [-Werror=implicit-function-declaration]
     452 |         vfree(trc->recs);
         |         ^~~~~
         |         kvfree
   In file included from include/linux/string.h:253,
                    from include/linux/bitmap.h:11,
                    from include/linux/cpumask.h:12,
                    from include/linux/smp.h:13,
                    from arch/mips/include/asm/cpu-type.h:12,
                    from arch/mips/include/asm/timex.h:19,
                    from include/linux/timex.h:67,
                    from include/linux/time32.h:13,
                    from include/linux/time.h:60,
                    from include/linux/stat.h:19,
                    from include/linux/module.h:13,
                    from drivers/scsi/qla2xxx/qla_def.h:12:
   In function 'fortify_memcpy_chk',
       inlined from 'qla24xx_els_dcmd2_iocb' at drivers/scsi/qla2xxx/qla_iocb.c:3065:2:
   include/linux/fortify-string.h:352:25: warning: call to '__read_overflow2_field' declared with attribute warning: detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Wattribute-warning]
     352 |                         __read_overflow2_field(q_size_field, size);
         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors
--
   In file included from drivers/scsi/qla2xxx/qla_def.h:5498,
                    from drivers/scsi/qla2xxx/qla_dfs.c:6:
   drivers/scsi/qla2xxx/qla_dbg.h: In function 'qla_trace_init':
>> drivers/scsi/qla2xxx/qla_dbg.h:438:21: error: implicit declaration of function 'vzalloc'; did you mean 'kvzalloc'? [-Werror=implicit-function-declaration]
     438 |         trc->recs = vzalloc(trc->num_entries *
         |                     ^~~~~~~
         |                     kvzalloc
>> drivers/scsi/qla2xxx/qla_dbg.h:438:19: warning: assignment to 'struct qla_trace_rec *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     438 |         trc->recs = vzalloc(trc->num_entries *
         |                   ^
   drivers/scsi/qla2xxx/qla_dbg.h: In function 'qla_trace_uninit':
>> drivers/scsi/qla2xxx/qla_dbg.h:452:9: error: implicit declaration of function 'vfree'; did you mean 'kvfree'? [-Werror=implicit-function-declaration]
     452 |         vfree(trc->recs);
         |         ^~~~~
         |         kvfree
   drivers/scsi/qla2xxx/qla_dfs.c: At top level:
   drivers/scsi/qla2xxx/qla_dfs.c:539:1: warning: 'qla_dfs_trace_write' defined but not used [-Wunused-function]
     539 | qla_dfs_trace_write(struct file *file, const char __user *buffer,
         | ^~~~~~~~~~~~~~~~~~~
   drivers/scsi/qla2xxx/qla_dfs.c:504:1: warning: 'qla_dfs_trace_show' defined but not used [-Wunused-function]
     504 | qla_dfs_trace_show(struct seq_file *s, void *unused)
         | ^~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +438 drivers/scsi/qla2xxx/qla_dbg.h

   424	
   425	static inline void
   426	qla_trace_init(struct qla_trace *trc, char *name, u32 num_entries)
   427	{
   428		if (trc->recs)
   429			return;
   430	
   431		memset(trc, 0, sizeof(*trc));
   432	
   433		trc->name = name;
   434		spin_lock_init(&trc->trc_lock);
   435		if (!num_entries)
   436			return;
   437		trc->num_entries = num_entries;
 > 438		trc->recs = vzalloc(trc->num_entries *
   439					sizeof(struct qla_trace_rec));
   440		if (!trc->recs)
   441			return;
   442	
   443		set_bit(QLA_TRACE_ENABLED, &trc->flags);
   444	}
   445	
   446	static inline void
   447	qla_trace_uninit(struct qla_trace *trc)
   448	{
   449		if (!trc->recs)
   450			return;
   451	
 > 452		vfree(trc->recs);
   453		trc->recs = NULL;
   454		clear_bit(QLA_TRACE_ENABLED, &trc->flags);
   455	}
   456	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [EXT] Re: [PATCH 2/6] qla2xxx: Add a generic tracing framework

[Steffen Maier]

On 7/20/22 00:09, Arun Easi wrote:
> On Tue, 19 Jul 2022, 1:25am, Daniel Wagner wrote:
>> On Mon, Jul 18, 2022 at 03:22:43PM -0400, Steven Rostedt wrote:
>>> On Mon, 18 Jul 2022 12:02:26 -0700
>>> Arun Easi <aeasi@marvell.com> wrote:
>>>
>>>> Many times when a problem was reported on the driver, we had to request
>>>> for a repro with extended error logging (via driver module parameter)
>>>> turned on. With this internal tracing in place, log messages that appear
>>>> only with extended error logging are captured by default in the internal
>>>> trace buffer.
>>>>
>>>> AFAIK, kernel tracing requires a user initiated action to be turned on,
>>>> like enabling individual tracepoints. Though a script (either startup or
>>>> udev) can do this job, enabling tracepoints by default for a single
>>>> driver, I think, may not be a preferred choice -- particularly when the
>>>> trace buffer is shared across the kernel. That also brings the extra
>>>> overhead of finding how this could be done across various distros.
>>>>
>>>> For cases where the memory/driver size matters, there is an option to
>>>> compile out this feature, plus choosing a lower default trace buffer size.
>>
>> I am really asking the question why do we need to add special debugging
>> code to every single driver? Can't we try to make more use of generic
>> code and extend it if necessary?
>>
>> Both FC drivers qla2xxx and lpfc are doing their own thing for

All three FC drivers:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/s390/scsi/zfcp_dbf.c

We have this flight recorder (multiple topical ring buffers per vHBA to avoid 
some higher frequency topics lose history of others) since a long time [maybe 
pre-dating ftrace], including crash tool support to extract it from post mortem 
kernel dumps. We use binary trace records, like tracepoints, with offline 
decoding/formatting (zfcpdbf in s390-tools package).
Other s390 kernel components share the underlying s390dbf infrastructure.

The crash extension "ftrace" is probably able to do an export from dump for the 
approach Steven suggested. I had used it with kernel function tracer. Very useful.

>> debugging/logging and I really fail to see why we can't not move towards
>> a more generic way. Dumping logs to the kernel log was the simplest way
>> but as this series shows, this is not something you can do in production
>> systems.
> 
> No contention here on a generic way. The per instance trace creation and
> enabling from within the kernel looks like such a one. Let me revisit the
> trace patches with this new info.
> 
> Regards,
> -Arun
> 
>>
>>> You can enable an ftrace instance from inside the kernel, and make it a
>>> compile time option.
>>>
>>> 	#include <linux/trace_events.h>
>>> 	#include <linux/trace.h>
>>>
>>> 	struct trace_array *tr;
>>>
>>> 	tr = trace_array_get_by_name("qla2xxx");
>>> 	trace_array_set_clr_event(tr, "qla", NULL, true);
>>>
>>> And now you have trace events running:
>>>
>>>   # cat /sys/kernel/tracing/instances/qla/trace
>>
>> Thanks Steve for the tip!
>>
>> Daniel
>>


-- 
Mit freundlichen Gruessen / Kind regards
Steffen Maier

Linux on IBM Z and LinuxONE

https://www.ibm.com/privacy/us/en/
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschaeftsfuehrung: David Faller
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: [EXT] Re: [PATCH 2/6] qla2xxx: Add a generic tracing framework

[Steven Rostedt]

On Sat, 23 Jul 2022 09:47:21 +0200
Steffen Maier <maier@linux.ibm.com> wrote:

> The crash extension "ftrace" is probably able to do an export from dump for the 
> approach Steven suggested. I had used it with kernel function tracer. Very useful.

Note, the crash extension is just called "trace" and you can get it here:

  https://github.com/fujitsu/crash-trace

-- Steve