linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Bhupesh Sharma <bhsharma@igalia.com>
To: Andres Rodriguez <andresx7@gmail.com>,
	Kees Cook <kees@kernel.org>, Bhupesh <bhupesh@igalia.com>
Cc: akpm@linux-foundation.org, kernel-dev@igalia.com,
	linux-kernel@vger.kernel.org, bpf@vger.kernel.org,
	linux-perf-users@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-mm@kvack.org, oliver.sang@intel.com, lkp@intel.com,
	laoar.shao@gmail.com, pmladek@suse.com, rostedt@goodmis.org,
	mathieu.desnoyers@efficios.com, arnaldo.melo@gmail.com,
	alexei.starovoitov@gmail.com, andrii.nakryiko@gmail.com,
	mirq-linux@rere.qmqm.pl, peterz@infradead.org,
	willy@infradead.org, david@redhat.com, viro@zeniv.linux.org.uk,
	ebiederm@xmission.com, brauner@kernel.org, jack@suse.cz,
	mingo@redhat.com, juri.lelli@redhat.com, bsegall@google.com,
	mgorman@suse.de, vschneid@redhat.com
Subject: Re: [PATCH RFC 0/2] Dynamically allocate memory to store task's full name
Date: Tue, 18 Mar 2025 16:49:28 +0530	[thread overview]
Message-ID: <8b11d5f6-bb16-7af6-8377-bb0951fcfb60@igalia.com> (raw)
In-Reply-To: <a73ea646-0a24-474a-9e14-d59ea5eaa662@gmail.com>

Hi,

Thanks for the review and inputs on the additional possible use-cases.
Please see my replies inline.

On 3/15/25 1:13 PM, Andres Rodriguez wrote:
>
>
> On 3/14/25 14:25, Kees Cook wrote:
>> On Fri, Mar 14, 2025 at 10:57:13AM +0530, Bhupesh wrote:
>>> While working with user-space debugging tools which work especially
>>> on linux gaming platforms, I found that the task name is truncated due
>>> to the limitation of TASK_COMM_LEN.
>>>
>>> For example, currently running 'ps', the task->comm value of a long
>>> task name is truncated due to the limitation of TASK_COMM_LEN.
>>>      create_very_lon
>>>
>>> This leads to the names passed from userland via pthread_setname_np()
>>> being truncated.
>>
>> So there have been long discussions about "comm", and it mainly boils
>> down to "leave it alone". For the /proc-scraping tools like "ps" and
>> "top", they check both "comm" and "cmdline", depending on mode. The more
>> useful (and already untruncated) stuff is in "cmdline", so I suspect it
>> may make more sense to have pthread_setname_np() interact with that
>> instead. Also TASK_COMM_LEN is basically considered userspace ABI at
>> this point and we can't sanely change its length without breaking the
>> world.
>>
>
> Completely agree that comm is best left untouched. TASK_COMM_LEN is 
> embedded into the kernel and the pthread ABI changes here should be 
> avoided.
>

So, basically my approach _does not_ touch TASK_COMM_LEN at all. The 
normal 'TASK_COMM_LEN' 16byte design remains untouched.
Which means that all the legacy / existing ABi which uses 'task->comm' 
and hence are designed / written to handle 'TASK_COMM_LEN' 16-byte name, 
continue to work as before using '/proc/$pid/task/$tid/comm'.

This change-set only adds a _parallel_ dynamically allocated 
'task->full_name' which can be used by interested users via 
'/proc/$pid/task/$tid/full_name'.

[PATCH 2/2] shows only a possible use-case of the same and can be 
dropped with only [PATCH 1/2] being considered to add the 
'/proc/$pid/task/$tid/full_name' interface.
>> Best to use /proc/$pid/task/$tid/cmdline IMO...
>
> Your recommendation works great for programs like ps and top, which are
> the examples proposed in the cover letter. However, I think the 
> opening email didn't point out use cases where the name is modified at 
> runtime. In those cases cmdline would be an unsuitable solution as it 
> should remain immutable across the process lifetime. An example of 
> this use case would be to set a thread's name for debugging purposes 
> and then trying to query it via gdb or perf.
>
> I wrote a quick and dirty example to illustrate what I mean:
> https://github.com/lostgoat/tasknames
>
> I think an alternative approach could be to have a separate entry in 
> procfs to store a tasks debug name (and leave comm completely 
> untouched), e.g. /proc/$pid/task/$tid/debug_name. This would allow 
> userspace apps to be updated with the following logic:
>
> get_task_debug_name() {
>     if ( !is_empty( debug_name ) )
>         return debug_name;
>     return comm;
> }
>
> "Legacy" userspace apps would remain ABI compatible as they would just 
> fall back to comm. And apps that want to opt in to the new behaviour 
> can be updated one at a time. Which would be work intensive, but even 
> just updating gdb and perf would be super helpful.

I am fine with adding either '/proc/$pid/task/$tid/full_name' or 
'/proc/$pid/task/$tid/debug_name' (actually both of these achieve the same).
The new / modified users (especially the debug applications you listed 
above) can switch easily to using '/proc/$pid/task/$tid/full_name' 
instead of ''/proc/$pid/task/$tid/comm'

AFAIK we already achieved for the kthreads using d6986ce24fc00 
("kthread: dynamically allocate memory to store kthread's full name"), 
which adds 'full_name' in parallel to 'comm' for kthread names.

Thanks,
Bhupesh


  reply	other threads:[~2025-03-18 11:20 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-14  5:27 Bhupesh
2025-03-14  5:27 ` [PATCH RFC 1/2] exec: " Bhupesh
2025-03-14  5:27 ` [PATCH RFC 2/2] fs/proc: Pass 'task->full_name' via 'proc_task_name()' Bhupesh
2025-03-14 21:25 ` [PATCH RFC 0/2] Dynamically allocate memory to store task's full name Kees Cook
2025-03-15  7:43   ` Andres Rodriguez
2025-03-18 11:19     ` Bhupesh Sharma [this message]
2025-03-18 15:51       ` Kees Cook
2025-03-18 18:06         ` Bhupesh Sharma

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8b11d5f6-bb16-7af6-8377-bb0951fcfb60@igalia.com \
    --to=bhsharma@igalia.com \
    --cc=akpm@linux-foundation.org \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andresx7@gmail.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=arnaldo.melo@gmail.com \
    --cc=bhupesh@igalia.com \
    --cc=bpf@vger.kernel.org \
    --cc=brauner@kernel.org \
    --cc=bsegall@google.com \
    --cc=david@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=jack@suse.cz \
    --cc=juri.lelli@redhat.com \
    --cc=kees@kernel.org \
    --cc=kernel-dev@igalia.com \
    --cc=laoar.shao@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=lkp@intel.com \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=mirq-linux@rere.qmqm.pl \
    --cc=oliver.sang@intel.com \
    --cc=peterz@infradead.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=vschneid@redhat.com \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox