* [PATCH 1/2] trace-cmd library: Make cpu_data[] match the cpus
2022-03-12 23:40 [PATCH 0/2] trace-cmd library: Fix sparse cpu_data Steven Rostedt
@ 2022-03-12 23:40 ` Steven Rostedt
2022-03-12 23:40 ` [PATCH 2/2] trace-cmd library: Do not read CPU greater than CPUs registered Steven Rostedt
2022-03-14 14:59 ` [PATCH 0/2] trace-cmd library: Fix sparse cpu_data Yordan Karadzhov
2 siblings, 0 replies; 7+ messages in thread
From: Steven Rostedt @ 2022-03-12 23:40 UTC (permalink / raw)
To: linux-trace-devel; +Cc: Steven Rostedt (Google)
From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
There's logic that expects cpu_data[cpu] to match the cpu buffer for the
given cpu. But now that the cpu_data array only holds the CPUs that exist
in the trace.dat file, it breaks this logic. Create the array for all cpus
regardless if a cpu exists or not. Just have the data structure be zero.
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
lib/trace-cmd/trace-input.c | 46 ++++++++++++++++++++++++++++++-------
1 file changed, 38 insertions(+), 8 deletions(-)
diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index 6e59a12f7807..da922834de1f 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -3196,7 +3196,9 @@ static int handle_buffer_option(struct tracecmd_input *handle,
unsigned short id, char *data, int size)
{
struct input_buffer_instance *buff;
+ struct cpu_file_data *cpu_data;
unsigned long long tmp;
+ long long max_cpu = -1;
int rsize = 0;
char *name;
int i;
@@ -3247,24 +3249,52 @@ static int handle_buffer_option(struct tracecmd_input *handle,
buff->cpus = tmp;
if (!buff->cpus)
return 0;
- buff->cpu_data = calloc(buff->cpus, sizeof(struct cpu_file_data));
- if (!buff->cpu_data)
+ cpu_data = calloc(buff->cpus, sizeof(*cpu_data));
+ if (!cpu_data)
return -1;
for (i = 0; i < buff->cpus; i++) {
if (save_read_number(handle->pevent, data, &size, &rsize, 4, &tmp))
- return -1;
- buff->cpu_data[i].cpu = tmp;
+ goto fail;
+ if ((long long)tmp > max_cpu)
+ max_cpu = tmp;
+ cpu_data[i].cpu = tmp;
if (save_read_number(handle->pevent, data,
- &size, &rsize, 8, &buff->cpu_data[i].offset))
- return -1;
+ &size, &rsize, 8, &cpu_data[i].offset))
+ goto fail;
if (save_read_number(handle->pevent, data,
- &size, &rsize, 8, &buff->cpu_data[i].size))
- return -1;
+ &size, &rsize, 8, &cpu_data[i].size))
+ goto fail;
+ }
+ if (buff->cpus == max_cpu + 1) {
+ /* Check to make sure cpus match the index */
+ for (i = 0; i < buff->cpus; i++) {
+ if (cpu_data[i].cpu != i)
+ goto copy_buffer;
+ }
+ buff->cpu_data = cpu_data;
+ } else {
+ copy_buffer:
+ buff->cpu_data = calloc(max_cpu + 1, sizeof(*cpu_data));
+ if (!buff->cpu_data)
+ goto fail;
+ for (i = 0; i < buff->cpus; i++) {
+ if (buff->cpu_data[cpu_data[i].cpu].size) {
+ tracecmd_warning("More than one buffer defined for CPU %d (buffer %d)\n",
+ cpu_data[i].cpu, i);
+ goto fail;
+ }
+ buff->cpu_data[cpu_data[i].cpu] = cpu_data[i];
+ }
+ buff->cpus = max_cpu + 1;
+ free(cpu_data);
}
} else {
buff->latency = true;
}
return 0;
+fail:
+ free(cpu_data);
+ return -1;
}
static int handle_options(struct tracecmd_input *handle)
--
2.35.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/2] trace-cmd library: Do not read CPU greater than CPUs registered
2022-03-12 23:40 [PATCH 0/2] trace-cmd library: Fix sparse cpu_data Steven Rostedt
2022-03-12 23:40 ` [PATCH 1/2] trace-cmd library: Make cpu_data[] match the cpus Steven Rostedt
@ 2022-03-12 23:40 ` Steven Rostedt
2022-03-14 14:59 ` [PATCH 0/2] trace-cmd library: Fix sparse cpu_data Yordan Karadzhov
2 siblings, 0 replies; 7+ messages in thread
From: Steven Rostedt @ 2022-03-12 23:40 UTC (permalink / raw)
To: linux-trace-devel; +Cc: Steven Rostedt (Google)
From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
If tracecmd_read_cpu_first() is called with a CPU that is greater than the
number of CPUs in the handle, just return NULL.
This happened when kernelshark would get the number of CPUs returned by
the tep handler, but they are not stored in the trace.dat file.
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
lib/trace-cmd/trace-input.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index da922834de1f..313534d09e86 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -2025,9 +2025,15 @@ int tracecmd_refresh_record(struct tracecmd_input *handle,
struct tep_record *
tracecmd_read_cpu_first(struct tracecmd_input *handle, int cpu)
{
+ unsigned long long page_offset;
int ret;
- ret = get_page(handle, cpu, handle->cpu_data[cpu].file_offset);
+ if (cpu > handle->cpus)
+ return NULL;
+
+ page_offset = calc_page_offset(handle, handle->cpu_data[cpu].file_offset);
+
+ ret = get_page(handle, cpu, page_offset);
if (ret < 0)
return NULL;
--
2.35.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 0/2] trace-cmd library: Fix sparse cpu_data
2022-03-12 23:40 [PATCH 0/2] trace-cmd library: Fix sparse cpu_data Steven Rostedt
2022-03-12 23:40 ` [PATCH 1/2] trace-cmd library: Make cpu_data[] match the cpus Steven Rostedt
2022-03-12 23:40 ` [PATCH 2/2] trace-cmd library: Do not read CPU greater than CPUs registered Steven Rostedt
@ 2022-03-14 14:59 ` Yordan Karadzhov
2022-03-14 16:01 ` Steven Rostedt
2 siblings, 1 reply; 7+ messages in thread
From: Yordan Karadzhov @ 2022-03-14 14:59 UTC (permalink / raw)
To: Steven Rostedt, linux-trace-devel
Hi Steven,
Is this fixing the issue with kernelshark crashing when trying to open the new format?
thanks,
Yordan
On 13.03.22 г. 1:40 ч., Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
>
> I have a new box that has 128 logical CPUs, and when I do trace-cmd record, as
> most of my CPUs are idle, they are not recorded into the CPU file.
>
> I had a test run that only had 35 of the 128 CPUs have data, and trace-cmd
> created a 35 cpu_data array to hold this information. But when I tried to load
> this into KernelShark, it crashed.
>
> It crashed because it uses the CPU number to retrieve information. Thus, when
> it asked for CPU 45 and the internal trace-cmd library only had 35 cpu_data
> items, it overlooked the array.
>
> Make the array non sparse. That is, if there's only one CPU in the file, and
> it's CPU 100, make an array of 101 (including CPU 0) where all the CPUs with
> no data just have a cpu_data filled with zeros.
>
> Also, fix tracecmd_read_cpu_first() return NULL if the CPU passed to it is
> greater than the number of items in the cpu_data array. This is because it
> would ask for cpu 64 but the last CPU with data in it was CPU 63, and
> trace-cmd library only made an array of 64 (0-63) items.
>
> Steven Rostedt (Google) (2):
> trace-cmd library: Make cpu_data[] match the cpus
> trace-cmd library: Do not read CPU greater than CPUs registered
>
> lib/trace-cmd/trace-input.c | 54 ++++++++++++++++++++++++++++++-------
> 1 file changed, 45 insertions(+), 9 deletions(-)
>
^ permalink raw reply [flat|nested] 7+ messages in thread