[PATCH v3] ufs: core: add hba parameter to trace events

[Steven Rostedt]

On Fri, 14 Feb 2025 16:29:36 +0800
<peter.wang at mediatek.com> wrote:

> From: Peter Wang <peter.wang at mediatek.com>
> 
> Included the ufs_hba structure as a parameter in various trace events
> to provide more context and improve debugging capabilities.
> Also remove dev_name which can replace by dev_name(hba->dev).
> 
> V3:
>  - Remove dev_name entry form TP_STRUCT__entry() to reduce the size.
> 


> V2:
>  - Remove dev_name and replace it with dev_name(hba->dev).

This patch needs to be reverted, as the above causes a bug.

> 
> Signed-off-by: Peter Wang <peter.wang at mediatek.com>
> ---
>  drivers/ufs/core/ufs_trace.h | 135 ++++++++++++++++++-----------------
>  drivers/ufs/core/ufshcd.c    |  68 +++++++++---------
>  2 files changed, 103 insertions(+), 100 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufs_trace.h b/drivers/ufs/core/ufs_trace.h
> index 84deca2b841d..caa32e23ffa5 100644
> --- a/drivers/ufs/core/ufs_trace.h
> +++ b/drivers/ufs/core/ufs_trace.h
> @@ -83,34 +83,34 @@ UFS_CMD_TRACE_TSF_TYPES
>  
>  TRACE_EVENT(ufshcd_clk_gating,
>  
> -	TP_PROTO(const char *dev_name, int state),
> +	TP_PROTO(struct ufs_hba *hba, int state),
>  
> -	TP_ARGS(dev_name, state),
> +	TP_ARGS(hba, state),
>  
>  	TP_STRUCT__entry(
> -		__string(dev_name, dev_name)
> +		__field(struct ufs_hba *, hba)
>  		__field(int, state)
>  	),
>  
>  	TP_fast_assign(
> -		__assign_str(dev_name);
> +		__entry->hba = hba;
>  		__entry->state = state;
>  	),
>  
>  	TP_printk("%s: gating state changed to %s",
> -		__get_str(dev_name),
> +		dev_name(__entry->hba->dev),

NO YOU CAN NOT DO THIS!!!!

The TP_fast_assign() happens when the trace event occurs. That is, where
the trace_ufshcd_clk_gating() function is called.

The TP_printk() happens when a user reads the "trace" file. Which could be
seconds, minutes, hours, days, even months later! There is absolutely no
guarantee that the pointer to __entry->hba would be around. This would
cause a crash of the kernel when the "trace" file is read.

I'm adding a boot up test to cause code like this to trigger a warning when
the trace event is registered (how I found this bug):

  https://lore.kernel.org/all/20260630164439.51e61b71@gandalf.local.home/

>  		__print_symbolic(__entry->state, UFSCHD_CLK_GATING_STATES))
>  );

The same goes for the rest of this file.

-- Steve