Linux Trace Kernel
 help / color / mirror / Atom feed
* [PATCH v3 08/11] scsi: ufs: Use trace_call__##name() at guarded tracepoint call sites
@ 2026-05-15 13:59 Vineeth Pillai (Google)
  2026-05-15 15:27 ` Bart Van Assche
  0 siblings, 1 reply; 3+ messages in thread
From: Vineeth Pillai (Google) @ 2026-05-15 13:59 UTC (permalink / raw)
  To: James E.J. Bottomley, Martin K. Petersen
  Cc: linux-scsi, Steven Rostedt, linux-trace-kernel, Vineeth Pillai,
	Peter Zijlstra

From: Vineeth Pillai <vineeth@bitbyteword.org>

Replace trace_foo() with the new trace_call__foo() at sites already
guarded by trace_foo_enabled(), avoiding a redundant
static_branch_unlikely() re-evaluation inside the tracepoint.
trace_call__foo() calls the tracepoint callbacks directly without
utilizing the static branch again.

Original v2 series:
https://lore.kernel.org/linux-trace-kernel/20260323160052.17528-1-vineeth@bitbyteword.org/

Parts of the original v2 series have already been merged in mainline.
This patch is being reposted as a follow-up cleanup for the remaining
unmerged pieces.

Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Vineeth Pillai (Google) <vineeth@bitbyteword.org>
Assisted-by: Claude:claude-sonnet-4-6
---
 drivers/ufs/core/ufshcd.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index c3f08957d179..07f3126d2a94 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -421,8 +421,8 @@ static void ufshcd_add_cmd_upiu_trace(struct ufs_hba *hba,
 	else
 		header = &lrb->ucd_rsp_ptr->header;
 
-	trace_ufshcd_upiu(hba, str_t, header, &rq->sc.cdb,
-			  UFS_TSF_CDB);
+	trace_call__ufshcd_upiu(hba, str_t, header, &rq->sc.cdb,
+			       UFS_TSF_CDB);
 }
 
 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);
 }
 
 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);
 }
 
 static void ufshcd_add_uic_command_trace(struct ufs_hba *hba,
@@ -470,10 +470,10 @@ static void ufshcd_add_uic_command_trace(struct ufs_hba *hba,
 	else
 		cmd = ufshcd_readl(hba, REG_UIC_COMMAND);
 
-	trace_ufshcd_uic_command(hba, str_t, cmd,
-				 ufshcd_readl(hba, REG_UIC_COMMAND_ARG_1),
-				 ufshcd_readl(hba, REG_UIC_COMMAND_ARG_2),
-				 ufshcd_readl(hba, REG_UIC_COMMAND_ARG_3));
+	trace_call__ufshcd_uic_command(hba, str_t, cmd,
+				       ufshcd_readl(hba, REG_UIC_COMMAND_ARG_1),
+				       ufshcd_readl(hba, REG_UIC_COMMAND_ARG_2),
+				       ufshcd_readl(hba, REG_UIC_COMMAND_ARG_3));
 }
 
 static void ufshcd_add_command_trace(struct ufs_hba *hba, struct scsi_cmnd *cmd,
@@ -522,8 +522,9 @@ static void ufshcd_add_command_trace(struct ufs_hba *hba, struct scsi_cmnd *cmd,
 	} else {
 		doorbell = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
 	}
-	trace_ufshcd_command(cmd->device, hba, str_t, tag, doorbell, hwq_id,
-			     transfer_len, intr, lba, opcode, group_id);
+	trace_call__ufshcd_command(cmd->device, hba, str_t, tag, doorbell,
+				   hwq_id, transfer_len, intr, lba, opcode,
+				   group_id);
 }
 
 static void ufshcd_print_clk_freqs(struct ufs_hba *hba)
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v3 08/11] scsi: ufs: Use trace_call__##name() at guarded tracepoint call sites
  2026-05-15 13:59 [PATCH v3 08/11] scsi: ufs: Use trace_call__##name() at guarded tracepoint call sites Vineeth Pillai (Google)
@ 2026-05-15 15:27 ` Bart Van Assche
  2026-05-15 18:50   ` Steven Rostedt
  0 siblings, 1 reply; 3+ messages in thread
From: Bart Van Assche @ 2026-05-15 15:27 UTC (permalink / raw)
  To: Vineeth Pillai (Google), James E.J. Bottomley, Martin K. Petersen
  Cc: linux-scsi, Steven Rostedt, linux-trace-kernel, Peter Zijlstra


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.

>   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().

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3 08/11] scsi: ufs: Use trace_call__##name() at guarded tracepoint call sites
  2026-05-15 15:27 ` Bart Van Assche
@ 2026-05-15 18:50   ` Steven Rostedt
  0 siblings, 0 replies; 3+ messages in thread
From: Steven Rostedt @ 2026-05-15 18:50 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Vineeth Pillai (Google), James E.J. Bottomley, Martin K. Petersen,
	linux-scsi, linux-trace-kernel, Peter Zijlstra

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?

Makes sense.

> 
> >   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.

-- Steve



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-15 18:50 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-15 13:59 [PATCH v3 08/11] scsi: ufs: Use trace_call__##name() at guarded tracepoint call sites Vineeth Pillai (Google)
2026-05-15 15:27 ` Bart Van Assche
2026-05-15 18:50   ` Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox