RE: [PATCH v4 3/6] cxl/events: Update General Media Event Record to CXL spec rev 3.1

[Shiju Jose <shiju.jose@huawei.com>]

>-----Original Message-----
>From: Shiju Jose <shiju.jose@huawei.com>
>Sent: 28 November 2024 10:02
>To: Steven Rostedt <rostedt@goodmis.org>
>Cc: dave.jiang@intel.com; dan.j.williams@intel.com; Jonathan Cameron
><jonathan.cameron@huawei.com>; alison.schofield@intel.com;
>nifan.cxl@gmail.com; vishal.l.verma@intel.com; ira.weiny@intel.com;
>dave@stgolabs.net; linux-cxl@vger.kernel.org; linux-kernel@vger.kernel.org;
>Linuxarm <linuxarm@huawei.com>; tanxiaofei <tanxiaofei@huawei.com>;
>Zengtao (B) <prime.zeng@hisilicon.com>
>Subject: RE: [PATCH v4 3/6] cxl/events: Update General Media Event Record to
>CXL spec rev 3.1
>
>>-----Original Message-----
>>From: Steven Rostedt <rostedt@goodmis.org>
>>Sent: 27 November 2024 18:34
>>To: Shiju Jose <shiju.jose@huawei.com>
>>Cc: dave.jiang@intel.com; dan.j.williams@intel.com; Jonathan Cameron
>><jonathan.cameron@huawei.com>; alison.schofield@intel.com;
>>nifan.cxl@gmail.com; vishal.l.verma@intel.com; ira.weiny@intel.com;
>>dave@stgolabs.net; linux-cxl@vger.kernel.org;
>>linux-kernel@vger.kernel.org; Linuxarm <linuxarm@huawei.com>;
>>tanxiaofei <tanxiaofei@huawei.com>; Zengtao (B)
>><prime.zeng@hisilicon.com>
>>Subject: Re: [PATCH v4 3/6] cxl/events: Update General Media Event
>>Record to CXL spec rev 3.1
>>
>>On Wed, 27 Nov 2024 18:20:26 +0000
>>Shiju Jose <shiju.jose@huawei.com> wrote:
>>
>>> I tested removing hdr_uuid and region_uuid from the rasdaemon test
>>> setup as you suggested. As a result, libtraceevent parses correctly,
>>> as you
>>mentioned.
>>>
>>> However, I encounter  similar parsing error ("FAILED TO PARSE") when
>>> I add two additional decoded strings (%s) to the TP_printk, replacing
>>> (%u). Please see the attached format file,
>>"format_cxl_general_media_v3.1_basic", for your reference.
>>
>>Are you sure. I don't see anything wrong with that one. Which version
>>of libtraceevent do you have?
>
>libtraceevent source code version 1.8.4,  build for arm64.

Hi Steve,

I debug this case and please find the info,
1. rasdaemon :  read() from format file return around 1/3rd of the 
    actual data in the file only when the total size of the format's file
    is above 4K bytes (page size), For example, in this case, the total size was 4512 bytes,
    but read only 1674 bytes.
    This partial data resulted  tep_parse_event() in libtraceevent failed internally in the parse_format()     
    and  since __parse_event() does not return error when parse_format() fail,
    thus initialization of the event does not fail.
     
   The following  solution in rasdaemon solved the issue, 
   (provided if no other fix for the above issue with read()), 
    1. read() and accumulate content of format file until EOF reached.
    2. Increased the buffer size from 4K bytes to  8K bytes.
    3. May be __parse_event()  in libtraceevent  and thus tep_parse_event()  return error
        when parse_format() fail so that the initialization would fail if the input data is
        corrupted or partial?   
>
>>
>>>
>>> I've also attached another format file,
>>> "format_cxl_general_media_v3.1_full",
>>> which contains the complete TP_printk() intended.
>>
>>This one has some complex arguments and also uses the '&' address of an
>>argument.
>Thanks.
>I will debug this when basic one is working.

This case too worked  with above workaround after removing '&' address
from the TP_printk().
>>
>>>
>>> Can you please help or else can you share how to debug these errors
>>> in the libtraceevent setup?
>>
>>Basically, I use the attached program (that just links to libtraceevent).
>>
>>Note, I need to delete the first line of your files, which has the "cat"
>>command. But you can run this on the file itself:
>>
>>  ./tep_load_event
>> /sys/kernel/tracing/events/cxl/cxl_general_media/format
>>
>>But you may need to be root to do that. If root just copies that file,
>>you can then run it as non-root.
>>
>> # cp /sys/kernel/tracing/events/cxl/cxl_general_media/format /tmp  $
>>./tep_load_event /tmp/format
>>
>>I run it under gdb and see where it fails. But it should let you know
>>if it will pass or not. I put a breakpoint on tep_warning and when it
>>gets hit, I examine what it did up to that point.
>
>Thanks Steve for the instructions. I will try this.

I built tep_load_event.c for arm64 and it worked after copy
format file to the /tmp/ folder as you said.
>>
>>-- Steve
>

Thanks,
Shiju