| Message ID | 20201214202014.13835-1-huobean@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Several changes for the UPIU trace | expand |
Bean Hi, I support this series. I think it is a good idea to print the response on complete, But you need to change the prefix strings, otherwise you are breaking the current parsers. Say that you have a trace log, generated sometime during 2020 using the current upiu trace. It would look something like: "send" <request upiu> "complete" <request upiu> And another log generated sometime during 2021 after your change is merged: "send" <request upiu> "complete" < ****response upiu ****> The current parser won't be able to differentiate between those logs. Just change the prefix strings to be "send_req" and "complete_rsp", or something, so the parsing tools that support the new format will be able to differentiate it from the old one. Also, once the parser can differentiate the new format from the old, whatever follows its fine: cdb / osf / tsf or whatever makes sense to you. Thanks, Avri > -----Original Message----- > From: Bean Huo <huobean@gmail.com> > Sent: Monday, December 14, 2020 10:20 PM > To: alim.akhtar@samsung.com; Avri Altman <Avri.Altman@wdc.com>; > asutoshd@codeaurora.org; jejb@linux.ibm.com; > martin.petersen@oracle.com; stanley.chu@mediatek.com; > beanhuo@micron.com; bvanassche@acm.org; tomas.winkler@intel.com; > cang@codeaurora.org; rostedt@goodmis.org; joe@perches.com > Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: [PATCH v3 0/6] Several changes for the UPIU trace > > CAUTION: This email originated from outside of Western Digital. Do not click > on links or open attachments unless you recognize the sender and know that > the content is safe. > > > From: Bean Huo <beanhuo@micron.com> > > Changelog: > > V2--V3: > 1. Fix a typo in patch 1/6 (Reported-by: Joe Perches <joe@perches.com>) > > V1--V2: > 1. Convert __get_str(str) to __print_symbolic() > 2. Add new patches 1/6, 2/6,3/6 > 3. Use __print_symbolic() in patch 6/6 > > Bean Huo (6): > scsi: ufs: Remove stringize operator '#' restriction > scsi: ufs: Use __print_symbolic() for UFS trace string print > scsi: ufs: Don't call trace_ufshcd_upiu() in case trace poit is > disabled > scsi: ufs: Distinguish between query REQ and query RSP in query trace > scsi: ufs: Distinguish between TM request UPIU and response UPIU in TM > UPIU trace > scsi: ufs: Make UPIU trace easier differentiate among CDB, OSF, and TM > > drivers/scsi/ufs/ufs.h | 17 ++++++ > drivers/scsi/ufs/ufshcd.c | 72 ++++++++++++++++--------- > include/trace/events/ufs.h | 108 +++++++++++++++++++++++-------------- > 3 files changed, 131 insertions(+), 66 deletions(-) > > -- > 2.17.1
On Mon, 2020-12-14 at 22:13 +0000, Avri Altman wrote: > Bean Hi, > I support this series. > I think it is a good idea to print the response on complete, > But you need to change the prefix strings, otherwise you are breaking > the current parsers. > > Say that you have a trace log, generated sometime during 2020 using > the current upiu trace. > It would look something like: > "send" <request upiu> > "complete" <request upiu> > > And another log generated sometime during 2021 after your change is > merged: > "send" <request upiu> > "complete" < ****response upiu ****> > > The current parser won't be able to differentiate between those logs. > Just change the prefix strings to be "send_req" and "complete_rsp", > or something, > so the parsing tools that support the new format will be able to > differentiate it from the old one. Avri, I still don't understand, this change doesn't break you current parser. if you still trace "send", "complete", "CDB", "query_send/complte", they are still there, doesn't change. I suggest you just run on your system. see if there is conflict. Regarding your suggestion: This is not problem now, we just change this definition. do you mean just "send" and "complete" or all? #define UFS_CMD_TRACE_STRINGS \ EM(UFS_CMD_SEND, "send_req") \ EM(UFS_CMD_COMP, "complete_rsp") \ below also need add "req" and "rsp"? EM(UFS_DEV_COMP, "dev_complete_rsp") \ EM(UFS_QUERY_SEND, "query_send") \ EM(UFS_QUERY_COMP, "query_complete") \ EM(UFS_QUERY_ERR, "query_complete_err") \ EM(UFS_TM_SEND, "tm_send") \ EM(UFS_TM_COMP, "tm_complete") \ EM(UFS_TM_ERR, "tm_complete_err") > > Also, once the parser can differentiate the new format from the old, > whatever follows its fine: cdb / osf / tsf or whatever makes sense to > you. > > Thanks, > Avri
On Mon, 2020-12-14 at 23:37 +0100, Bean Huo wrote: > > And another log generated sometime during 2021 after your change is > > merged: > > "send" <request upiu> > > "complete" < ****response upiu ****> > > > > The current parser won't be able to differentiate between those > > logs. > > Just change the prefix strings to be "send_req" and "complete_rsp", > > or something, > > so the parsing tools that support the new format will be able to > > differentiate it from the old one. > > Avri, > I still don't understand, this change doesn't break you current > parser. > if you still trace "send", "complete", "CDB", "query_send/complte", > they are still there, doesn't change. I suggest you just run on your > system. see if there is conflict. > > Regarding your suggestion: > This is not problem now, we just change this definition. > > do you mean just "send" and "complete" or all? > > #define > UFS_CMD_TRACE_STRINGS > \ > > EM(UFS_CMD_SEND, "send_req") > \ > > EM(UFS_CMD_COMP, "complete_rsp") > \ > > below also need add "req" and "rsp"? > > > EM(UFS_DEV_COMP, "dev_complete_rsp") > \ > > EM(UFS_QUERY_SEND, "query_send") > \ > > EM(UFS_QUERY_COMP, "query_complete") > \ > > EM(UFS_QUERY_ERR, "query_complete_err") > \ > > EM(UFS_TM_SEND, "tm_send") > \ > > EM(UFS_TM_COMP, "tm_complete") > \ > > EM(UFS_TM_ERR, "tm_complete_err") Hi Avri I am waiting for your answer. How can I change these strings to back- compatible with your tool? Tt seems only you use these strings. Thanks, Bean
On Tue, 2020-12-15 at 23:18 +0100, Bean Huo wrote: > On Mon, 2020-12-14 at 23:37 +0100, Bean Huo wrote: > > > And another log generated sometime during 2021 after your change > > > is > > > merged: > > > "send" <request upiu> > > > "complete" < ****response upiu ****> > > > > > > The current parser won't be able to differentiate between those > > > logs. > > > Just change the prefix strings to be "send_req" and > > > "complete_rsp", > > > or something, > > > so the parsing tools that support the new format will be able to > > > differentiate it from the old one. > > > > Avri, > > I still don't understand, this change doesn't break you current > > parser. > > if you still trace "send", "complete", "CDB", "query_send/complte", > > they are still there, doesn't change. I suggest you just run on > > your > > system. see if there is conflict. > > > > Regarding your suggestion: > > This is not problem now, we just change this definition. > > > > do you mean just "send" and "complete" or all? > > > > #define > > UFS_CMD_TRACE_STRINGS > > \ > > > > EM(UFS_CMD_SEND, "send_req") > > \ > > > > EM(UFS_CMD_COMP, "complete_rsp") > > \ > > > > below also need add "req" and "rsp"? > > > > > > EM(UFS_DEV_COMP, "dev_complete_rsp") > > \ > > > > EM(UFS_QUERY_SEND, "query_send") > > \ > > > > EM(UFS_QUERY_COMP, "query_complete") > > \ > > > > EM(UFS_QUERY_ERR, "query_complete_err") > > \ > > > > EM(UFS_TM_SEND, "tm_send") > > \ > > > > EM(UFS_TM_COMP, "tm_complete") > > \ > > > > EM(UFS_TM_ERR, "tm_complete_err") > > > Hi Avri > > I am waiting for your answer. How can I change these strings to back- > compatible with your tool? Tt seems only you use these strings. > > Thanks, > Bean > Hi Avri Before sending next version, double confirm with your. I think you just need change: "send" to "send_req" "complete" to "complete_rsp" Bean
From: Bean Huo <beanhuo@micron.com> Changelog: V2--V3: 1. Fix a typo in patch 1/6 (Reported-by: Joe Perches <joe@perches.com>) V1--V2: 1. Convert __get_str(str) to __print_symbolic() 2. Add new patches 1/6, 2/6,3/6 3. Use __print_symbolic() in patch 6/6 Bean Huo (6): scsi: ufs: Remove stringize operator '#' restriction scsi: ufs: Use __print_symbolic() for UFS trace string print scsi: ufs: Don't call trace_ufshcd_upiu() in case trace poit is disabled scsi: ufs: Distinguish between query REQ and query RSP in query trace scsi: ufs: Distinguish between TM request UPIU and response UPIU in TM UPIU trace scsi: ufs: Make UPIU trace easier differentiate among CDB, OSF, and TM drivers/scsi/ufs/ufs.h | 17 ++++++ drivers/scsi/ufs/ufshcd.c | 72 ++++++++++++++++--------- include/trace/events/ufs.h | 108 +++++++++++++++++++++++-------------- 3 files changed, 131 insertions(+), 66 deletions(-)