| Message ID | 20230215190448.1687786-1-jaegeuk@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | scsi: ufs: support IO traces for zoned block device | expand |
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>
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".
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.
> 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. For us as well. Moreover, we are mainly using upiu tracing which is not available at scsi ML. Thanks, Avri
On Mon, Feb 27, 2023 at 03:15:51PM -0800, Jaegeuk Kim wrote: > 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 The group ID isn't even ever set, how can it be useful to you?
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 --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") \