Re: [PATCH v3 08/11] scsi: ufs: Use trace_call__##name() at guarded tracepoint call sites

[Bart Van Assche <bvanassche@acm.org>]

On 5/15/26 11:50 AM, Steven Rostedt wrote:
> On Fri, 15 May 2026 08:27:27 -0700
> Bart Van Assche <bvanassche@acm.org> 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.