[igt-dev] [PATCH i-g-t] tools/i915-perf-recorder: fix topology alignment issue

[Chris Wilson]

Quoting Lionel Landwerlin (2020-04-01 12:19:37)
> On 01/04/2020 14:15, Chris Wilson wrote:
> > Quoting Lionel Landwerlin (2020-04-01 11:36:06)
> >> The additional alignment added when writing into the output was not
> >> accounted in the header. This is preventing reading the recorded data.
> >>
> >> Instead of adding the alignment when writing, just account for it when
> >> querying the topology.
> >>
> >> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> >> Fixes: f08865e58cd3 ("tools: add i915 perf recorder tool")
> >> ---
> >>   tools/i915-perf/i915_perf_recorder.c | 10 ++--------
> >>   1 file changed, 2 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/tools/i915-perf/i915_perf_recorder.c b/tools/i915-perf/i915_perf_recorder.c
> >> index 4d729b0e..104a425b 100644
> >> --- a/tools/i915-perf/i915_perf_recorder.c
> >> +++ b/tools/i915-perf/i915_perf_recorder.c
> >> @@ -485,8 +485,8 @@ get_topology(int drm_fd, uint32_t *topology_size)
> >>                  return NULL;
> >>   
> >>          assert(item.length > 0);
> >> -       *topology_size = item.length;
> >> -       topo_info = malloc(item.length);
> >> +       *topology_size = ALIGN(item.length, 8);
> >> +       topo_info = malloc(*topology_size);
> >>          item.data_ptr = (uintptr_t) topo_info;
> > As I understand the problem statement, when we used the length the
> > kernel told us to use, the kernel rejected the query?
> >
> > Could you clarify as that sounds like a kernel bug.
> > -Chris
> 
> We get the result of the query from the kernel and write it to the file.
> 
> To keep structures in the file aligned to 8 bytes (so that we can mmap 
> stuff and it doesn't fall on weird alignments), the topology data size 
> has to be aligned as well.
> 
> 
> The problem is that we align the data when writing it to the file, but 
> fail to update the header.size field properly written just before the 
> topology data.

Gotcha, you're talking about the returned *topology_size not matching
what was fwritten to the pipe.

Ok, one problem I see here is that you are passing around uninitialised
bytes (for some value of problem).
-Chris