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 ABE5CC64EC4 for ; Thu, 9 Feb 2023 00:54:20 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CCCE76B0071; Wed, 8 Feb 2023 19:54:19 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id C7CEE6B0072; Wed, 8 Feb 2023 19:54:19 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B44BC6B0074; Wed, 8 Feb 2023 19:54:19 -0500 (EST) 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 A558C6B0071 for ; Wed, 8 Feb 2023 19:54:19 -0500 (EST) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 3672F14072E for ; Thu, 9 Feb 2023 00:54:19 +0000 (UTC) X-FDA: 80445932238.04.B4CA624 Received: from mail-yb1-f174.google.com (mail-yb1-f174.google.com [209.85.219.174]) by imf14.hostedemail.com (Postfix) with ESMTP id 7A01A100002 for ; Thu, 9 Feb 2023 00:54:16 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=sBenSKQ7; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf14.hostedemail.com: domain of jstultz@google.com designates 209.85.219.174 as permitted sender) smtp.mailfrom=jstultz@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1675904056; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=HA3fnt3w/WkldBLqUixquvviOvTsBAQjIdHUUjb56M0=; b=yMr2Spc0rQGiYeYsZKVLWYZ5S63BMaAsB0D9Eeiu8qS2UrK+cPSVZDqhpzCTDdVP/yXzcj ANii0lRhrmpcLPOPospLocxUWrFcxMyVB8OeV4wvIn+QEKZpQQMjamFPmbhfrLBFvxFN05 uxF4sroVuuGMhZujqEy5mMW9ma9jRUc= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=sBenSKQ7; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf14.hostedemail.com: domain of jstultz@google.com designates 209.85.219.174 as permitted sender) smtp.mailfrom=jstultz@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1675904056; a=rsa-sha256; cv=none; b=MaMpmM0SgYlHiIsS8cxa+aVI+4tWDDNpc5czDteWK/qKLOled+fVuFeLdjQMHR3dJkcZA2 +EBhs6RflcEEUNnV4kYsPZCNvxqH+EopQleJph1xz/X8tnBR1S5WfWPSI6WiGHfiKg2qw/ 4rM4wVKdiRsPDWKYZsvYhrpjISHf+cE= Received: by mail-yb1-f174.google.com with SMTP id u7so547796ybi.6 for ; Wed, 08 Feb 2023 16:54:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=HA3fnt3w/WkldBLqUixquvviOvTsBAQjIdHUUjb56M0=; b=sBenSKQ7FVZf9KJQB47/1z3eFo5uPqCWNUETrOZxLzOJtcLUPNVIZaxX+GVCS8uFqv iwRXjPWW2bB7JalEf8dr+4OdWdJW799KnVv5wR3Re1phRxVIyLFcHw8DCeqrZtz8wpVy XIQufLeg9mKvB3udTuoMH/+0CfFZbJdy/ZyhssQ4fqr17mtOH/3uHYHZZM4TwzEBnk1c ETarg2Djh7AnXFAKIXF7pYXNcN+vvceHsCfOFtij66RhDNrLMs1FODaLt+CFqesZPH49 wQnMqvfxtvRKT7lRAJvvjBs1NxndyUcfpXc7Mg5lNSVf9hg2QrUDTpTa+8R2UV3OhcvX IYYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=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=HA3fnt3w/WkldBLqUixquvviOvTsBAQjIdHUUjb56M0=; b=xGgoQNzHkQPFNFS2nLN1T4dcU120CwE7fNsxvpTmfCrx4tpQ91wEFj7YylSAJjGz1Z p5b8ViP3MEXqS+z2zXvJ2Bd+qEeo9VacctNzOFU4ttZu0NcrfESKL/P6jJpYtT3/YiNE wazZtli/AobVYM5ms2R/SeVvAqgUw0/9UP8lIGCboHM4zyTI9r0kNfSniRsk3EerPiSu UW/oF6F+tP6dUSQF36Hyf/xmCf1OtEY0BcCB+htYgY+y/1VObIyxVyyZeFcJTiYnJYZB 3G4ZnCvTuaUFcAvdDi2KXa/xFf129kmq3scEpMar58GtUnteyb4COSSEzDON+uPCRZ61 fs1Q== X-Gm-Message-State: AO0yUKVIVef16zM7Og3YnbwyYh8k3z/iISLymHXgSfG0auspiHoych5f TNE4GyDpT/EDGNAeuF3dMg4R7oFfNeHRmuoLBTjG X-Google-Smtp-Source: AK7set9ZNH349+n5tqrkBZ3G/4EXSKW8hwOrbG7RXJtrVdxo+rv0LsXkOz6w3KeCd3ROkx+iQMrf5PG0n+BJqgs4m/M= X-Received: by 2002:a5b:b87:0:b0:8b6:6ae:3bbe with SMTP id l7-20020a5b0b87000000b008b606ae3bbemr1101327ybq.340.1675904055502; Wed, 08 Feb 2023 16:54:15 -0800 (PST) MIME-Version: 1.0 References: <20211120112738.45980-1-laoar.shao@gmail.com> <20211120112738.45980-8-laoar.shao@gmail.com> In-Reply-To: From: John Stultz Date: Wed, 8 Feb 2023 16:54:03 -0800 Message-ID: Subject: Re: [PATCH v2 7/7] tools/testing/selftests/bpf: replace open-coded 16 with TASK_COMM_LEN To: Alexei Starovoitov Cc: Yafang Shao , Andrew Morton , Network Development , bpf , "linux-perf-use." , Linux-Fsdevel , linux-mm , LKML , kernel test robot , kbuild test robot , Andrii Nakryiko , David Hildenbrand , Mathieu Desnoyers , Arnaldo Carvalho de Melo , Andrii Nakryiko , Michal Miroslaw , Peter Zijlstra , Steven Rostedt , Matthew Wilcox , Al Viro , Kees Cook , Petr Mladek , Kajetan Puchalski , Lukasz Luba , Qais Yousef , Daniele Di Proietto Content-Type: text/plain; charset="UTF-8" X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 7A01A100002 X-Stat-Signature: s7zcdwz9p77medddjt41dfa9kp5ujtjy X-HE-Tag: 1675904056-121558 X-HE-Meta: U2FsdGVkX1/wUUINIGh3ZJgISVtGEcIs5r/kf6SH0+kY4r5rbLj4w7CAeIQVEvdS87GdYxGwsKOiJpsB9zUgscaKZz4+JvLlTIgnO2T7ywc6Qr980QkpFwFBcwIYzLY2bUBy40S4+DnVZSMEV7T4tpXkX4PP4/vK0002O7jWjVr7H8e3397WyFFRY/MVEb1FG5YzxZ/UKoFVL/Lg3aXLQEZDgy6AN3bkRuOoBSIGRTIo4NoorgefxxZkA8GMrkzYG3K1RVNev9KLOTYLhMXbVLXTV0+4m+H/rmaRCFUBLMABZ5WHgnBaHB+lYCEkqpTB7nYORKSTYh9/rLhja+yWPGuq2D9sAG3mVR7Wh8aAhoQlkaHSyqvTUYaj4LuqAjgipn3itlmjdtp3fF5Orm/toKAAxz+uz4bXIMXk1FkYa+DtoWBa0L1y1rzEN8H7O3YL22ukDcEm0Sj6gp04sqPy56O32fd6Zs1+xhkYPIoRa45wvaH17Tqob7AF7mNyopiXDkHF4IjWWueETVzbnB8ulsv2CFO5tcNTUKwf4zCL6QdXCRnq5CjfA6F1HnFtS9bbKO3Nh+xrco5xIx2a7sDycrWVinbfGYR3u7/96JgplOlVMzX18wahZ5XoDxok8CchX9wmvtBg/Nlmw4ZDii5KhmGsxDbSoWkR2Ew2DBp8D5IcZcfNmePH+euP1aCJwLUrK81/e72Sb2IgTyjWlvezFymBS+UAeAryILNieMyy+c/aB67ZhlI7MN/HQxRhfQZiWEW/4h096zHaE8P6OTGANgO1qnZ6ufl5qM9v2A5bSecYErnVcA8KaW3b45lnx5Vz/ckBEZ5eDxGuw2ntkZmxrIJEK98nuiR8gR714qEROTFbquT7KOzrQSgDXQf8UmcbcCjxGUphCImbqaO0h6jc0SBIhPKu1nLB6rl1xyh2We3h4hb9D9Dm8/KwnFVCvXUqBerB+7MkDHEZjb6KRsU HkdCdKq9 unKyjeaXx6rXUP1o52WQA/5rKbrNlQJrkbMRotI+pHoTIQA8XVP6WNHGI/cjny92tf5zVXtdHsQdiCkFhoJVQYI6KGMYOLw1E9bBw3kbO5490mh9ehLg4M3TINU5veO7Ce23WH6SwJt8hrjAMQJM1Fy7vyiG4RMkTmQ0bbU+dKjmWKQP/5j6z5fjkN/fjBUilaE2GcF68p7Rovd4dm36t96YswqIfAt/axb+6ErTsr9UV3oODXWrTXhYW1lBnOeMgt4godvVUN1b33F9B2W4oOulpgKULZJm/vgMatEYay6QMdwY71NMIFycgC1/WSPLoWCO+YPEJWUk6kitDX1SjYaIyX2m86GnaxHLzuAblOMN++U1HJOuTgi5XGJ09mDx7L4XerSQI7R//dsDV/fp3ckd8UoumUlFAPqvxOJjAAsi2SOLBAY6O1beZDDTtPe+ksPMPgH+kb1oqArFvne93iHURqMIYjXirHWXnvFmKe8GoYAVVmBcLLXyJ5O9nFAYcHSGgb38mbWKRanTOdozZElRFC1F+fFprknlSp7opXAvrt5UahRGuWv8u8fo6DZRwRJvUxsgU1n6C3o0uyhquBrLlJazkMAbw/MLpN2SlEBqbdBRe/ijurNHHi96sOAEEfC9gMceajEw+lN3gbDeloAGfwWmtTCE+6Np8XnzTkC1NvYPZriaAG/PHoWJHtJWdEhszuGu+m+4T5KhLAreb9HI1zcGQaukA8cldPF+nRvU+bOTbMWYk3WQJZrnQ0hVXWWZLggUeICbaMdllb9Vkw6XgDYUx3YPuVk9qnE+g2bm6982364qjjLLDk7YHCdYErohed7QsRcU8M0R2oAdlTCREu092P0bg0Kl6REg0LhuC+E5akJ/vFYcMrVs98vN0Jye0Tx6hyELCnSD72btQn9icZg== 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: On Wed, Feb 8, 2023 at 4:11 PM Alexei Starovoitov wrote: > > On Wed, Feb 8, 2023 at 2:01 PM John Stultz wrote: > > > > On Sat, Nov 20, 2021 at 11:27:38AM +0000, Yafang Shao wrote: > > > As the sched:sched_switch tracepoint args are derived from the kernel, > > > we'd better make it same with the kernel. So the macro TASK_COMM_LEN is > > > converted to type enum, then all the BPF programs can get it through BTF. > > > > > > The BPF program which wants to use TASK_COMM_LEN should include the header > > > vmlinux.h. Regarding the test_stacktrace_map and test_tracepoint, as the > > > type defined in linux/bpf.h are also defined in vmlinux.h, so we don't > > > need to include linux/bpf.h again. > > > > > > Signed-off-by: Yafang Shao > > > Acked-by: Andrii Nakryiko > > > Acked-by: David Hildenbrand > > > Cc: Mathieu Desnoyers > > > Cc: Arnaldo Carvalho de Melo > > > Cc: Andrii Nakryiko > > > Cc: Michal Miroslaw > > > Cc: Peter Zijlstra > > > Cc: Steven Rostedt > > > Cc: Matthew Wilcox > > > Cc: David Hildenbrand > > > Cc: Al Viro > > > Cc: Kees Cook > > > Cc: Petr Mladek > > > --- > > > include/linux/sched.h | 9 +++++++-- > > > tools/testing/selftests/bpf/progs/test_stacktrace_map.c | 6 +++--- > > > tools/testing/selftests/bpf/progs/test_tracepoint.c | 6 +++--- > > > 3 files changed, 13 insertions(+), 8 deletions(-) > > > > Hey all, > > I know this is a little late, but I recently got a report that > > this change was causiing older versions of perfetto to stop > > working. > > > > Apparently newer versions of perfetto has worked around this > > via the following changes: > > https://android.googlesource.com/platform/external/perfetto/+/c717c93131b1b6e3705a11092a70ac47c78b731d%5E%21/ > > https://android.googlesource.com/platform/external/perfetto/+/160a504ad5c91a227e55f84d3e5d3fe22af7c2bb%5E%21/ > > > > But for older versions of perfetto, reverting upstream commit > > 3087c61ed2c4 ("tools/testing/selftests/bpf: replace open-coded 16 > > with TASK_COMM_LEN") is necessary to get it back to working. > > > > I haven't dug very far into the details, and obviously this doesn't > > break with the updated perfetto, but from a high level this does > > seem to be a breaking-userland regression. > > > > So I wanted to reach out to see if there was more context for this > > breakage? I don't want to raise a unnecessary stink if this was > > an unfortuante but forced situation. > > Let me understand what you're saying... > > The commit 3087c61ed2c4 did > > -/* Task command name length: */ > -#define TASK_COMM_LEN 16 > +/* > + * Define the task command name length as enum, then it can be visible to > + * BPF programs. > + */ > +enum { > + TASK_COMM_LEN = 16, > +}; > > > and that caused: > > cat /sys/kernel/debug/tracing/events/task/task_newtask/format > > to print > field:char comm[TASK_COMM_LEN]; offset:12; size:16; signed:0; > instead of > field:char comm[16]; offset:12; size:16; signed:0; > > so the ftrace parsing android tracing tool had to do: > > - if (Match(type_and_name.c_str(), R"(char [a-zA-Z_]+\[[0-9]+\])")) { > + if (Match(type_and_name.c_str(), > + R"(char [a-zA-Z_][a-zA-Z_0-9]*\[[a-zA-Z_0-9]+\])")) { > > to workaround this change. > Right? I believe so. > And what are you proposing? I'm not proposing anything. I was just wanting to understand more context around this, as it outwardly appears to be a user-breaking change, and that is usually not done, so I figured it was an issue worth raising. If the debug/tracing/*/format output is in the murky not-really-abi space, that's fine, but I wanted to know if this was understood as something that may require userland updates or if this was a unexpected side-effect. thanks -john