linux-kernel - Re: [PATCH workqueue/for-6.10-fixes] workqueue: Refactor worker ID formatting and make wq_worker

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALOAHbCYpV1ubO3Z3hjMWCQnSmGd9-KYARY29p9OnZxMhXKs4g@mail.gmail.com>
Date: Tue, 21 May 2024 10:33:37 +0800
From: Yafang Shao <laoar.shao@...il.com>
To: Tejun Heo <tj@...nel.org>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>, Jan Engelhardt <jengelh@...i.de>, 
	Craig Small <csmall@....com.au>, linux-kernel@...r.kernel.org, 
	Lai Jiangshan <jiangshanlai@...il.com>
Subject: Re: [PATCH workqueue/for-6.10-fixes] workqueue: Refactor worker ID
 formatting and make wq_worker_comm() use full ID string

On Tue, May 21, 2024 at 6:30 AM Tejun Heo <tj@...nel.org> wrote:
>
> Currently, worker ID formatting is open coded in create_worker(),
> init_rescuer() and worker_thread() (for %WORKER_DIE case). The formatted ID
> is saved into task->comm and wq_worker_comm() uses it as the base name to
> append extra information to when generating the name to be shown as.
>
> However, TASK_COMM_LEN is only 16 leading to badly truncated names for
> rescuers. For example, the rescuer for the inet_frag_wq workqueue becomes:
>
>   $ ps -ef | grep '[k]worker/R-inet'
>   root         483       2  0 Apr26 ?        00:00:00 [kworker/R-inet_]
>
> Even for non-rescue workers, it's easy to run over 15 characters on
> moderately large machines.
>
> Fit it by consolidating worker ID formatting into a new helper
> format_worker_id() and calling it from wq_worker_comm() to obtain the
> untruncated worker ID.
>
>   $ ps -ef | grep '[k]worker/R-inet'
>   root          60       2  0 12:10 ?        00:00:00 [kworker/R-inet_frag_wq]
>
> Signed-off-by: Tejun Heo <tj@...nel.org>
> Reported-by: Jan Engelhardt <jengelh@...i.de>
> Suggested-by: Linus Torvalds <torvalds@...ux-foundation.org>
> ---
> (cc'ing LKML and Lai)
>
> On Mon, May 20, 2024 at 10:17:00AM -0700, Linus Torvalds wrote:
> > Oh well. I suspect this would be trivial to fix. I think the
> > get_kthread_comm() should use the full name, and only *then* attach
> > the extra worker pool information if it exists..
> >
> > Tejun?
>
> Yeah, looks like even the unbound worker IDs are getting truncated on larger
> machines. This patch should fix it. I'll apply it to wq/for-6.10-fixes soon.
>
> > Also, independently of the WQ worker issue, I do think we could
> > possibly expand TASK_COMM_LEN a bit more. 16 bytes is too short for
> > user-level names too, and while it's seldom used for 'ps', it's
> > visible in things like oops messages etc where it gets truncated.
>
> That'd be great. ISTR this being discussed a while ago. Am I imagining that
> we were going to raise this to 32?

We discussed extending it to 24 characters several years ago [0], but
some userspace tools might break. Therefore, we ultimately decided to
dynamically allocate the kthread's full name instead[1].

[0] https://lore.kernel.org/linux-mm/20211101060419.4682-1-laoar.shao@gmailcom/
[1] https://lore.kernel.org/linux-mm/20211120112738.45980-1-laoar.shao@gmail.com/