On Sun, 6 Dec 2020 17:42:26 +0100
Bean Huo <huobean@xxxxxxxxx> wrote:
> From: Bean Huo <beanhuo@xxxxxxxxxx>
>
> Transaction Specific Fields (TSF) in the UPIU package could be CDB
> (SCSI/UFS Command Descriptor Block), OSF (Opcode Specific Field), and
> TM I/O parameter (Task Management Input/Output Parameter). But, currently,
> we take all of these as CDB in the UPIU trace. Thus makes user confuse
> among CDB, OSF, and TM message. So fix it with this patch.
>
> Signed-off-by: Bean Huo <beanhuo@xxxxxxxxxx>
> ---
> drivers/scsi/ufs/ufshcd.c | 9 +++++----
> include/trace/events/ufs.h | 10 +++++++---
> 2 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 29d7240a61bf..5b2219e44743 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -315,7 +315,8 @@ static void ufshcd_add_cmd_upiu_trace(struct ufs_hba *hba, unsigned int tag,
> {
> struct utp_upiu_req *rq = hba->lrb[tag].ucd_req_ptr;
>
> - trace_ufshcd_upiu(dev_name(hba->dev), str, &rq->header, &rq->sc.cdb);
> + trace_ufshcd_upiu(dev_name(hba->dev), str, &rq->header, &rq->sc.cdb,
> + "CDB");
> }
>
> static void ufshcd_add_query_upiu_trace(struct ufs_hba *hba, unsigned int tag,
> @@ -329,7 +330,7 @@ static void ufshcd_add_query_upiu_trace(struct ufs_hba *hba, unsigned int tag,
> rq_rsp = (struct utp_upiu_req *)hba->lrb[tag].ucd_rsp_ptr;
>
> trace_ufshcd_upiu(dev_name(hba->dev), str, &rq_rsp->header,
> - &rq_rsp->qr);
> + &rq_rsp->qr, "OSF");
> }
>
> static void ufshcd_add_tm_upiu_trace(struct ufs_hba *hba, unsigned int tag,
> @@ -340,10 +341,10 @@ static void ufshcd_add_tm_upiu_trace(struct ufs_hba *hba, unsigned int tag,
>
> if (!strcmp("tm_send", str))
> trace_ufshcd_upiu(dev_name(hba->dev), str, &descp->req_header,
> - &descp->input_param1);
> + &descp->input_param1, "TM_INPUT");
> else
> trace_ufshcd_upiu(dev_name(hba->dev), str, &descp->rsp_header,
> - &descp->output_param1);
> + &descp->output_param1, "TM_OUTPUT");
You could save some space on the ring buffer, if you made the above into an
enum, and then used print_symbolic().
> }
>
> static void ufshcd_add_uic_command_trace(struct ufs_hba *hba,
> diff --git a/include/trace/events/ufs.h b/include/trace/events/ufs.h
> index 0bd54a184391..68e8e97a9b47 100644
> --- a/include/trace/events/ufs.h
> +++ b/include/trace/events/ufs.h
> @@ -295,15 +295,17 @@ TRACE_EVENT(ufshcd_uic_command,
> );
You could make this:
#define TRACE_TSF_TYPES \
EM(CDB) \
EM(OSF) \
EM(TM_INPUT) \
EMe(TM_OUTPUT)
#ifndef TRACE_TSF_TYPES_ENUMS
#define TRACE_TSF_TYPES_ENUMS
#undef EM
#undef EMe
#define EM(x) TRACE_TSF_##x,
#define EMe(x) TRACE_TSF_##x
enum {
TRACE_TSF_TYPES
}
#endif /* TRACE_TSF_TYPES_ENUMS */
#undef EM
#undef EMe
/* These export the enum names to user space */
#define EM(x) TRACE_DEFINE_ENUM(TRACE_TSF_##x)
#define EMe(x) TRACE_DEFINE_ENUM(TRACE_TSF_##x)
TRACE_TSF_TYPES
#undef EM
#undef EMe
/* These are used in the print_symbolic */
#define EM(x) { TRACE_TSF_##x, #x },
#define EMe(x) { TRACE_TSF_##x, #x }
>
> TRACE_EVENT(ufshcd_upiu,
> - TP_PROTO(const char *dev_name, const char *str, void *hdr, void *tsf),
> + TP_PROTO(const char *dev_name, const char *str, void *hdr, void *tsf,
> + const char *tsf_type),
int tsf_type;
>
> - TP_ARGS(dev_name, str, hdr, tsf),
> + TP_ARGS(dev_name, str, hdr, tsf, tsf_type),
>
> TP_STRUCT__entry(
> __string(dev_name, dev_name)
> __string(str, str)
> __array(unsigned char, hdr, 12)
> __array(unsigned char, tsf, 16)
> + __string(tsf_type, tsf_type)
__field(int, tsf_type)
> ),
>
> TP_fast_assign(
> @@ -311,12 +313,14 @@ TRACE_EVENT(ufshcd_upiu,
> __assign_str(str, str);
> memcpy(__entry->hdr, hdr, sizeof(__entry->hdr));
> memcpy(__entry->tsf, tsf, sizeof(__entry->tsf));
> + __assign_str(tsf_type, tsf_type);
__entry->tsf_type = tsf_type;
> ),
>
> TP_printk(
> - "%s: %s: HDR:%s, CDB:%s",
> + "%s: %s: HDR:%s, %s:%s",
> __get_str(str), __get_str(dev_name),
> __print_hex(__entry->hdr, sizeof(__entry->hdr)),
> + __get_str(tsf_type),
print_symbolic(tsf_type, TRACE_TSF_TYPES),
-- Steve
> __print_hex(__entry->tsf, sizeof(__entry->tsf))
> )
> );
