[PATCH v1 0/2] This patchset adds tracepoints to pm80xx

[PATCH v1 0/2] This patchset adds tracepoints to pm80xx

[Changyuan Lyu]

Changyuan Lyu (2):
  scsi: pm80xx: Add tracepoints
  scsi: pm80xx: Add pm80xx_mpi_build_cmd tracepoint

 drivers/scsi/pm8001/Makefile             |   7 +-
 drivers/scsi/pm8001/pm8001_hwi.c         |   5 +
 drivers/scsi/pm8001/pm8001_sas.c         |  16 ++++
 drivers/scsi/pm8001/pm80xx_hwi.c         |   7 ++
 drivers/scsi/pm8001/pm80xx_tracepoints.c |  10 ++
 drivers/scsi/pm8001/pm80xx_tracepoints.h | 113 +++++++++++++++++++++++
 6 files changed, 156 insertions(+), 2 deletions(-)
 create mode 100644 drivers/scsi/pm8001/pm80xx_tracepoints.c
 create mode 100644 drivers/scsi/pm8001/pm80xx_tracepoints.h

-- 
2.33.0.1079.g6e70778dc9-goog

[PATCH v1 1/2] scsi: pm80xx: Add tracepoints

[Changyuan Lyu]

Tracepoints for tracking controller and ATA commands issued and
completed.

Signed-off-by: Akshat Jain <akshatzen@google.com>
Signed-off-by: Changyuan Lyu <changyuanl@google.com>
---
 drivers/scsi/pm8001/Makefile             |  7 +-
 drivers/scsi/pm8001/pm8001_sas.c         | 16 +++++
 drivers/scsi/pm8001/pm80xx_hwi.c         |  7 ++
 drivers/scsi/pm8001/pm80xx_tracepoints.c | 10 +++
 drivers/scsi/pm8001/pm80xx_tracepoints.h | 85 ++++++++++++++++++++++++
 5 files changed, 123 insertions(+), 2 deletions(-)
 create mode 100644 drivers/scsi/pm8001/pm80xx_tracepoints.c
 create mode 100644 drivers/scsi/pm8001/pm80xx_tracepoints.h

diff --git a/drivers/scsi/pm8001/Makefile b/drivers/scsi/pm8001/Makefile
index 02b7338999cc..bbb51b7312f1 100644
--- a/drivers/scsi/pm8001/Makefile
+++ b/drivers/scsi/pm8001/Makefile
@@ -6,9 +6,12 @@
 
 
 obj-$(CONFIG_SCSI_PM8001) += pm80xx.o
+
+CFLAGS_pm80xx_tracepoints.o := -I$(src)
+
 pm80xx-y += pm8001_init.o \
 		pm8001_sas.o  \
 		pm8001_ctl.o  \
 		pm8001_hwi.o  \
-		pm80xx_hwi.o
-
+		pm80xx_hwi.o  \
+		pm80xx_tracepoints.o
diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index 32e60f0c3b14..fcd12da1125c 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -40,6 +40,7 @@
 
 #include <linux/slab.h>
 #include "pm8001_sas.h"
+#include "pm80xx_tracepoints.h"
 
 /**
  * pm8001_find_tag - from sas task to find out  tag that belongs to this task
@@ -527,6 +528,9 @@ int pm8001_queue_command(struct sas_task *task, gfp_t gfp_flags)
 void pm8001_ccb_task_free(struct pm8001_hba_info *pm8001_ha,
 	struct sas_task *task, struct pm8001_ccb_info *ccb, u32 ccb_idx)
 {
+	struct ata_queued_cmd *qc;
+	struct pm8001_device *pm8001_dev;
+
 	if (!ccb->task)
 		return;
 	if (!sas_protocol_ata(task->task_proto))
@@ -549,6 +553,18 @@ void pm8001_ccb_task_free(struct pm8001_hba_info *pm8001_ha,
 		/* do nothing */
 		break;
 	}
+
+	if (sas_protocol_ata(task->task_proto)) {
+		// For SCSI/ATA commands uldd_task points to ata_queued_cmd
+		qc = task->uldd_task;
+		pm8001_dev = ccb->device;
+		trace_pm80xx_request_complete(pm8001_ha->id,
+			pm8001_dev ? pm8001_dev->attached_phy : PM8001_MAX_PHYS,
+			ccb_idx, 0 /* ctlr_opcode not known */,
+			qc ? qc->tf.command : 0, // ata opcode
+			pm8001_dev ? atomic_read(&pm8001_dev->running_req) : -1);
+	}
+
 	task->lldd_task = NULL;
 	ccb->task = NULL;
 	ccb->ccb_tag = 0xFFFFFFFF;
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index 6ffe17b849ae..015244e0227b 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -42,6 +42,7 @@
  #include "pm80xx_hwi.h"
  #include "pm8001_chips.h"
  #include "pm8001_ctl.h"
+#include "pm80xx_tracepoints.h"
 
 #define SMP_DIRECT 1
 #define SMP_INDIRECT 2
@@ -4504,6 +4505,7 @@ static int pm80xx_chip_sata_req(struct pm8001_hba_info *pm8001_ha,
 	struct sas_task *task = ccb->task;
 	struct domain_device *dev = task->dev;
 	struct pm8001_device *pm8001_ha_dev = dev->lldd_dev;
+	struct ata_queued_cmd *qc = task->uldd_task;
 	u32 tag = ccb->ccb_tag;
 	int ret;
 	u32 q_index, cpu_id;
@@ -4723,6 +4725,11 @@ static int pm80xx_chip_sata_req(struct pm8001_hba_info *pm8001_ha,
 			}
 		}
 	}
+	trace_pm80xx_request_issue(pm8001_ha->id,
+				ccb->device ? ccb->device->attached_phy : PM8001_MAX_PHYS,
+				ccb->ccb_tag, opc,
+				qc ? qc->tf.command : 0, // ata opcode
+				ccb->device ? atomic_read(&ccb->device->running_req) : 0);
 	ret = pm8001_mpi_build_cmd(pm8001_ha, circularQ, opc,
 			&sata_cmd, sizeof(sata_cmd), q_index);
 	return ret;
diff --git a/drivers/scsi/pm8001/pm80xx_tracepoints.c b/drivers/scsi/pm8001/pm80xx_tracepoints.c
new file mode 100644
index 000000000000..344aface9cdb
--- /dev/null
+++ b/drivers/scsi/pm8001/pm80xx_tracepoints.c
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Trace events in pm8001 driver.
+ *
+ * Copyright 2020 Google LLC
+ * Author: Akshat Jain <akshatzen@google.com>
+ */
+
+#define CREATE_TRACE_POINTS
+#include "pm80xx_tracepoints.h"
diff --git a/drivers/scsi/pm8001/pm80xx_tracepoints.h b/drivers/scsi/pm8001/pm80xx_tracepoints.h
new file mode 100644
index 000000000000..84fcfecfd624
--- /dev/null
+++ b/drivers/scsi/pm8001/pm80xx_tracepoints.h
@@ -0,0 +1,85 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Trace events in pm8001 driver.
+ *
+ * Copyright 2020 Google LLC
+ * Author: Akshat Jain <akshatzen@google.com>
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM pm80xx
+
+#if !defined(_TRACE_PM80XX_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_PM80XX_H
+
+#include <linux/tracepoint.h>
+#include "pm8001_sas.h"
+
+TRACE_EVENT(pm80xx_request_issue,
+	    TP_PROTO(u32 id, u32 phy_id, u32 htag, u32 ctlr_opcode,
+		     u16 ata_opcode, int running_req),
+
+	    TP_ARGS(id, phy_id, htag, ctlr_opcode, ata_opcode, running_req),
+
+	    TP_STRUCT__entry(
+		    __field(u32, id)
+		    __field(u32, phy_id)
+		    __field(u32, htag)
+		    __field(u32, ctlr_opcode)
+		    __field(u16,  ata_opcode)
+		    __field(int, running_req)
+		    ),
+
+	    TP_fast_assign(
+		    __entry->id = id;
+		    __entry->phy_id = phy_id;
+		    __entry->htag = htag;
+		    __entry->ctlr_opcode = ctlr_opcode;
+		    __entry->ata_opcode = ata_opcode;
+		    __entry->running_req = running_req;
+		    ),
+
+	    TP_printk("ctlr_id = %u phy_id = %u htag = %#x, ctlr_opcode = %#x ata_opcode = %#x running_req = %d",
+		    __entry->id, __entry->phy_id, __entry->htag,
+		    __entry->ctlr_opcode, __entry->ata_opcode,
+		    __entry->running_req)
+);
+
+TRACE_EVENT(pm80xx_request_complete,
+	    TP_PROTO(u32 id, u32 phy_id, u32 htag, u32 ctlr_opcode,
+		     u16 ata_opcode, int running_req),
+
+	    TP_ARGS(id, phy_id, htag, ctlr_opcode, ata_opcode, running_req),
+
+	    TP_STRUCT__entry(
+		    __field(u32, id)
+		    __field(u32, phy_id)
+		    __field(u32, htag)
+		    __field(u32, ctlr_opcode)
+		    __field(u16,  ata_opcode)
+		    __field(int, running_req)
+		    ),
+
+	    TP_fast_assign(
+		    __entry->id = id;
+		    __entry->phy_id = phy_id;
+		    __entry->htag = htag;
+		    __entry->ctlr_opcode = ctlr_opcode;
+		    __entry->ata_opcode = ata_opcode;
+		    __entry->running_req = running_req;
+		    ),
+
+	    TP_printk("ctlr_id = %u phy_id = %u htag = %#x, ctlr_opcode = %#x ata_opcode = %#x running_req = %d",
+		    __entry->id, __entry->phy_id, __entry->htag,
+		    __entry->ctlr_opcode, __entry->ata_opcode,
+		    __entry->running_req)
+);
+
+#endif /* _TRACE_PM80XX_H_ */
+
+#undef TRACE_INCLUDE_PATH
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_PATH .
+#define TRACE_INCLUDE_FILE pm80xx_tracepoints
+
+#include <trace/define_trace.h>
-- 
2.33.0.1079.g6e70778dc9-goog

[PATCH v1 2/2] scsi: pm80xx: Add pm80xx_mpi_build_cmd tracepoint

[Changyuan Lyu]

pm8001_mpi_build_cmd() prepares and sends all commands to a controller.
Having pm80xx_mpi_build_cmd tracepoint can help us with latency issues.

this patch depends on patch "scsi: pm80xx: Add tracepoints".

Signed-off-by: Igor Pylypiv <ipylypiv@google.com>
Signed-off-by: Changyuan Lyu <changyuanl@google.com>
---
 drivers/scsi/pm8001/pm8001_hwi.c         |  5 +++++
 drivers/scsi/pm8001/pm80xx_tracepoints.h | 28 ++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index 63690508313b..a56ae05bb9d3 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -42,6 +42,7 @@
  #include "pm8001_hwi.h"
  #include "pm8001_chips.h"
  #include "pm8001_ctl.h"
+ #include "pm80xx_tracepoints.h"
 
 /**
  * read_main_config_table - read the configure table and save it.
@@ -1324,6 +1325,10 @@ int pm8001_mpi_build_cmd(struct pm8001_hba_info *pm8001_ha,
 	unsigned long flags;
 	int q_index = circularQ - pm8001_ha->inbnd_q_tbl;
 	int rv;
+	u32 htag = le32_to_cpu(*(__le32 *)payload);
+
+	trace_pm80xx_mpi_build_cmd(pm8001_ha->id, opCode, htag, q_index,
+		circularQ->producer_idx, circularQ->consumer_index);
 
 	WARN_ON(q_index >= PM8001_MAX_INB_NUM);
 	spin_lock_irqsave(&circularQ->iq_lock, flags);
diff --git a/drivers/scsi/pm8001/pm80xx_tracepoints.h b/drivers/scsi/pm8001/pm80xx_tracepoints.h
index 84fcfecfd624..5e669a8a9344 100644
--- a/drivers/scsi/pm8001/pm80xx_tracepoints.h
+++ b/drivers/scsi/pm8001/pm80xx_tracepoints.h
@@ -75,6 +75,34 @@ TRACE_EVENT(pm80xx_request_complete,
 		    __entry->running_req)
 );
 
+TRACE_EVENT(pm80xx_mpi_build_cmd,
+	    TP_PROTO(u32 id, u32 opc, u32 htag, u32 qi, u32 pi, u32 ci),
+
+	    TP_ARGS(id, opc, htag, qi, pi, ci),
+
+	    TP_STRUCT__entry(
+		    __field(u32, id)
+		    __field(u32, opc)
+		    __field(u32, htag)
+		    __field(u32, qi)
+		    __field(u32, pi)
+		    __field(u32, ci)
+		    ),
+
+	    TP_fast_assign(
+		    __entry->id = id;
+		    __entry->opc = opc;
+		    __entry->htag = htag;
+		    __entry->qi = qi;
+		    __entry->pi = pi;
+		    __entry->ci = ci;
+		    ),
+
+	    TP_printk("ctlr_id = %u opc = %#x htag = %#x QI = %u PI = %u CI = %u",
+		    __entry->id, __entry->opc, __entry->htag, __entry->qi,
+		    __entry->pi, __entry->ci)
+);
+
 #endif /* _TRACE_PM80XX_H_ */
 
 #undef TRACE_INCLUDE_PATH
-- 
2.33.0.1079.g6e70778dc9-goog

Re: [PATCH v1 2/2] scsi: pm80xx: Add pm80xx_mpi_build_cmd tracepoint

[kernel test robot]

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

Hi Changyuan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on jejb-scsi/for-next v5.15-rc7 next-20211026]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Changyuan-Lyu/This-patchset-adds-tracepoints-to-pm80xx/20211021-054356
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: alpha-randconfig-s032-20211027 (attached as .config)
compiler: alpha-linux-gcc (GCC) 11.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-dirty
        # https://github.com/0day-ci/linux/commit/b48fdd621388924781f93fff019ef42bd9f22c60
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Changyuan-Lyu/This-patchset-adds-tracepoints-to-pm80xx/20211021-054356
        git checkout b48fdd621388924781f93fff019ef42bd9f22c60
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=alpha 

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


sparse warnings: (new ones prefixed by >>)
>> drivers/scsi/pm8001/pm8001_hwi.c:1331:51: sparse: sparse: incorrect type in argument 6 (different base types) @@     expected unsigned int [usertype] ci @@     got restricted __le32 [usertype] consumer_index @@
   drivers/scsi/pm8001/pm8001_hwi.c:1331:51: sparse:     expected unsigned int [usertype] ci
   drivers/scsi/pm8001/pm8001_hwi.c:1331:51: sparse:     got restricted __le32 [usertype] consumer_index
   drivers/scsi/pm8001/pm8001_hwi.c:1866:36: sparse: sparse: invalid assignment: |=
   drivers/scsi/pm8001/pm8001_hwi.c:1866:36: sparse:    left side has type restricted __le32
   drivers/scsi/pm8001/pm8001_hwi.c:1866:36: sparse:    right side has type int
   drivers/scsi/pm8001/pm8001_hwi.c:4661:35: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __le32 [addressable] [assigned] [usertype] ds_ads_m @@     got int @@
   drivers/scsi/pm8001/pm8001_hwi.c:4661:35: sparse:     expected restricted __le32 [addressable] [assigned] [usertype] ds_ads_m
   drivers/scsi/pm8001/pm8001_hwi.c:4661:35: sparse:     got int

vim +1331 drivers/scsi/pm8001/pm8001_hwi.c

  1307	
  1308	/**
  1309	 * pm8001_mpi_build_cmd- build the message queue for transfer, update the PI to
  1310	 * FW to tell the fw to get this message from IOMB.
  1311	 * @pm8001_ha: our hba card information
  1312	 * @circularQ: the inbound queue we want to transfer to HBA.
  1313	 * @opCode: the operation code represents commands which LLDD and fw recognized.
  1314	 * @payload: the command payload of each operation command.
  1315	 * @nb: size in bytes of the command payload
  1316	 * @responseQueue: queue to interrupt on w/ command response (if any)
  1317	 */
  1318	int pm8001_mpi_build_cmd(struct pm8001_hba_info *pm8001_ha,
  1319				 struct inbound_queue_table *circularQ,
  1320				 u32 opCode, void *payload, size_t nb,
  1321				 u32 responseQueue)
  1322	{
  1323		u32 Header = 0, hpriority = 0, bc = 1, category = 0x02;
  1324		void *pMessage;
  1325		unsigned long flags;
  1326		int q_index = circularQ - pm8001_ha->inbnd_q_tbl;
  1327		int rv;
  1328		u32 htag = le32_to_cpu(*(__le32 *)payload);
  1329	
  1330		trace_pm80xx_mpi_build_cmd(pm8001_ha->id, opCode, htag, q_index,
> 1331			circularQ->producer_idx, circularQ->consumer_index);
  1332	
  1333		WARN_ON(q_index >= PM8001_MAX_INB_NUM);
  1334		spin_lock_irqsave(&circularQ->iq_lock, flags);
  1335		rv = pm8001_mpi_msg_free_get(circularQ, pm8001_ha->iomb_size,
  1336				&pMessage);
  1337		if (rv < 0) {
  1338			pm8001_dbg(pm8001_ha, IO, "No free mpi buffer\n");
  1339			rv = -ENOMEM;
  1340			goto done;
  1341		}
  1342	
  1343		if (nb > (pm8001_ha->iomb_size - sizeof(struct mpi_msg_hdr)))
  1344			nb = pm8001_ha->iomb_size - sizeof(struct mpi_msg_hdr);
  1345		memcpy(pMessage, payload, nb);
  1346		if (nb + sizeof(struct mpi_msg_hdr) < pm8001_ha->iomb_size)
  1347			memset(pMessage + nb, 0, pm8001_ha->iomb_size -
  1348					(nb + sizeof(struct mpi_msg_hdr)));
  1349	
  1350		/*Build the header*/
  1351		Header = ((1 << 31) | (hpriority << 30) | ((bc & 0x1f) << 24)
  1352			| ((responseQueue & 0x3F) << 16)
  1353			| ((category & 0xF) << 12) | (opCode & 0xFFF));
  1354	
  1355		pm8001_write_32((pMessage - 4), 0, cpu_to_le32(Header));
  1356		/*Update the PI to the firmware*/
  1357		pm8001_cw32(pm8001_ha, circularQ->pi_pci_bar,
  1358			circularQ->pi_offset, circularQ->producer_idx);
  1359		pm8001_dbg(pm8001_ha, DEVIO,
  1360			   "INB Q %x OPCODE:%x , UPDATED PI=%d CI=%d\n",
  1361			   responseQueue, opCode, circularQ->producer_idx,
  1362			   circularQ->consumer_index);
  1363	done:
  1364		spin_unlock_irqrestore(&circularQ->iq_lock, flags);
  1365		return rv;
  1366	}
  1367	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31741 bytes --]