Re: [PATCH v2] kthread: dynamically allocate memory to store kthread's full name

From: Petr Mladek
Date: Thu Nov 25 2021 - 09:48:19 EST


On Thu 2021-11-25 10:36:49, David Hildenbrand wrote:
> On 20.11.21 12:28, Yafang Shao wrote:
> > When I was implementing a new per-cpu kthread cfs_migration, I found the
> > comm of it "cfs_migration/%u" is truncated due to the limitation of
> > TASK_COMM_LEN. For example, the comm of the percpu thread on CPU10~19 are
> > all with the same name "cfs_migration/1", which will confuse the user. This
> > issue is not critical, because we can get the corresponding CPU from the
> > task's Cpus_allowed. But for kthreads correspoinding to other hardware
> > devices, it is not easy to get the detailed device info from task comm,
> > for example,
> >
> > jbd2/nvme0n1p2-
> > xfs-reclaim/sdf
> >
> > Currently there are so many truncated kthreads:
> >
> > rcu_tasks_kthre
> > rcu_tasks_rude_
> > rcu_tasks_trace
> > poll_mpt3sas0_s
> > ext4-rsv-conver
> > xfs-reclaim/sd{a, b, c, ...}
> > xfs-blockgc/sd{a, b, c, ...}
> > xfs-inodegc/sd{a, b, c, ...}
> > audit_send_repl
> > ecryptfs-kthrea
> > vfio-irqfd-clea
> > jbd2/nvme0n1p2-
> > ...
> >
> > We can shorten these names to work around this problem, but it may be
> > not applied to all of the truncated kthreads. Take 'jbd2/nvme0n1p2-' for
> > example, it is a nice name, and it is not a good idea to shorten it.
> >
> > One possible way to fix this issue is extending the task comm size, but
> > as task->comm is used in lots of places, that may cause some potential
> > buffer overflows. Another more conservative approach is introducing a new
> > pointer to store kthread's full name if it is truncated, which won't
> > introduce too much overhead as it is in the non-critical path. Finally we
> > make a dicision to use the second approach. See also the discussions in
> > this thread:
> > https://lore.kernel.org/lkml/20211101060419.4682-1-laoar.shao@xxxxxxxxx/
> >
> > After this change, the full name of these truncated kthreads will be
> > displayed via /proc/[pid]/comm:
> >
> > rcu_tasks_kthread
> > rcu_tasks_rude_kthread
> > rcu_tasks_trace_kthread
> > poll_mpt3sas0_statu
> > ext4-rsv-conversion
> > xfs-reclaim/sdf1
> > xfs-blockgc/sdf1
> > xfs-inodegc/sdf1
> > audit_send_reply
> > ecryptfs-kthread
> > vfio-irqfd-cleanup
> > jbd2/nvme0n1p2-8
>
> I do wonder if that could break some user space that assumes these names
> have maximum length ..

There is high chance that we will be on the safe side. Workqueue
kthreads already provided longer names. They are even dynamic
because the currently handled workqueue name is part of the name,
see wq_worker_comm().

Best Regards,
Petr