On Tue, 29 Jun 2021 17:34:05 -0700 Paul Burton <paulburton@xxxxxxxxxx> wrote: > The tgid_map array records a mapping from pid to tgid, where the index > of an entry within the array is the pid & the value stored at that index > is the tgid. > > The saved_tgids_next() function iterates over pointers into the tgid_map > array & dereferences the pointers which results in the tgid, but then it > passes that dereferenced value to trace_find_tgid() which treats it as a > pid & does a further lookup within the tgid_map array. It seems likely > that the intent here was to skip over entries in tgid_map for which the > recorded tgid is zero, but instead we end up skipping over entries for > which the thread group leader hasn't yet had its own tgid recorded in > tgid_map. > > A minimal fix would be to remove the call to trace_find_tgid, turning: > > if (trace_find_tgid(*ptr)) > > into: > > if (*ptr) > > ..but it seems like this logic can be much simpler if we simply let > seq_read() iterate over the whole tgid_map array & filter out empty > entries by returning SEQ_SKIP from saved_tgids_show(). Here we take that > approach, removing the incorrect logic here entirely. > > Signed-off-by: Paul Burton <paulburton@xxxxxxxxxx> > Fixes: d914ba37d714 ("tracing: Add support for recording tgid of tasks") > Cc: Steven Rostedt <rostedt@xxxxxxxxxxx> > Cc: Ingo Molnar <mingo@xxxxxxxxxx> > Cc: Joel Fernandes <joelaf@xxxxxxxxxx> > Cc: <stable@xxxxxxxxxxxxxxx> > --- Joel, Can you review this please. -- Steve