Re: [PATCH 2/2] tracing: Keep pid and comm[] in the same structure

[Steven Rostedt <rostedt@goodmis.org>]

On Fri, 26 Jun 2026 22:23:56 +0100
David Laight <david.laight.linux@gmail.com> wrote:

> Rather than have two separate dynamic arrays on the end of struct
> saved_commandlines_buffer have a single dynamic array where each
> entry contains the pid and associated task->comm[].
> This simplifies the initialisation and lookup.
> 
> Don't bother trying to initialise the pid field no a non-zero value,
> it only matters in the tracing_saved_cmdlines_seq_ops code.
> Allocate entry [0] first so that the tracing_saved_cmdlines_seq_ops
> code can just index the array with the file offset.
> 
> The code now uses the correct size when determining the page 'order'
> to free the structure. The smaller size will always give the same
> 'order'.
> 
> Signed-off-by: David Laight <david.laight.linux@gmail.com>
> ---
> 
> Is there any reason why this code uses alloc_pages() rather
> than vmalloc()?

It's been a long time since I worked on this, but IIRC, it was to keep
the pressure down on the TLB when tracing. It updates at every
sched_switch that has a trace event occurring so, I likely used normal
pages which are part of the huge pages the kernel sets up and doesn't
affect the TLB as much. vmalloc does have impact on the TLB pressure,
and tracing should always try to avoid that.

> map_pid_to_cmdline[] is 64k*sizeof(int) so the whole structure
> expands to 512k with about 64k/20 (about 3200) pid entries even
> though the default is 128.

That's because it is not dynamic. That array needs to be able to hold
most PIDs. The default is 128 but it will expand to how much it can
hold to allocate the full map_pid_to_cmdline. The real default for 4098
page sized architectures is 6552 entries.


> AFAICT there is only one copy of the data - so it could be static.
> Perhaps with pointers to map_pid_cmdline[] and (after this patch)
> pid_comm[], both of which could be separately resized.

map_pid_t_cmdline[] is to hold the PID_MAX_DEFAULT amount of PIDs to
avoid collisions. I wouldn't resize it.

> 
> I also noticed that map_pid_to_cmdline[] contains indexes into
> pid_comm[], restricting these to 16bits would half the data area.

Hmm, yeah, this could be useful, as it doesn't appear one could make
saved_cmdline_size greater than 65536 (or even close to that).

-- Steve