From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4A4DE3890FB for ; Wed, 24 Jun 2026 06:47:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782283669; cv=none; b=TtRd2ItjlfBdK+UtUuF1cpOZMFcZRTtNBy1/cr40smRadD7ZyyizTN+nGtvcCCCRXK8fV79EKa+ttWN3s4IVI2asSPefCBt/ckkGRENwRiKt2ILrtmuEfaXAw2kZxc/eC8liE6IM5K1hGE1kT8tsQkqi1CMtfg5aNoSbxCR2KhE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782283669; c=relaxed/simple; bh=9KOIoHENInRIyFr1V0E2ePyNkctr48xJ/8SrVJv7he8=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=oh6j+f7kK7bQ8IrlimXTCgUdtSHTu0R2jHN6TRjYUuqWQsotZ+nWDu3Ro0YE4uRbKQNUSD0o0tru23CTgiZAeylWOIdTQ/e1JA/6NICHbpyXtOUNFPd0mIsTegqvsi/X5/DfcuF5C3hLXa/ev4F3s7deyW4uVn90GGbyEX+CZxA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=NosqBzWt; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=Jp3RmmY5; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="NosqBzWt"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="Jp3RmmY5" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1782283665; h=from:from: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; bh=0igFXozW1XpGYfz7uHbcb09PTKxi8TUnE588/OdtT/U=; b=NosqBzWtKtghP9p+K9Jp7jCpdmzuIdODOxX8uoS9epXBjSGvMxGtKW6fL8i+O7qC4RbBMz rej77Iu5alStA6LHYENYjBqKujVZ9ELOcEN4qSIGLEXXONih3wXtu7LGAlZxjrFbYCQtag /W51ElfSW8VtJIenc+BRrj5LLTgg7Vs= Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-13-rsFgFh8pO8aOzmLI2aI69A-1; Wed, 24 Jun 2026 02:47:43 -0400 X-MC-Unique: rsFgFh8pO8aOzmLI2aI69A-1 X-Mimecast-MFC-AGG-ID: rsFgFh8pO8aOzmLI2aI69A_1782283662 Received: by mail-wr1-f72.google.com with SMTP id ffacd0b85a97d-45f3d008865so471570f8f.0 for ; Tue, 23 Jun 2026 23:47:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1782283662; x=1782888462; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:content-language:from :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=0igFXozW1XpGYfz7uHbcb09PTKxi8TUnE588/OdtT/U=; b=Jp3RmmY5aPGA/bSGns3n7h0yBBcn580t+b2P/Tw14WY9G1WdJAyOcZasK3h/bQcuys d/j2uiOyDIVU3gqBCuxMeEIcaSF40fjCkri1Ufoa0CEBYzUfYMNLq2+U4jhtsMfwx94c /o+EpuiOvpZWviUzHO5kZRaM1501Gl7kG8Nk3KtisELBZPEYkrHhIwtDD73yj+0n8GLF MhPyZnklcTtdJGBx4dQFP50PJ4z1bDeEmxfFFcDjcIA6rMItYU3wq+YjA76YZVVvlgB8 9UVtYdq1mPbdYWsILOs/8Hq7ux9YjF6Nw/iZLy1o3QkQ71l45RXBswrRF3VVfFO8xQLg wvwg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1782283662; x=1782888462; h=content-transfer-encoding:in-reply-to:content-language:from :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=0igFXozW1XpGYfz7uHbcb09PTKxi8TUnE588/OdtT/U=; b=iVRdnBa+QuNFIdxHLFGHds5QDQzcmbSduDh2k7sd/uo8+vfoi/1d8AF4F0EGahGuno B8lqjzSmEu98rPiLqqXHjU1tnJ+YrYvtz9QR/yy9RqsI2UabmGE0YlTYR/quEmqrPIuK K3Tp5/gYJhEZsj8OqyPTIqe5J8WthZFguhFcboHULEL99Dh12iojiTLhAll0cZjBVWhr KtXWR3SIOitjq2qFL6uLzC9CIAcbu41Qin36EYc+kce+jSwzGwY0I24xofs+I2KBk3sw C+nETo6qChzOpQmbhvCTncCHm/yruQJeBw9SYpVGHTURsBLHn760rGuh7NZaDN8/f8rV V+Ng== X-Gm-Message-State: AOJu0YxKPBKdM75YF9IGBY4dFifrZI1YguP/h4oKulRjM1CQ9r1v9JCu Y5Wo/S5hUkGmeQSoPCvZJvWtUvET77xtxwROik3ltPxEBUsb6U0AzqbU7dcigaEdOGfnn7BdOPq sOqjEqAe8v0iSec9844sKLH+LbIXKxIdvdWt+qTANENgLhRYfJ9psP0uRpe/QHPO8TOOb5Q== X-Gm-Gg: AfdE7clyH8hCAf+Bkf/1UxbikF8c77vHdpjZC8205ulzXTbQ6xMSDk7WTi1pqUlAPxY zzOVxMqAhN3ZJfDrkSl0pr+jyuL6m/+crKsRgvNiSM5TnWrIqdUNWgXo0n/i5l+ykTv3tVd1jnG 5hGL6NbV+gODKVVGRQgu8i2I9nFCkA8Mk4qK9n4CjApyKnbbe3AdKwV+XlTwChwCrcB6B4QHhtR h0ccixgDky6XuhtSSoGs+fInpwI/1xEm4qWc+BdwJU71wudmqWSg2Nx9OVBMjQvt+pCg1KzVljo xqEVaNo7aqNms3JfcRjNvGfJK82+fJyl1293sIto3sVfWxiMUuY4tABjVSNXqZOvTnIv5f8krPS 56jr+gJuq4foMzRse1VWjyA+gCIHmtyYGnFKKMebeO6B1xQ== X-Received: by 2002:a05:6000:2683:b0:465:4305:6460 with SMTP id ffacd0b85a97d-46a7f66caefmr10121781f8f.9.1782283661699; Tue, 23 Jun 2026 23:47:41 -0700 (PDT) X-Received: by 2002:a05:6000:2683:b0:465:4305:6460 with SMTP id ffacd0b85a97d-46a7f66caefmr10121727f8f.9.1782283661201; Tue, 23 Jun 2026 23:47:41 -0700 (PDT) Received: from [192.168.0.135] (185-219-167-205-static.vivo.cz. [185.219.167.205]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-46c1ee01c6csm4270068f8f.14.2026.06.23.23.47.39 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 23 Jun 2026 23:47:40 -0700 (PDT) Message-ID: Date: Wed, 24 Jun 2026 08:47:38 +0200 Precedence: bulk X-Mailing-List: linux-perf-users@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] perf trace: Refactor augmented_raw_syscalls using bpf_loop To: Namhyung Kim , Alexei Starovoitov Cc: linux-perf-users@vger.kernel.org, Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Alexander Shishkin , Jiri Olsa , Ian Rogers , Adrian Hunter , James Clark , Howard Chu , linux-kernel@vger.kernel.org, bpf@vger.kernel.org, Michael Petlan , stable@vger.kernel.org References: <20260623112533.1151502-1-vmalik@redhat.com> From: Viktor Malik Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 6/23/26 19:10, Namhyung Kim wrote: > Hello, > > On Tue, Jun 23, 2026 at 08:27:39AM -0700, Alexei Starovoitov wrote: >> On Tue Jun 23, 2026 at 4:25 AM PDT, Viktor Malik wrote: >>> The loop for processing syscall args in augment_raw_syscalls has a >>> history of breaking with Clang updates, see e.g. commit 013eb043f37b >>> ("perf trace: Fix BPF loading failure (-E2BIG)") from Clang 15 to 16. >>> >>> Now, a similar thing happened between Clang 21 and 22. While the issue >>> is mitigated on the main line by a recent verifier update, it remains >>> broken on the 6.12 and 6.18 stable branches: >>> >>> [linux-6.18.y]# sudo perf trace true >>> libbpf: prog 'sys_enter': BPF program load failed: -E2BIG >>> libbpf: prog 'sys_enter': -- BEGIN PROG LOAD LOG -- >>> [...] >>> BPF program is too large. Processed 1000001 insn >>> processed 1000001 insns (limit 1000000) max_states_per_insn 40 total_states 37941 peak_states 232 mark_read 0 >>> -- END PROG LOAD LOG -- >>> libbpf: prog 'sys_enter': failed to load: -E2BIG >>> libbpf: failed to load object 'augmented_raw_syscalls_bpf' >>> libbpf: failed to load BPF skeleton 'augmented_raw_syscalls_bpf': -E2BIG >>> Error: failed to get syscall or beauty map fd >>> [...] >>> >>> The reason is that the loop is quite complex and the BPF verifier often >>> struggles to prove that it terminates. >>> >>> Fix the issue by refactoring the loop body into a callback function and >>> calling the bpf_loop helper. This should prevent future breakages of >>> this kind since the callback function has no loops. It also allows to >>> drop a few artificial checks to help the verifier, including the changes >>> introduced by 013eb043f37b. > > Thanks for working on this. I encountered this issue before and never > found time to take a deeper look yet. > >>> >>> Signed-off-by: Viktor Malik >>> Fixes: a68fd6a6cdd3 ("perf trace: Collect augmented data using BPF") >>> Fixes: 013eb043f37b ("perf trace: Fix BPF loading failure (-E2BIG)") >>> Cc: stable@vger.kernel.org >>> --- >>> .../bpf_skel/augmented_raw_syscalls.bpf.c | 157 +++++++++++------- >>> 1 file changed, 96 insertions(+), 61 deletions(-) >>> >>> diff --git a/tools/perf/util/bpf_skel/augmented_raw_syscalls.bpf.c b/tools/perf/util/bpf_skel/augmented_raw_syscalls.bpf.c >>> index 2a6e61864ee0..6d553ed3ac23 100644 >>> --- a/tools/perf/util/bpf_skel/augmented_raw_syscalls.bpf.c >>> +++ b/tools/perf/util/bpf_skel/augmented_raw_syscalls.bpf.c >>> @@ -429,15 +429,96 @@ static bool pid_filter__has(struct pids_filtered *pids, pid_t pid) >>> return bpf_map_lookup_elem(pids, &pid) != NULL; >>> } >>> >>> +struct args_loop_ctx { >>> + struct syscall_enter_args *args; >>> + unsigned int *beauty_map; >>> + void *payload_offset; >>> + int value_size; >>> + u64 *output; >>> + bool *do_output; >>> +}; >>> + >>> +static long process_arg_cb(u64 i, void *ctx) >>> +{ >>> + /* >>> + * Determine what type of argument and how many bytes to read from user space, using the >>> + * value in the beauty_map. This is the relation of parameter type and its corresponding >>> + * value in the beauty map, and how many bytes we read eventually: >>> + * >>> + * string: 1 -> size of string >>> + * struct: size of struct -> size of struct >>> + * buffer: -1 * (index of paired len) -> value of paired len (maximum: TRACE_AUG_MAX_BUF) >>> + */ >>> + struct augmented_arg *augmented_arg; >>> + struct args_loop_ctx *loop_ctx; >>> + int aug_size, size, index; >>> + bool augmented; >>> + void *arg; >>> + >>> + /* Bounds check for the below map access to help the verifier */ >>> + if (i < 0 || i >= 6) >>> + return 1; >>> + >>> + loop_ctx = (struct args_loop_ctx *)ctx; >>> + arg = (void *)loop_ctx->args->args[i]; >>> + augmented = false; >>> + size = loop_ctx->beauty_map[i]; >>> + aug_size = size; /* size of the augmented data read from user space */ >>> + augmented_arg = (struct augmented_arg *)loop_ctx->payload_offset; >>> + >>> + if (size == 0 || arg == NULL) >>> + return 0; /* continue */ >>> + >>> + if (size == 1) { /* string */ >>> + aug_size = bpf_probe_read_user_str(augmented_arg->value, loop_ctx->value_size, arg); >>> + augmented = true; >>> + } else if (size > 0 && size <= loop_ctx->value_size) { /* struct */ >>> + if (!bpf_probe_read_user(augmented_arg->value, size, arg)) >>> + augmented = true; >>> + } else if (size < 0 && size >= -6) { /* buffer */ >>> + index = -(size + 1); >>> + barrier_var(index); // Prevent clang (noticed with v18) from removing the &= 7 trick. >>> + index &= 7; // Satisfy the bounds checking with the verifier in some kernels. >>> + aug_size = loop_ctx->args->args[index]; >>> + >>> + if (aug_size > TRACE_AUG_MAX_BUF) >>> + aug_size = TRACE_AUG_MAX_BUF; >>> + >>> + if (aug_size > 0) { >>> + if (!bpf_probe_read_user(augmented_arg->value, aug_size, arg)) >>> + augmented = true; >>> + } >>> + } >>> + >>> + /* Augmented data size is limited to sizeof(augmented_arg->unnamed union with value field) */ >>> + if (aug_size > loop_ctx->value_size) >>> + aug_size = loop_ctx->value_size; >>> + >>> + /* write data to payload */ >>> + if (augmented) { >>> + int written = offsetof(struct augmented_arg, value) + aug_size; >>> + >>> + if (written < 0 || written > sizeof(struct augmented_arg)) >>> + return 1; /* break */ >>> + >>> + augmented_arg->size = aug_size; >>> + *loop_ctx->output += written; >>> + loop_ctx->payload_offset += written; >>> + *loop_ctx->do_output = true; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> static int augment_sys_enter(void *ctx, struct syscall_enter_args *args) >>> { >>> - bool augmented, do_output = false; >>> - int zero = 0, index, value_size = sizeof(struct augmented_arg) - offsetof(struct augmented_arg, value); >>> + bool do_output = false; >>> + int zero = 0, value_size = sizeof(struct augmented_arg) - offsetof(struct augmented_arg, value); >>> u64 output = 0; /* has to be u64, otherwise it won't pass the verifier */ >>> - s64 aug_size, size; >>> unsigned int nr, *beauty_map; >>> struct beauty_payload_enter *payload; >>> - void *arg, *payload_offset; >>> + void *payload_offset; >>> + long iters; >>> >>> /* fall back to do predefined tail call */ >>> if (args == NULL) >>> @@ -457,63 +538,17 @@ static int augment_sys_enter(void *ctx, struct syscall_enter_args *args) >>> /* copy the sys_enter header, which has the syscall_nr */ >>> __builtin_memcpy(&payload->args, args, sizeof(struct syscall_enter_args)); >>> >>> - /* >>> - * Determine what type of argument and how many bytes to read from user space, using the >>> - * value in the beauty_map. This is the relation of parameter type and its corresponding >>> - * value in the beauty map, and how many bytes we read eventually: >>> - * >>> - * string: 1 -> size of string >>> - * struct: size of struct -> size of struct >>> - * buffer: -1 * (index of paired len) -> value of paired len (maximum: TRACE_AUG_MAX_BUF) >>> - */ >>> - for (int i = 0; i < 6; i++) { >>> - arg = (void *)args->args[i]; >>> - augmented = false; >>> - size = beauty_map[i]; >>> - aug_size = size; /* size of the augmented data read from user space */ >>> - >>> - if (size == 0 || arg == NULL) >>> - continue; >>> - >>> - if (size == 1) { /* string */ >>> - aug_size = bpf_probe_read_user_str(((struct augmented_arg *)payload_offset)->value, value_size, arg); >>> - /* minimum of 0 to pass the verifier */ >>> - if (aug_size < 0) >>> - aug_size = 0; >>> - >>> - augmented = true; >>> - } else if (size > 0 && size <= value_size) { /* struct */ >>> - if (!bpf_probe_read_user(((struct augmented_arg *)payload_offset)->value, size, arg)) >>> - augmented = true; >>> - } else if ((int)size < 0 && size >= -6) { /* buffer */ >>> - index = -(size + 1); >>> - barrier_var(index); // Prevent clang (noticed with v18) from removing the &= 7 trick. >>> - index &= 7; // Satisfy the bounds checking with the verifier in some kernels. >>> - aug_size = args->args[index] > TRACE_AUG_MAX_BUF ? TRACE_AUG_MAX_BUF : args->args[index]; >>> - >>> - if (aug_size > 0) { >>> - if (!bpf_probe_read_user(((struct augmented_arg *)payload_offset)->value, aug_size, arg)) >>> - augmented = true; >>> - } >>> - } >>> - >>> - /* Augmented data size is limited to sizeof(augmented_arg->unnamed union with value field) */ >>> - if (aug_size > value_size) >>> - aug_size = value_size; >>> - >>> - /* write data to payload */ >>> - if (augmented) { >>> - int written = offsetof(struct augmented_arg, value) + aug_size; >>> - >>> - if (written < 0 || written > sizeof(struct augmented_arg)) >>> - return 1; >>> - >>> - ((struct augmented_arg *)payload_offset)->size = aug_size; >>> - output += written; >>> - payload_offset += written; >>> - do_output = true; >>> - } >>> - } >>> + struct args_loop_ctx loop_ctx = { >>> + .args = args, >>> + .beauty_map = beauty_map, >>> + .payload_offset = payload_offset, >>> + .value_size = value_size, >>> + .output = &output, >>> + .do_output = &do_output >>> + }; >>> + iters = bpf_loop(6, process_arg_cb, &loop_ctx, 0); >> >> bpf_loop() is old and generally not recommended. >> Please use bpf_for() then the diff will be one line change and >> can scale to any number of args. Not just 6. Thanks Alexei, I didn't know about this preference. > One thing we should take care is to support old kernels. The oldest > LTS kernel in the kernel.org is 5.10 and bpf_loop() was introduced in > 5.17 and bpf_for (bpf_iter_num) was 6.4. The problematic loop was introduced in 6.12 by a68fd6a6cdd3 ("perf trace: Collect augmented data using BPF") so we should be good using bpf_for. Or is perf from 7.2 supposed to work on 5.10 LTS kernels? I'll refactor with bpf_for and will send v2. It should be then backported to stable kernels down to 6.12 LTS. Viktor > > Maybe we can factor out the loop body and call it from different > mechanisms like open-coded loop, bpf_loop or bpf_for depending on the > kernel version. But not sure it'd fix the verifier issue though. > > Thanks, > Namhyung >