| Message ID | 20250110122641.1668-2-shiju.jose@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | rasdaemon: cxl: Update CXL event logging and recording to CXL spec rev 3.1 | expand |
On Fri, 10 Jan 2025 12:26:27 +0000 <shiju.jose@huawei.com> wrote: > From: Shiju Jose <shiju.jose@huawei.com> > > 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@gandalf.local.home/ > > When rasdaemon reads a trace event format file,the maximum data size space after that comma if spinning a v3. > 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@huawei.com> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > 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);
Hi Jonathan, Thanks for the review. >-----Original Message----- >From: Jonathan Cameron <jonathan.cameron@huawei.com> >Sent: 10 January 2025 16:03 >To: Shiju Jose <shiju.jose@huawei.com> >Cc: linux-edac@vger.kernel.org; linux-cxl@vger.kernel.org; >mchehab@kernel.org; dave.jiang@intel.com; dan.j.williams@intel.com; >alison.schofield@intel.com; nifan.cxl@gmail.com; vishal.l.verma@intel.com; >ira.weiny@intel.com; dave@stgolabs.net; linux-kernel@vger.kernel.org; >Linuxarm <linuxarm@huawei.com>; tanxiaofei <tanxiaofei@huawei.com>; >Zengtao (B) <prime.zeng@hisilicon.com> >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@huawei.com> wrote: > >> From: Shiju Jose <shiju.jose@huawei.com> >> >> 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@gandalf.local.home >> / >> >> 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@huawei.com> > >Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > >> --- >> 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);
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);