Re: [PATCH 2/6] tracing: Replace memcpy() with __get_task_comm()

[Steven Rostedt <rostedt@goodmis.org>]

On Mon, 3 Jun 2024 15:23:48 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Mon, 3 Jun 2024 at 15:18, Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > The logic behind __string() and __assign_str() will always add a NUL
> > character.  
> 
> Ok. But then you still end up with the issue that now the profiles are
> different, and you have a 8-byte pointer to dynamically allocated
> memory instead of just the simpler comm[TASK_COMM_LEN].

It's actually a 4 byte meta data that holds it.

	__data_offsets->item##_ptr_ = src;

The __data_offsets is a local helper structure that holds the information
about where the string data will be in the ring buffer event, while the
event is being recorded. The actual data in the ring buffer is a 4 byte
word, where 2 bytes is for the size of the string and 2 bytes is for the
offset into the event.

If you have a task->comm = "ps", that will take up 12 bytes in the ring buffer.

   field: 2 bytes: for where in the event the "ps" is.
          2 bytes: for the length of ps.

Then after the data, you have 3 or 4 bytes to hold "ps\0". (the data always
ends on a 4 byte alignment).

The amount of data in the ring buffer to hold "ps" just went from 16 bytes
down to 12 bytes, and nothing is truncated if we extend the size of comm.

> 
> Is that actually a good idea for tracing?
> 
> We're trying to fix the core code to be cleaner for places that may
> actually *care* (like 'ps').
> 
> Would we really want to touch this part of tracing?

Note, I've been wanting to get rid of the hard coded TASK_COMM_LEN from the
events for a while. As I mentioned before, the only reason the memcpy exists
is because it was added before the __string() logic was. Then it became
somewhat of a habit to do that for everything that referenced task->comm. :-/

-- Steve