RE: [PATCH v2 01/14] rasdaemon: Fix for parsing error when trace event's format file is larger than PAGE_SIZE

[Shiju Jose]

Hi Jonathan, 

Thanks for the review.

>-----Original Message-----
>From: Jonathan Cameron <jonathan.cameron@xxxxxxxxxx>
>Sent: 10 January 2025 16:03
>To: Shiju Jose <shiju.jose@xxxxxxxxxx>
>Cc: linux-edac@xxxxxxxxxxxxxxx; linux-cxl@xxxxxxxxxxxxxxx;
>mchehab@xxxxxxxxxx; dave.jiang@xxxxxxxxx; dan.j.williams@xxxxxxxxx;
>alison.schofield@xxxxxxxxx; nifan.cxl@xxxxxxxxx; vishal.l.verma@xxxxxxxxx;
>ira.weiny@xxxxxxxxx; dave@xxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
>Linuxarm <linuxarm@xxxxxxxxxx>; tanxiaofei <tanxiaofei@xxxxxxxxxx>;
>Zengtao (B) <prime.zeng@xxxxxxxxxxxxx>
>Subject: Re: [PATCH v2 01/14] rasdaemon: Fix for parsing error when trace
>event's format file is larger than PAGE_SIZE
>
>On Fri, 10 Jan 2025 12:26:27 +0000
><shiju.jose@xxxxxxxxxx> wrote:
>
>> From: Shiju Jose <shiju.jose@xxxxxxxxxx>
>>
>> When a trace event's format file is larger than PAGE_SIZE (4096) then
>> libtraceevent returns parsing failed when rasdaemon reads the fields.
>> The reason found that tep_parse_event() call in the
>> add_event_handler() internally fails in libtraceevent because of the
>> incomplete format file data read. However libtraceevent did not return
>> error in this stage, which is fixed in the following patch for libtraceevent.
>> https://lore.kernel.org/all/20250109102338.6128644d@xxxxxxxxxxxxxxxxxx
>> /
>>
>> When rasdaemon reads a trace event format file,the maximum data size
>
>space after that comma if spinning a v3.
Sure.
If no v3, then I will fix when the pull request is submitted.

Thanks,
Shiju
>
>> that can be read is limited to PAGE_SIZE by the seq_read() and
>> seq_read_iter() functions in the kernel. This results in userspace
>> receiving partial data if the format file is larger than PAGE_SIZE,
>> requiring fix in the rasdaemon to read the complete data from the
>> format file.
>>
>> Add fix for reading trace event format files larger than PAGE_SIZE in
>> add_event_handler().
>>
>> Signed-off-by: Shiju Jose <shiju.jose@xxxxxxxxxx>
>
>Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
>
>> ---
>>  ras-events.c | 21 +++++++++++++--------
>>  1 file changed, 13 insertions(+), 8 deletions(-)
>>
>> diff --git a/ras-events.c b/ras-events.c index 5d8118a..6692a31 100644
>> --- a/ras-events.c
>> +++ b/ras-events.c
>> @@ -376,7 +376,7 @@ static int filter_ras_mc_event(struct ras_events
>> *ras, char *group, char *event,
>>
>>  static int get_pagesize(struct ras_events *ras, struct tep_handle
>> *pevent)  {
>> -	int fd, len, page_size = 4096;
>> +	int fd, len, page_size = 8192;
>>  	char buf[page_size];
>>
>>  	fd = open_trace(ras, "events/header_page", O_RDONLY); @@ -827,7
>> +827,8 @@ static int add_event_handler(struct ras_events *ras, struct
>tep_handle *pevent,
>>  			     unsigned int page_size, char *group, char *event,
>>  			     tep_event_handler_func func, char *filter_str, int id)
>{
>> -	int fd, size, rc;
>> +	int fd, rc;
>> +	int size = 0;
>>  	char *page, fname[MAX_PATH + 1];
>>  	struct tep_event_filter *filter = NULL;
>>
>> @@ -857,13 +858,17 @@ static int add_event_handler(struct ras_events *ras,
>struct tep_handle *pevent,
>>  		return rc;
>>  	}
>>
>> -	size = read(fd, page, page_size);
>> +	do {
>> +		rc = read(fd, page + size, page_size);
>> +		if (rc < 0) {
>> +			log(TERM, LOG_ERR, "Can't get arch page size\n");
>> +			free(page);
>> +			close(fd);
>> +			return size;
>> +		}
>> +		size += rc;
>> +	} while (rc > 0);
>>  	close(fd);
>> -	if (size < 0) {
>> -		log(TERM, LOG_ERR, "Can't get arch page size\n");
>> -		free(page);
>> -		return size;
>> -	}
>>
>>  	/* Registers the special event handlers */
>>  	rc = tep_register_event_handler(pevent, -1, group, event, func,
>> ras);