diff mbox series

scsi: ufs: support IO traces for zoned block device

Message ID 20230215190448.1687786-1-jaegeuk@kernel.org
State New
Headers show
Series scsi: ufs: support IO traces for zoned block device | expand

Commit Message

Jaegeuk Kim Feb. 15, 2023, 7:04 p.m. UTC 
From: Jaegeuk Kim <jaegeuk@google.com>

Let's support WRITE_16, READ_16, ZBC_IN, ZBC_OUT.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 drivers/ufs/core/ufshcd.c  | 8 ++++----
 include/trace/events/ufs.h | 4 +++-
 2 files changed, 7 insertions(+), 5 deletions(-)

Comments

Bart Van Assche Feb. 15, 2023, 7:13 p.m. UTC | #1 
On 2/15/23 11:04, Jaegeuk Kim wrote:
> Let's support WRITE_16, READ_16, ZBC_IN, ZBC_OUT.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Christoph Hellwig Feb. 16, 2023, 6:01 a.m. UTC | #2 
Why does UFS even have it's own common tracing instad of just relying
on the core SCSI one, and even worse pokes into command set specifics
which is a no-go for LLDDs.  This code simply needs to go away instead
of beeing "enhanced".
Jaegeuk Kim Feb. 27, 2023, 11:15 p.m. UTC | #3 
On 02/15, Christoph Hellwig wrote:
> Why does UFS even have it's own common tracing instad of just relying
> on the core SCSI one, and even worse pokes into command set specifics
> which is a no-go for LLDDs.  This code simply needs to go away instead
> of beeing "enhanced".

I'm not sure how all the other vendors use the trace tho, at least to me,
it's quite useful when debugging any UFS-specific information such as
group_id and doorbell status along with the attached scsi command, in
addition to the accurate latency measurements.
Christoph Hellwig March 20, 2023, 12:55 p.m. UTC | #4 
On Tue, Feb 28, 2023 at 07:28:35AM +0000, Avri Altman wrote:
> > doorbell status along with the attached scsi command, in addition to the
> > accurate latency measurements.
> For us as well.
> Moreover, we are mainly using upiu tracing which is not available at scsi ML.

So let's rework the tracepoints to work at the UPIU level and stop the
gracious poking into SCSI CDBs.
diff mbox series

Patch

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index e18c9f4463ec..235d2e2d828a 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -402,15 +402,15 @@  static void ufshcd_add_command_trace(struct ufs_hba *hba, unsigned int tag,
 
 	opcode = cmd->cmnd[0];
 
-	if (opcode == READ_10 || opcode == WRITE_10) {
-		/*
-		 * Currently we only fully trace read(10) and write(10) commands
-		 */
+	if (opcode == READ_10 || opcode == WRITE_10 ||
+				opcode == READ_16 || opcode == WRITE_16) {
 		transfer_len =
 		       be32_to_cpu(lrbp->ucd_req_ptr->sc.exp_data_transfer_len);
 		lba = scsi_get_lba(cmd);
 		if (opcode == WRITE_10)
 			group_id = lrbp->cmd->cmnd[6];
+		if (opcode == WRITE_16)
+			group_id = lrbp->cmd->cmnd[14];
 	} else if (opcode == UNMAP) {
 		/*
 		 * The number of Bytes to be unmapped beginning with the lba.
diff --git a/include/trace/events/ufs.h b/include/trace/events/ufs.h
index 599739ee7b20..f82a9e15fd78 100644
--- a/include/trace/events/ufs.h
+++ b/include/trace/events/ufs.h
@@ -18,7 +18,9 @@ 
 		{ READ_16,		"READ_16" },			\
 		{ READ_10,		"READ_10" },			\
 		{ SYNCHRONIZE_CACHE,	"SYNC" },			\
-		{ UNMAP,		"UNMAP" })
+		{ UNMAP,		"UNMAP" },			\
+		{ ZBC_IN,		"ZBC_IN" },			\
+		{ ZBC_OUT,		"ZBC_OUT" })
 
 #define UFS_LINK_STATES						\
 	EM(UIC_LINK_OFF_STATE,		"UIC_LINK_OFF_STATE")		\