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 9C8DAC83F1B for ; Wed, 16 Jul 2025 20:47:30 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3F0598D0003; Wed, 16 Jul 2025 16:47:30 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3C8088D0001; Wed, 16 Jul 2025 16:47:30 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 304898D0003; Wed, 16 Jul 2025 16:47:30 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 2176A8D0001 for ; Wed, 16 Jul 2025 16:47:30 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 629F05970C for ; Wed, 16 Jul 2025 20:47:29 +0000 (UTC) X-FDA: 83671313418.19.3D64902 Received: from mail-pj1-f53.google.com (mail-pj1-f53.google.com [209.85.216.53]) by imf15.hostedemail.com (Postfix) with ESMTP id AAC49A0002 for ; Wed, 16 Jul 2025 20:47:27 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=Le3YPRah; spf=pass (imf15.hostedemail.com: domain of andrii.nakryiko@gmail.com designates 209.85.216.53 as permitted sender) smtp.mailfrom=andrii.nakryiko@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1752698847; 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=YfwkMeGXYqzQAGqk8KkRHnOsvdBHdZtOcGBtO2IOIII=; b=qjAsJsFSkPonKWhjNbhyAcjsVEYE0Y1SC1NsU1WyYhSkPggsipKZGdak0BTQn2ufandUK7 YMGkYg0gwCHc+ycNWfl81DlKjOuqZn95Vd7NjCoHGviIgO1FIhTRWKlCdz0X3ASiP+6v6M Yn4qd23xK7NIjW0Ju8UbCmOahsuTlGY= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=Le3YPRah; spf=pass (imf15.hostedemail.com: domain of andrii.nakryiko@gmail.com designates 209.85.216.53 as permitted sender) smtp.mailfrom=andrii.nakryiko@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1752698847; a=rsa-sha256; cv=none; b=opvaNyLTxVlTitDWdDS8CF+d1RhSHcUknWycCmvlQvj1ryleiIJvM3VsI/khYEeM2X6LxP osVzjOsS36AkZEwbF+m3dA7NNrFS5zPoYSq7fDegrQlDWtQaGQXKIDfL9X8o8hInsRbMCA cIKKEBob75loncpPkjR85SXSTa+zans= Received: by mail-pj1-f53.google.com with SMTP id 98e67ed59e1d1-311da0bef4aso344809a91.3 for ; Wed, 16 Jul 2025 13:47:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1752698846; x=1753303646; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=YfwkMeGXYqzQAGqk8KkRHnOsvdBHdZtOcGBtO2IOIII=; b=Le3YPRahK0ZmtBd7dC9/tc/Nvl8XaZOezj/hb8Kbv1PyXJr/3DjOzBM4M0imb0Sc3x cIupuhLz6kyj/cgKaS/eYxUgU2xJ4ywCDvhDHsz5z6qYIgwGxkHEvsIMKG6cze4bVS+n FYMcNrVIBVPEmCU+71+1qTHnJRat5pFOHqoWXRGkq3rLmSSsv3uVkCb5/1N5tMJEIQjR TNLLgMKHkFqVhD8X0zgHAcbbczwicKrx0WNlqkDEuW54rYUYKhQdiWG0PxQ/15PNDva1 BgtH2N72nRR7bZu0Bmc2+hu/wdny9IVmsjvhvEo7EWee37HkzWcuGmP4pedGIleBqF+e ZMBw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1752698846; x=1753303646; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=YfwkMeGXYqzQAGqk8KkRHnOsvdBHdZtOcGBtO2IOIII=; b=Xni0/Ub3YTQh01CnaYBlgpYP4Rk/JmBw2WBLqf6HMV3aKGdcO4Ar7hU/t/mY+gwm2R 0lX8Ikr5OHOnJ7PrLKjmIE7UjM2mcbl7YxAq4BJcCaPatHxHfInwO/Pze3tAPY+2j1NB v8QjOs8kxn7pO0ZvL2B6t0FweKkWIvWb5bvefPcrnx5BcD3xm0xHOtKKuPA3UkfPtXeU gqo2QqojsSA2XVLMzsDkO5TYeNSVcOBX3S3ZlF/n9eTru4koqelHozMN6k7BWDb0PoBu CEzo/jSiCMt9WZKGtl70ZeblSnfC2ZejYjUzHcFNGNULTu1O7RH/bmJKGGIu50O/xq3I kOSQ== X-Forwarded-Encrypted: i=1; AJvYcCWGxl2ToCb2WQ3gDl5sbI04RhmAh0Y1dOpMY5uT/vMlbKomYAWen/6mn+lH+UmzuN7DZVc19ueP+Q==@kvack.org X-Gm-Message-State: AOJu0YzcBItzkxJ3x7340qVLJc8LBW6uZlA6BB2uCf/5dRxsOUOBurEf vdv2OMJVQCjdRL67Yax5Uc/Krzjg85GhBm90BDml6REO5YGbRVF6ji1myFx2q2sqCVyGt1BgwM7 MAibT6eQUZJoJjtFDLieV3VgifxOCBxY= X-Gm-Gg: ASbGncs8i+Un/qtu1bAk+Pipw2vqSfIJODrTLX5KmENcgjnqz9Tns6yMCugVZFy7Idu IOqyB5PZZSg/MqOh+8YDdtZmAwy/oErXXyDbTtaFFYE9t/mWZR8Ep3hLP/W17jAlPv2WL3GWk7S nudC1NxmbS5WVuZK13YbVFfRYcJZRkXqzASXXXNWomIo3gDgARfHhk3gWFduRTd6dgBLH0dO2vL x/xdg== X-Google-Smtp-Source: AGHT+IFeFCZTHwQq6CWNE4DeVyMG40lwDfFF0lAyew534oOu3sXQDYX/YiTMj9puYV6XchTRtToJMlDgmQp2I20b5wI= X-Received: by 2002:a17:90b:270b:b0:312:e8ed:763 with SMTP id 98e67ed59e1d1-31c9f42412cmr4868436a91.22.1752698846319; Wed, 16 Jul 2025 13:47:26 -0700 (PDT) MIME-Version: 1.0 References: <20250716123916.511889-1-bhupesh@igalia.com> <20250716123916.511889-4-bhupesh@igalia.com> In-Reply-To: From: Andrii Nakryiko Date: Wed, 16 Jul 2025 13:47:14 -0700 X-Gm-Features: Ac12FXzdMIUrwTNSRAbXY9z4R-Vg46K1zjuhZoodouCKkxjzhdaHWwCSZWFsfng Message-ID: Subject: Re: [PATCH v5 3/3] treewide: Switch from tsk->comm to tsk->comm_str which is 64 bytes long To: Bhupesh Sharma Cc: Bhupesh , 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 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: mzixc5usq9p1yykapc1kzbjbpn8ocmdx X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: AAC49A0002 X-Rspam-User: X-HE-Tag: 1752698847-849731 X-HE-Meta: U2FsdGVkX1+6TCq93Km70OrMnij13TmJyamIIEonXLTatJ8M7S4pjrZb3SM2EincRuCBzMYPyTidElU5BLY+0QrQ4Khq+/zx1kxXMxsoeyt74mRpVAaFn2MQz53s8OECCLJ12ctrZQ8AbwkNPfPqZ1dXU9+4SWovAucyr3tAttgdS8Ncvq8MbUvFN0x06AUJ33VQHYu8VDIawK9KhQyIfOZ+8gAEOaSp3Af7uVxJ6sQobeoZqydWzitKXzvtbsrc400fZgfuZ83r6bZd/enSIo5309YguddtchDBhb4p+fP1k/vMLVWWUnBEFWSkpS/mLi+oM7brbmugwXojixfU4lv8UQhy0J4rR9FCG79Zpb1a1BgxJvtl+VobKecVRUZ/lBFtQtvg1LaeHtdeNCCAo6GyEtNAVdg5Pk0S8v62ZngbIJuuqbEN9vxT13XdNuo+QfH7ATEsghn4mQKPJgl2DZLIrrgyWOwP07QGdbGg8Hbj70fGGjHYYTXLd+LlSlzqEZIQ+WUjG5M8RFlXwNOG6KSu75oEsw/aMxW4lteSUhYbVlEtcAU0DTTEstJ4yoWYM6LOg/CI0deCV4uLZDkaA8EuI1FzInU+InXKca7Ve0yJy/adyUGRIHCJ1RdzwYKCMEFNORPLF3KeBY5IyV93MWcggyAJIys4s/KZlMo7W070M62aBEg2ghl6P0oJy5COdYCp66QzeD3tC/Glcyyng2vmAQsg+u4pUaEmFBdRazRhRXNIsc+xFmU7G6LYpxbivVeMduq9MgchOjJ2eNCO8s2dtLCghjF/e81WgX+n8YmmtlWJk676sMy1NANs0Redsr9amEPqmL20cN3zr2L1DhdHwUjbjkdarIhyT8kOrrU+0+VT2YZ9yJI++L+QJ3L0N8oRxRzk755FOEIHQayYl9v0BeUIpEl6oGFlHXn6Jc/PufN5YF/6Y6JdyjPRw0Z/ziYx2pFb8KlDA+FTtHV UV3Qi8LL xiJmWBW1ebFsKFB/rKXhp7sa2AuRlKvimhV4pk0zy5hQ6kNppHpAj1E6lFxj2BqLAVdp5yyg6Y6OXi1hMrO1lY3/FacwhS20mmzj4VRTFHysurgSjQf6uiX7osAFIYz7CF+tTY6em5f/XuwcBh1I57Req7sNRo2g1s586ddSGk8sVZT8= 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 Wed, Jul 16, 2025 at 1:18=E2=80=AFPM Bhupesh Sharma wrote: > > On 7/16/25 11:40 PM, Andrii Nakryiko wrote: > > On Wed, Jul 16, 2025 at 5:40=E2=80=AFAM Bhupesh wr= ote: > >> 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 kernel-internal data structures are not ABI to BPF programs, even though they do look into kernel internals routinely for observability and other reasons, so there is no ABI/API stability issue here. Having said that, unless absolutely necessary, renaming something so commonly used as task->comm is just an unnecessary widespread disruption, which ideally we just avoid, unless absolutely necessary. > (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. > It's simpler on BPF side, we can check the existence of task_struct's comm field, and handle either task->comm or task->comm_str accordingly. This has been done many times for various things that evolved in the kernel. But given how frequently task->comm is referenced (pretty much any profiler or tracer will capture this), it's just the widespread nature of accessing task->comm in BPF programs/scripts that will cause a lot of adaptation churn. And given the reason for renaming was to catch missing cases during refactoring, my ask was to do this renaming locally, validate all kernel code was modified, and then switch the field name back to "comm" (which you already did, so the remaining part would be just to rename comm_str back to comm). Unless I'm missing something else, of course. > 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.c= om/ > [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->co= mm_str' > >> truncated to a maximum of 'TASK_COMM_LEN' (16-bytes) to maintain AB= I, > >> - New / Modified users of 'get_task_comm'/ 'set_task_comm' will get > >> 'tsk->comm_str' supported for a maximum of 'TASK_COMM_EXTLEN' (64-by= tes). > >> > >> 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(-) > >> > > [...] >