From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 43561C83F22 for ; Wed, 16 Jul 2025 20:18:46 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8EB1F6B009A; Wed, 16 Jul 2025 16:18:45 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8C2246B00A1; Wed, 16 Jul 2025 16:18:45 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7FF4E6B00A3; Wed, 16 Jul 2025 16:18:45 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 6E3A06B009A for ; Wed, 16 Jul 2025 16:18:45 -0400 (EDT) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 58E751603A3 for ; Wed, 16 Jul 2025 20:18:44 +0000 (UTC) X-FDA: 83671240968.25.9CC107F Received: from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56]) by imf16.hostedemail.com (Postfix) with ESMTP id 17A08180008 for ; Wed, 16 Jul 2025 20:18:40 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=igalia.com header.s=20170329 header.b=kmThNzHO; dmarc=pass (policy=none) header.from=igalia.com; spf=pass (imf16.hostedemail.com: domain of bhsharma@igalia.com designates 213.97.179.56 as permitted sender) smtp.mailfrom=bhsharma@igalia.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1752697122; a=rsa-sha256; cv=none; b=u/VIyU2hzLOcy+94MD0FOccPaBvTFXL7edKz5vgz9Nd3Y2Wt2rgrEubDnQdN2WPdWeV4Re CwuFHl34Je41sMxJvyqqSWXjj1X6XRH+N/r+DL/Mm/IpIBm8Vs04U9UkWSleeMermaLe93 JnjzSfZimSMJODmTxZDuLfA2US+45z0= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=igalia.com header.s=20170329 header.b=kmThNzHO; dmarc=pass (policy=none) header.from=igalia.com; spf=pass (imf16.hostedemail.com: domain of bhsharma@igalia.com designates 213.97.179.56 as permitted sender) smtp.mailfrom=bhsharma@igalia.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1752697122; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=hN5UcNOceAW9gJ/Y8MIabvr/9qWRaAS173z8GQgYkMI=; b=2TlOS9rbrn2eQcDd02yfT8MOGysIsVPaBOdzRVL7cPBzqCgMB2Q2LUHBOYtVuCXzK3bFu7 eQ7cJ1OBvYbIDoH2HTm72hW0d/FMlROO755lUOmsd1CdVWbTxr54gBg2uoF1+sRkLfIgNx 0FEng2+TH184JQpqTSLx/qjnacK38D8= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:From: References:Cc:To:Subject:MIME-Version:Date:Message-ID:Sender:Reply-To: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=hN5UcNOceAW9gJ/Y8MIabvr/9qWRaAS173z8GQgYkMI=; b=kmThNzHOvISsdaYQElDiHi8q5Q CwMCw5ANyqqa8r1lg+P+8naMfvfrJIkC/CWnWEQyqcFxuKeUJObRs7MriLA5B+cdBY9gPn1zIGH5E lnhtujB8iEsamDUPd7uNmY5vEFLqxUFpveusUW4xjZvcgeVxQ/pn0n9Mb5tHa4kilZXkEAFQ6XI42 8vACu5Yd512HTgNJXam34PFAWkrLc+hEIrAT4qxyDoMF7GylRRZru68eUVyllh+TrBbxfHNn321E3 O7CyIhXxOHXlRLYOyG227XMg+fwBp6x1lNWh/cFgFfeTvvLHtw3Bsy40e/GX9KVZE3v10/SSvZUQj NyRdXUjw==; Received: from [223.233.66.171] (helo=[192.168.1.12]) by fanzine2.igalia.com with esmtpsa (Cipher TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_128_GCM:128) (Exim) id 1uc8aA-00HTUe-A4; Wed, 16 Jul 2025 22:18:26 +0200 Message-ID: Date: Thu, 17 Jul 2025 01:48:17 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.5.0 Subject: Re: [PATCH v5 3/3] treewide: Switch from tsk->comm to tsk->comm_str which is 64 bytes long Content-Language: en-US To: Andrii Nakryiko , Bhupesh 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, mirq-linux@rere.qmqm.pl, peterz@infradead.org, willy@infradead.org, david@redhat.com, viro@zeniv.linux.org.uk, keescook@chromium.org, 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, linux-trace-kernel@vger.kernel.org, kees@kernel.org, torvalds@linux-foundation.org References: <20250716123916.511889-1-bhupesh@igalia.com> <20250716123916.511889-4-bhupesh@igalia.com> From: Bhupesh Sharma In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Stat-Signature: 8k459oc86jktk6qnrfdyk73xo4xyq7zw X-Rspam-User: X-Rspamd-Queue-Id: 17A08180008 X-Rspamd-Server: rspam02 X-HE-Tag: 1752697120-671595 X-HE-Meta: U2FsdGVkX19zb0Co5mQwaG21y5BKAZHztHV0esfTGG+bS48OPm46u4JGB4bCD6MoayFDzEZnnEuENnnRnVvaxBeVsnmhOzf9HNkiHxcZ/QQ8UNi+4/PNo0IJkMswUPPW9qNDDrlkSmGhb5OevXq8CC4bwrp78JGgSJfQUc2m2kH3cPu6534DhFVyEZOmbUY5Jgf0YzxO0RPNoy1klpGsRbKXT7efW9hjXTNE+QyOD6xu84IKeyfrNWN48h4bfDVlS9k/Uy1Uc92ft1y9OPc6dTAn0HUYWntQ8SsoI7bqZPw3pqbAOii2CkTitvcRiVZDCM3lt6E1IQzqYvsUEwirqtTq6YKG7q2nxuBiKWnL5hBlPH9nhFHgR1UKjN/4DFRTE0HxelAYf7VO1olMsjWKBEH7eAX/z/5mi1Sye3ODXGQh4jfDDjT9tN4cdn837uXVxLwcCuOeuNRTLTLWCXXAKMmdVxv/QsGL7B6nmaXwUyBrswWQI8srJWpxyqHsdNaYX3ItK22yMGrVzJliSZRbOWy/5i3Kq1r+jDUB/MbN+yYq6xoeAb3kDaDS5xLT4IvWeDoxL8rHogyPYtZ3dgbh+bxBuOLKKorUmGqDUSRA6ca3g2DEkxy6Ya1ZUvXzdgKEmSNWKTOWtktsKUg4qU129vO542Qfu56d/1eHGErI7fSKuMWagBhrCMwkAppvqxEq1F14wp0VqEHg7xTfzmykpkil6uP0sOY12cnK2sjo7M0hzMjmmtX7xJaiTN8fhYgfzsvJ5CAW3MjPhsWx2YgT6iK2d5cvCSWMZ33rqhSkvGlg6bqdA3BC0EcBNVpODFEbSCrF6bgoNPx5k9XV/N37cBMpUthOrQCjtaCEFwK3kJwi9b32JgMrvIEAjLZzSfErqRBzUz/mCoAxcNtKix8xiUcqLpbPaOP2HvXDB6WMMNZ/7anXInmn8CAdzl0cFYfA219g73LN4uwjREUFfyA bEY+8wst gxvyPCzCd4lLChlxTBT8PPRthnWigUS5ldJm2U/hvghpwN4NruLoO1IW8u+OfVJ24CUWbKsmTEqMbSTNNUfPpXETduVMaWE6UivSMJu2/lSJj/Pxsko129aZokk6KgV3okTfcwwarj8O9rcdn8vyLGFSvasb7Gs47WyBxz2FyzycbNhwWTt0l3JsaaRM8ecfpye+F+uQ3JMw5l8Giup+2hODU4KnWnzczIvSq9kF54tjVv/J44JLNLoIqWzK/bM6nEOeZEeIk6C4Zya7EBNQhPkGi3E+9K9D9gD5+1YenO0c2UiBEvqKmYGVuHDggQ/+AzY+mAgGlOprugWNdJ5/qRGKdg6yF+FxvhW3dzhz7AF6o/YIFCX1g5IrN+FZxZMpuJSlLMIcUwQqbzMjMbxt4iQuCPLke4eWGzfFzzFF+RTi9syXsgmOjFUIQadS96NQIANHgbuKEqfcRbXNrEAeU/cS0gbvnAjlZa7yh9wpYdDrF9F77c1jrBdKEig== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 7/16/25 11:40 PM, Andrii Nakryiko wrote: > On Wed, Jul 16, 2025 at 5:40 AM Bhupesh wrote: >> Historically due to the 16-byte length of TASK_COMM_LEN, the >> users of 'tsk->comm' are restricted to use a fixed-size target >> buffer also of TASK_COMM_LEN for 'memcpy()' like use-cases. >> >> To fix the same, Kees suggested in [1] that we can replace tsk->comm, >> with tsk->comm_str, inside 'task_struct': >> union { >> char comm_str[TASK_COMM_EXT_LEN]; >> }; >> >> where TASK_COMM_EXT_LEN is 64-bytes. > Do we absolutely have to rename task->comm field? I understand as an > intermediate step to not miss any existing users in the kernel > sources, but once that all is done, we can rename that back to comm, > right? > > The reason I'm asking is because accessing task->comm is *very common* > with all sorts of BPF programs and scripts. Yes, we have way to deal > with that with BPF CO-RE, but every single use case would need to be > updated now to work both with task->comm name on old kernels and > task->comm_str on new kernels (because BPF programs are written in > more or less kernel version agnostic way, so they have to handle many > kernel releases). > > So, unless absolutely necessary, can we please keep the field name the > same? Changing the size of that field is not really a problem for BPF, > so no objections against that. So, as a background we have had several previous discussions regarding renaming the 'tsk->comm' to 'task->comm_str' on the list (please see [1] and [2] for details), and as per Kees's recommendations we have taken this approach in the v5 patchset (may be Kees can add further details if I have missed adding something in the log message above). That being said, ideally one would not like to break any exiting ABI (which includes existing / older BPF programs). I was having a look at the BPF CO_RE reference guide (see [3]), and was able to make out that BPF CO_RE has a definition of |s||truct task_struct|which doesn't need to match the kernel's struct task_struct definition exactly (as [3] mentions -> only a necessary subset of fields have to be present and compatible): |struct task_struct { intpid; charcomm[16]; struct task_struct*group_leader; } __attribute__((preserve_access_index)); | So, if we add a check here to add  '|charcomm[16]' or||charcomm_str[16]' to BPF CO RE's internal 'struct task_struct' on basis of the underlying kernel version being used (something like using  'KERNEL_VERSION(x, y, 0)' for example), will that suffice? I have ||used and seen these checks being used in the user-space applications (for example, see [4]) at several occasions. Please let me know your views. |[1]. https://lore.kernel.org/all/202505222041.B639D482FB@keescook/ [2]. https://lore.kernel.org/all/ba4ddf27-91e7-0ecc-95d5-c139f6978812@igalia.com/ [3]. https://nakryiko.com/posts/bpf-core-reference-guide/ [4]. https://github.com/crash-utility/crash/blob/master/memory_driver/crash.c#L41C25-L41C49 Thanks. >> And then modify 'get_task_comm()' to pass 'tsk->comm_str' >> to the existing users. >> >> This would mean that ABI is maintained while ensuring that: >> >> - Existing users of 'get_task_comm'/ 'set_task_comm' will get 'tsk->comm_str' >> truncated to a maximum of 'TASK_COMM_LEN' (16-bytes) to maintain ABI, >> - New / Modified users of 'get_task_comm'/ 'set_task_comm' will get >> 'tsk->comm_str' supported for a maximum of 'TASK_COMM_EXTLEN' (64-bytes). >> >> Note, that the existing users have not been modified to migrate to >> 'TASK_COMM_EXT_LEN', in case they have hard-coded expectations of >> dealing with only a 'TASK_COMM_LEN' long 'tsk->comm_str'. >> >> [1]. https://lore.kernel.org/all/202505231346.52F291C54@keescook/ >> >> Signed-off-by: Bhupesh >> --- >> arch/arm64/kernel/traps.c | 2 +- >> arch/arm64/kvm/mmu.c | 2 +- >> block/blk-core.c | 2 +- >> block/bsg.c | 2 +- >> drivers/char/random.c | 2 +- >> drivers/hid/hid-core.c | 6 +++--- >> drivers/mmc/host/tmio_mmc_core.c | 6 +++--- >> drivers/pci/pci-sysfs.c | 2 +- >> drivers/scsi/scsi_ioctl.c | 2 +- >> drivers/tty/serial/serial_core.c | 2 +- >> drivers/tty/tty_io.c | 8 ++++---- >> drivers/usb/core/devio.c | 16 ++++++++-------- >> drivers/usb/core/message.c | 2 +- >> drivers/vfio/group.c | 2 +- >> drivers/vfio/vfio_iommu_type1.c | 2 +- >> drivers/vfio/vfio_main.c | 2 +- >> drivers/xen/evtchn.c | 2 +- >> drivers/xen/grant-table.c | 2 +- >> fs/binfmt_elf.c | 2 +- >> fs/coredump.c | 4 ++-- >> fs/drop_caches.c | 2 +- >> fs/exec.c | 8 ++++---- >> fs/ext4/dir.c | 2 +- >> fs/ext4/inode.c | 2 +- >> fs/ext4/namei.c | 2 +- >> fs/ext4/super.c | 12 ++++++------ >> fs/hugetlbfs/inode.c | 2 +- >> fs/ioctl.c | 2 +- >> fs/iomap/direct-io.c | 2 +- >> fs/jbd2/transaction.c | 2 +- >> fs/locks.c | 2 +- >> fs/netfs/internal.h | 2 +- >> fs/proc/base.c | 2 +- >> fs/read_write.c | 2 +- >> fs/splice.c | 2 +- >> include/linux/coredump.h | 2 +- >> include/linux/filter.h | 2 +- >> include/linux/ratelimit.h | 2 +- >> include/linux/sched.h | 11 ++++++++--- >> init/init_task.c | 2 +- >> ipc/sem.c | 2 +- >> kernel/acct.c | 2 +- >> kernel/audit.c | 4 ++-- >> kernel/auditsc.c | 10 +++++----- >> kernel/bpf/helpers.c | 2 +- >> kernel/capability.c | 4 ++-- >> kernel/cgroup/cgroup-v1.c | 2 +- >> kernel/cred.c | 4 ++-- >> kernel/events/core.c | 2 +- >> kernel/exit.c | 6 +++--- >> kernel/fork.c | 9 +++++++-- >> kernel/freezer.c | 4 ++-- >> kernel/futex/waitwake.c | 2 +- >> kernel/hung_task.c | 10 +++++----- >> kernel/irq/manage.c | 2 +- >> kernel/kthread.c | 2 +- >> kernel/locking/rtmutex.c | 2 +- >> kernel/printk/printk.c | 2 +- >> kernel/sched/core.c | 22 +++++++++++----------- >> kernel/sched/debug.c | 4 ++-- >> kernel/signal.c | 6 +++--- >> kernel/sys.c | 6 +++--- >> kernel/sysctl.c | 2 +- >> kernel/time/itimer.c | 4 ++-- >> kernel/time/posix-cpu-timers.c | 2 +- >> kernel/tsacct.c | 2 +- >> kernel/workqueue.c | 6 +++--- >> lib/dump_stack.c | 2 +- >> lib/nlattr.c | 6 +++--- >> mm/compaction.c | 2 +- >> mm/filemap.c | 4 ++-- >> mm/gup.c | 2 +- >> mm/memfd.c | 2 +- >> mm/memory-failure.c | 10 +++++----- >> mm/memory.c | 2 +- >> mm/mmap.c | 4 ++-- >> mm/oom_kill.c | 18 +++++++++--------- >> mm/page_alloc.c | 4 ++-- >> mm/util.c | 2 +- >> net/core/sock.c | 2 +- >> net/dns_resolver/internal.h | 2 +- >> net/ipv4/raw.c | 2 +- >> net/ipv4/tcp.c | 2 +- >> net/socket.c | 2 +- >> security/lsm_audit.c | 4 ++-- >> 85 files changed, 171 insertions(+), 161 deletions(-) >> > [...]