On 5/15/26 11:50 AM, Steven Rostedt wrote:
On Fri, 15 May 2026 08:27:27 -0700
Bart Van Assche <[email protected]> wrote:

On 5/15/26 6:59 AM, Vineeth Pillai (Google) wrote:
   static void ufshcd_add_query_upiu_trace(struct ufs_hba *hba,
@@ -432,8 +432,8 @@ static void ufshcd_add_query_upiu_trace(struct ufs_hba *hba,
        if (!trace_ufshcd_upiu_enabled())
                return;
- trace_ufshcd_upiu(hba, str_t, &rq_rsp->header,
-                         &rq_rsp->qr, UFS_TSF_OSF);
+       trace_call__ufshcd_upiu(hba, str_t, &rq_rsp->header,
+                              &rq_rsp->qr, UFS_TSF_OSF);
   }

Instead of making this change, please remove the
trace_ufshcd_upiu_enabled() call because it is redundant.

You mean to remove the ufshcd_add_query_upiu_trace() function and just use
a tracepoint where it is called?

That would be even better.

   static void ufshcd_add_tm_upiu_trace(struct ufs_hba *hba, unsigned int tag,
@@ -445,15 +445,15 @@ static void ufshcd_add_tm_upiu_trace(struct ufs_hba *hba, 
unsigned int tag,
                return;
if (str_t == UFS_TM_SEND)
-               trace_ufshcd_upiu(hba, str_t,
-                                 &descp->upiu_req.req_header,
-                                 &descp->upiu_req.input_param1,
-                                 UFS_TSF_TM_INPUT);
+               trace_call__ufshcd_upiu(hba, str_t,
+                                       &descp->upiu_req.req_header,
+                                       &descp->upiu_req.input_param1,
+                                       UFS_TSF_TM_INPUT);
        else
-               trace_ufshcd_upiu(hba, str_t,
-                                 &descp->upiu_rsp.rsp_header,
-                                 &descp->upiu_rsp.output_param1,
-                                 UFS_TSF_TM_OUTPUT);
+               trace_call__ufshcd_upiu(hba, str_t,
+                                       &descp->upiu_rsp.rsp_header,
+                                       &descp->upiu_rsp.output_param1,
+                                       UFS_TSF_TM_OUTPUT);
   }

Same comment here: I think it would be better to remove the
trace_ufshcd_upiu_enabled() call rather than
changing trace_ufshcd_upiu() into trace_call__ufshcd_upiu().

Well, removing it here would mean placing the if (str == UFS_TM_SEND) into
the code and processing it even when tracing is disabled. With the
trace_*_enabled() helper, it's all a nop.

The ufshcd_add_tm_upiu_trace() function is only called from the UFS
error handler and hence is not performance sensitive. The execution of
an additional if-test in this function is not a concern at all.

Thanks,

Bart.

Reply via email to