diff mbox series

[RFC,2/2] cxl: Add tprintk support for header log hex dump

Message ID 20230113154058.16227-3-Jonathan.Cameron@huawei.com
State New, archived
Headers show
Series CXL UE RAS Multiple Header Logging support | expand

Commit Message

Jonathan Cameron Jan. 13, 2023, 3:40 p.m. UTC
May not make sense in general, but very helpful when writing multiple
header logging support.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/cxl/core/trace.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Dave Jiang Jan. 17, 2023, 6:08 p.m. UTC | #1
On 1/13/23 8:40 AM, Jonathan Cameron wrote:
> May not make sense in general, but very helpful when writing multiple
> header logging support.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

The user logging software would be retrieving from the log data directly 
instead of parsing the trace output right? Will people visually inspect 
the formatted trace output? Useful for debugging?

> ---
>   drivers/cxl/core/trace.h | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
> index 20ca2fe2ca8e..64f6ad13529d 100644
> --- a/drivers/cxl/core/trace.h
> +++ b/drivers/cxl/core/trace.h
> @@ -62,10 +62,13 @@ TRACE_EVENT(cxl_aer_uncorrectable_error,
>   		 */
>   		memcpy(__entry->header_log, hl, CXL_HEADERLOG_SIZE);
>   	),
> -	TP_printk("%s: status: '%s' first_error: '%s'",
> +	TP_printk("%s: status: '%s' first_error: '%s' header_log: %s",
>   		  __get_str(dev_name),
>   		  show_uc_errs(__entry->status),
> -		  show_uc_errs(__entry->first_error)
> +		  show_uc_errs(__entry->first_error),
> +		__print_hex_dump("", DUMP_PREFIX_OFFSET, 32, 4,
> +				(char *)__entry->header_log,
> +				CXL_HEADERLOG_SIZE_U32, false)
>   	)
>   );
>
Jonathan Cameron Jan. 18, 2023, 9:43 a.m. UTC | #2
On Tue, 17 Jan 2023 11:08:26 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> On 1/13/23 8:40 AM, Jonathan Cameron wrote:
> > May not make sense in general, but very helpful when writing multiple
> > header logging support.
> > 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>  
> 
> The user logging software would be retrieving from the log data directly 
> instead of parsing the trace output right? Will people visually inspect 
> the formatted trace output? Useful for debugging?

Rather depends on the use case and rate of expected errors + if you are
trying to debug errors before your logging infrastructure is up.

Sure in production, likely these tracepoints would just get stored to a DB for later
querying.  But in debug, I often just use the kernel parameter
to directly dump them in the main kernel log, or perf's own decoding of a trace.

The nearest equivalent to this is AER which does print the log.
https://elixir.bootlin.com/linux/v6.2-rc4/source/include/ras/ras_event.h#L337

though that uses __print_array so maybe that's a better choice.
Don't get the nice offsets though if we do that though...

I'm not fussed about this patch going upstream or not, it just makes testing
patch 1 a lot easier :)

Jonathan


> 
> > ---
> >   drivers/cxl/core/trace.h | 7 +++++--
> >   1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
> > index 20ca2fe2ca8e..64f6ad13529d 100644
> > --- a/drivers/cxl/core/trace.h
> > +++ b/drivers/cxl/core/trace.h
> > @@ -62,10 +62,13 @@ TRACE_EVENT(cxl_aer_uncorrectable_error,
> >   		 */
> >   		memcpy(__entry->header_log, hl, CXL_HEADERLOG_SIZE);
> >   	),
> > -	TP_printk("%s: status: '%s' first_error: '%s'",
> > +	TP_printk("%s: status: '%s' first_error: '%s' header_log: %s",
> >   		  __get_str(dev_name),
> >   		  show_uc_errs(__entry->status),
> > -		  show_uc_errs(__entry->first_error)
> > +		  show_uc_errs(__entry->first_error),
> > +		__print_hex_dump("", DUMP_PREFIX_OFFSET, 32, 4,
> > +				(char *)__entry->header_log,
> > +				CXL_HEADERLOG_SIZE_U32, false)
> >   	)
> >   );
> >
Dave Jiang Jan. 18, 2023, 3:19 p.m. UTC | #3
On 1/18/23 2:43 AM, Jonathan Cameron wrote:
> On Tue, 17 Jan 2023 11:08:26 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
> 
>> On 1/13/23 8:40 AM, Jonathan Cameron wrote:
>>> May not make sense in general, but very helpful when writing multiple
>>> header logging support.
>>>
>>> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>>
>> The user logging software would be retrieving from the log data directly
>> instead of parsing the trace output right? Will people visually inspect
>> the formatted trace output? Useful for debugging?
> 
> Rather depends on the use case and rate of expected errors + if you are
> trying to debug errors before your logging infrastructure is up.
> 
> Sure in production, likely these tracepoints would just get stored to a DB for later
> querying.  But in debug, I often just use the kernel parameter
> to directly dump them in the main kernel log, or perf's own decoding of a trace.
> 
> The nearest equivalent to this is AER which does print the log.
> https://elixir.bootlin.com/linux/v6.2-rc4/source/include/ras/ras_event.h#L337
> 
> though that uses __print_array so maybe that's a better choice.
> Don't get the nice offsets though if we do that though...
> 
> I'm not fussed about this patch going upstream or not, it just makes testing
> patch 1 a lot easier :)

Thanks for the explanation.
Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> 
> Jonathan
> 
> 
>>
>>> ---
>>>    drivers/cxl/core/trace.h | 7 +++++--
>>>    1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
>>> index 20ca2fe2ca8e..64f6ad13529d 100644
>>> --- a/drivers/cxl/core/trace.h
>>> +++ b/drivers/cxl/core/trace.h
>>> @@ -62,10 +62,13 @@ TRACE_EVENT(cxl_aer_uncorrectable_error,
>>>    		 */
>>>    		memcpy(__entry->header_log, hl, CXL_HEADERLOG_SIZE);
>>>    	),
>>> -	TP_printk("%s: status: '%s' first_error: '%s'",
>>> +	TP_printk("%s: status: '%s' first_error: '%s' header_log: %s",
>>>    		  __get_str(dev_name),
>>>    		  show_uc_errs(__entry->status),
>>> -		  show_uc_errs(__entry->first_error)
>>> +		  show_uc_errs(__entry->first_error),
>>> +		__print_hex_dump("", DUMP_PREFIX_OFFSET, 32, 4,
>>> +				(char *)__entry->header_log,
>>> +				CXL_HEADERLOG_SIZE_U32, false)
>>>    	)
>>>    );
>>>      
>
diff mbox series

Patch

diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
index 20ca2fe2ca8e..64f6ad13529d 100644
--- a/drivers/cxl/core/trace.h
+++ b/drivers/cxl/core/trace.h
@@ -62,10 +62,13 @@  TRACE_EVENT(cxl_aer_uncorrectable_error,
 		 */
 		memcpy(__entry->header_log, hl, CXL_HEADERLOG_SIZE);
 	),
-	TP_printk("%s: status: '%s' first_error: '%s'",
+	TP_printk("%s: status: '%s' first_error: '%s' header_log: %s",
 		  __get_str(dev_name),
 		  show_uc_errs(__entry->status),
-		  show_uc_errs(__entry->first_error)
+		  show_uc_errs(__entry->first_error),
+		__print_hex_dump("", DUMP_PREFIX_OFFSET, 32, 4,
+				(char *)__entry->header_log,
+				CXL_HEADERLOG_SIZE_U32, false)
 	)
 );