RE: [PATCH 1/1] tracing: Support reading trace event format file larger than PAGE_SIZE

Shiju Jose
2025-01-06T18:15:39Z

>-----Original Message-----
>From: Steven Rostedt <rostedt@goodmis.org>
>Sent: 06 January 2025 17:22
>To: Shiju Jose <shiju.jose@huawei.com>
>Cc: mhiramat@kernel.org; mathieu.desnoyers@efficios.com; linux-trace-
>kernel@vger.kernel.org; linux-kernel@vger.kernel.org; Linuxarm
><linuxarm@huawei.com>; Jonathan Cameron
><jonathan.cameron@huawei.com>; tanxiaofei <tanxiaofei@huawei.com>;
>Zengtao (B) <prime.zeng@hisilicon.com>
>Subject: Re: [PATCH 1/1] tracing: Support reading trace event format file larger
>than PAGE_SIZE
>
>On Thu, 2 Jan 2025 17:43:17 +0000
><shiju.jose@huawei.com> wrote:
>
>> From: Shiju Jose <shiju.jose@huawei.com>
>>
>> When userspace reads a trace event format file, the maximum data size
>> that can be read is limited to PAGE_SIZE by the seq_read() and
>> seq_read_iter() functions. This results in userspace receiving partial
>> data if the format file is larger than PAGE_SIZE, requiring a
>> workaround to read the complete data from the format file.
>>
>> Add support for reading trace event format files larger than PAGE_SIZE
>> when needed by userspace.
>>
>> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
>
>How is this an issue? This is common for all pseudo files and can be handled
>properly from user space.
>
>Here, with this program:
>
>read.c:
>-------------------------8<-------------------------
>#include <stdlib.h>
>#include <stdio.h>
>#include <unistd.h>
>#include <fcntl.h>
>#include <sys/types.h>
>
>int main (int argc, char **argv)
>{
> char *buf;
> int fd;
> off_t size;
> int r, s;
>
> if (argc < 2) {
> printf("usage: %s file\n", argv[0]);
> exit(-1);
> }
>
> fd = open(argv[1], O_RDONLY);
> if (fd < 0) {
> perror(argv[1]);
> exit(-1);
> }
>
> size = BUFSIZ * 10;
>
> buf = malloc(size);
> for (s = 0, r = 1; r > 0; s += r) {
> r = read(fd, buf, size);
> if (r < 0) {
> perror(argv[1]);
> exit(-1);
> }
> printf("Read %d bytes from %s\n", r, argv[1]);
> }
> free(buf);
> close(fd);
> return 0;
>}
>------------------------->8-------------------------
>
> $ read /proc/kallsyms
>Read 4091 bytes from /proc/kallsyms
>Read 4075 bytes from /proc/kallsyms
>Read 4078 bytes from /proc/kallsyms
>Read 4083 bytes from /proc/kallsyms
>Read 4093 bytes from /proc/kallsyms
>Read 4076 bytes from /proc/kallsyms
>Read 4080 bytes from /proc/kallsyms
>Read 4086 bytes from /proc/kallsyms
>Read 4080 bytes from /proc/kallsyms
>Read 4064 bytes from /proc/kallsyms
>Read 4071 bytes from /proc/kallsyms
>Read 4063 bytes from /proc/kallsyms
>Read 4069 bytes from /proc/kallsyms
>Read 4079 bytes from /proc/kallsyms
>Read 4063 bytes from /proc/kallsyms
>Read 4072 bytes from /proc/kallsyms
>Read 4046 bytes from /proc/kallsyms
>Read 4091 bytes from /proc/kallsyms
>Read 4090 bytes from /proc/kallsyms
>Read 4067 bytes from /proc/kallsyms
>Read 4080 bytes from /proc/kallsyms
>Read 4066 bytes from /proc/kallsyms
>Read 4085 bytes from /proc/kallsyms
>Read 4095 bytes from /proc/kallsyms
>Read 4076 bytes from /proc/kallsyms
>Read 4090 bytes from /proc/kallsyms
>Read 4066 bytes from /proc/kallsyms
>Read 4073 bytes from /proc/kallsyms
>Read 4091 bytes from /proc/kallsyms
>Read 4075 bytes from /proc/kallsyms
>Read 4076 bytes from /proc/kallsyms
>Read 4048 bytes from /proc/kallsyms
>Read 4074 bytes from /proc/kallsyms
>Read 4058 bytes from /proc/kallsyms
>Read 4074 bytes from /proc/kallsyms
>[..]
>Read 4052 bytes from /proc/kallsyms
>Read 4061 bytes from /proc/kallsyms
>Read 4061 bytes from /proc/kallsyms
>Read 4053 bytes from /proc/kallsyms
>Read 4083 bytes from /proc/kallsyms
>Read 4066 bytes from /proc/kallsyms
>Read 4093 bytes from /proc/kallsyms
>Read 4072 bytes from /proc/kallsyms
>Read 1982 bytes from /proc/kallsyms
>Read 0 bytes from /proc/kallsyms
>
>You see, it requires multiple reads to pull in an entire kernel pseudo file. None of
>those reads are greater than PAGE_SIZE. Why should trace format files be any
>different?
Thanks for the reply.
Yes. I had a fix/workaround in the userspace rasdaemon with multiple reads like above
as reported previously in the following thread.
https://lore.kernel.org/lkml/3c9808a694d242cab35bab67602edebf@huawei.com/
However thought a solution in the common kernel code for the format file
may be better. I will go ahead with the user space solution.

Also shared an information in the above thread about libtraceevent __parse_event() does not
return error when parse_format() fail with incomplete formt data, which resulted initialization for the
trace event does not fail in the user space tool.
>libtracefs handles this perfectly fine:
>
> https://git.kernel.org/pub/scm/libs/libtrace/libtracefs.git/tree/src/tracefs-
>utils.c#n343
>
>Looks like you are trying to change the kernel to fix a user space bug :-/
>
> NAK!
>
>-- Steve

Thanks,
Shiju