From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ot1-f50.google.com (mail-ot1-f50.google.com [209.85.210.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BFF113D8901 for ; Tue, 23 Jun 2026 15:27:42 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.50 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782228465; cv=none; b=umJKfrPMWw2dFq+BYXyw4MM5G5JGVi1YVlFiJyQAL45dhSNpHhOim1p1z+08JfAxNSLC9LCbmugKmF6T/zqrMtPuOURU94mIHl8GBQkswbyOcioq6H2ZVENG7AD0bTIO+oWCwi7jft3vcWv5eC9+jdPIR65+ceeYFjWIL9TY1jU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782228465; c=relaxed/simple; bh=VGISIkuM6cBCm44W8mdczkbu3jpiPhtikc9p/vXJ+kA=; h=Mime-Version:Content-Type:Date:Message-Id:To:Cc:Subject:From: References:In-Reply-To; b=DE4+5sz/mfsnfyao9UW6GhFnz1VzjUhkeSw/hOeCuJSqdI+Qsa28d4LD1chAaL/rk2ms7jC7pq7prTL+w+sh54mbw7CucZKf5LjNUex5Jos+dEQ9jxMi4too8Mv9VgPrAmYGdPuKgISvJXdSDFRGgEPh7pRzNCzgYFDEFxKE8UQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=Drip6E7C; arc=none smtp.client-ip=209.85.210.50 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Drip6E7C" Received: by mail-ot1-f50.google.com with SMTP id 46e09a7af769-7e6b5c374e5so1113241a34.0 for ; Tue, 23 Jun 2026 08:27:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1782228462; x=1782833262; darn=vger.kernel.org; h=in-reply-to:references:from:subject:cc:to:message-id:date :content-transfer-encoding:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=8OAvyMh4VrWL/Mmr/l/ESjGHF6/uUimT4xTLWLq23Fo=; b=Drip6E7CBTxqSlCUZzu8Y3P8qtvMmWNHCC6zAgK7JRlOq0zbsY27fvkRtxWa4wqoh6 VnsHa5rX77gxD05KaRrze1yia2hZQZ26GonPeu8ALyDdStqVS+bE3pEc1obFbV81CKBk +5FVWARQwJk4zYD4xMqSqJdNDiFD4lVS8Fzh89Eel562AbG5oDg1aEW2+8GytRbTh3mj RDbYBlSY1uGp5U+iEcBuNQ4DsPB/PINSy+0N6PAAbDMHb8k8WncVliwSgsYovTAJ0uRr JErCDw3qZIBkek92EqRvV0BWuYspnsSi9x8hN1+duW0dajRaJtjBBEzf4euyliCpHmBB v8Qw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1782228462; x=1782833262; h=in-reply-to:references:from:subject:cc:to:message-id:date :content-transfer-encoding:mime-version:x-gm-gg:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=8OAvyMh4VrWL/Mmr/l/ESjGHF6/uUimT4xTLWLq23Fo=; b=p5qegJN53CLr+Cd6wvMusovXhLaqwe//JZin+D1paPdw5CZIYTOHiAAEb1uB99SBNH 2hF6pO7nGYvC1VOrSRg7hgpylO2o1rJZmGIulxJYHr0y61kVP3EEvwK04b3WRd/g+EZz JNpDcmDgX/vV7W5yKEXgus2oSPSD2e9Fx45eykLvQE5PM9QUwC3M9UsQ9bRX/4zG9c82 LKrY3tDCE1cYW7sc5WbRItcWW3Bd3QB7ZI6EnIHw4kYwRAXBvPIpbI2yhgwOWvs1b0cI TA+yUClz4oJJuzNtW5VZfiHEBIaEOm9othQ7n+RFzvSu2qbV5gtLF+Aiq3nC8RwE6Wku eXJg== X-Forwarded-Encrypted: i=1; AFNElJ8/fekSsNJE1h9REEMu7JaYBKnnjnYad6libTyIShdJnF7HOQZJ5PmTajGc/taqK4jvizf+NF95YcR6iSY=@vger.kernel.org X-Gm-Message-State: AOJu0YzOZyYpby8nioq79lSsrDp/wb4RcD6l5yE3FwZiwo2SCI2+pH4u nYaik/i2nYm/Iv8FV5+jOP3GussJ0iqBZp1goMZpPXC/z8SbFlSGXqON X-Gm-Gg: AfdE7cm97grL4kvF7Rf4nlyZ6c/T+wD40KdCE6ma5o3Q2yOn1v/sNuXzif54Jum6t/X CAQf3TxyOp6et7q7hQmKXF4cC4NyMnmtu5vr1sMFPBLKB/DYBuQbLEphdKEeGcruKjWfq1dqSd6 WWvgRPqF8K/YoCLb/n+rN0UMCc/jS5ew3IzczuC7EIDotXoW6naXqrFvlu+3AcvS7p/WVr06jII 8IJNgv7ANsay6uutDlZjzrUsr3knVo169Nk5DXQw2+wrrx41yD8EpXK/xO7Z5BMrHlfzfs2ZNgs NNeOw/30ZICI5r3mEnAV1WqKPuKf5hcPv9BIVgLphzBzXRKWHXbrN2rNwAXW1aAnfT5r2IXxTzm x/WWxFPy8NmWLIsMdn3poV4OngGuPxiZQl5raYVkbH3pk7EEnu+v39GqMxM35vOtDC2r4JNTqEb PVPb18dWb+JwPG+Qu0Ol8GkzJE4DgaQ9/NKYMD9vyTUJcFxA4N7b9qdfS5+rIoFRZvse6QN0+GX vyz/Q== X-Received: by 2002:a05:6830:65cf:10b0:7d7:dead:e388 with SMTP id 46e09a7af769-7e9743556e5mr1877454a34.15.1782228461598; Tue, 23 Jun 2026 08:27:41 -0700 (PDT) Received: from localhost ([2a03:2880:10ff:c::]) by smtp.gmail.com with ESMTPSA id 46e09a7af769-7e9442e98c4sm8823777a34.26.2026.06.23.08.27.39 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 23 Jun 2026 08:27:40 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Tue, 23 Jun 2026 08:27:39 -0700 Message-Id: To: "Viktor Malik" , Cc: "Peter Zijlstra" , "Ingo Molnar" , "Arnaldo Carvalho de Melo" , "Namhyung Kim" , "Mark Rutland" , "Alexander Shishkin" , "Jiri Olsa" , "Ian Rogers" , "Adrian Hunter" , "James Clark" , "Howard Chu" , , , "Michael Petlan" , Subject: Re: [PATCH] perf trace: Refactor augmented_raw_syscalls using bpf_loop From: "Alexei Starovoitov" X-Mailer: aerc References: <20260623112533.1151502-1-vmalik@redhat.com> In-Reply-To: <20260623112533.1151502-1-vmalik@redhat.com> 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': -E2= BIG > 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. > > 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/tool= s/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 *p= ids, pid_t pid) > return bpf_map_lookup_elem(pids, &pid) !=3D NULL; > } > =20 > +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 >=3D 6) > + return 1; > + > + loop_ctx =3D (struct args_loop_ctx *)ctx; > + arg =3D (void *)loop_ctx->args->args[i]; > + augmented =3D false; > + size =3D loop_ctx->beauty_map[i]; > + aug_size =3D size; /* size of the augmented data read from user space *= / > + augmented_arg =3D (struct augmented_arg *)loop_ctx->payload_offset; > + > + if (size =3D=3D 0 || arg =3D=3D NULL) > + return 0; /* continue */ > + > + if (size =3D=3D 1) { /* string */ > + aug_size =3D bpf_probe_read_user_str(augmented_arg->value, loop_ctx->v= alue_size, arg); > + augmented =3D true; > + } else if (size > 0 && size <=3D loop_ctx->value_size) { /* struct */ > + if (!bpf_probe_read_user(augmented_arg->value, size, arg)) > + augmented =3D true; > + } else if (size < 0 && size >=3D -6) { /* buffer */ > + index =3D -(size + 1); > + barrier_var(index); // Prevent clang (noticed with v18) from removing = the &=3D 7 trick. > + index &=3D 7; // Satisfy the bounds checking with the verifier in = some kernels. > + aug_size =3D loop_ctx->args->args[index]; > + > + if (aug_size > TRACE_AUG_MAX_BUF) > + aug_size =3D TRACE_AUG_MAX_BUF; > + > + if (aug_size > 0) { > + if (!bpf_probe_read_user(augmented_arg->value, aug_size, arg)) > + augmented =3D true; > + } > + } > + > + /* Augmented data size is limited to sizeof(augmented_arg->unnamed unio= n with value field) */ > + if (aug_size > loop_ctx->value_size) > + aug_size =3D loop_ctx->value_size; > + > + /* write data to payload */ > + if (augmented) { > + int written =3D offsetof(struct augmented_arg, value) + aug_size; > + > + if (written < 0 || written > sizeof(struct augmented_arg)) > + return 1; /* break */ > + > + augmented_arg->size =3D aug_size; > + *loop_ctx->output +=3D written; > + loop_ctx->payload_offset +=3D written; > + *loop_ctx->do_output =3D true; > + } > + > + return 0; > +} > + > static int augment_sys_enter(void *ctx, struct syscall_enter_args *args) > { > - bool augmented, do_output =3D false; > - int zero =3D 0, index, value_size =3D sizeof(struct augmented_arg) - of= fsetof(struct augmented_arg, value); > + bool do_output =3D false; > + int zero =3D 0, value_size =3D sizeof(struct augmented_arg) - offsetof(= struct augmented_arg, value); > u64 output =3D 0; /* has to be u64, otherwise it won't pass the verifie= r */ > - s64 aug_size, size; > unsigned int nr, *beauty_map; > struct beauty_payload_enter *payload; > - void *arg, *payload_offset; > + void *payload_offset; > + long iters; > =20 > /* fall back to do predefined tail call */ > if (args =3D=3D NULL) > @@ -457,63 +538,17 @@ static int augment_sys_enter(void *ctx, struct sysc= all_enter_args *args) > /* copy the sys_enter header, which has the syscall_nr */ > __builtin_memcpy(&payload->args, args, sizeof(struct syscall_enter_args= )); > =20 > - /* > - * 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 =3D 0; i < 6; i++) { > - arg =3D (void *)args->args[i]; > - augmented =3D false; > - size =3D beauty_map[i]; > - aug_size =3D size; /* size of the augmented data read from user space = */ > - > - if (size =3D=3D 0 || arg =3D=3D NULL) > - continue; > - > - if (size =3D=3D 1) { /* string */ > - aug_size =3D 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 =3D 0; > - > - augmented =3D true; > - } else if (size > 0 && size <=3D value_size) { /* struct */ > - if (!bpf_probe_read_user(((struct augmented_arg *)payload_offset)->va= lue, size, arg)) > - augmented =3D true; > - } else if ((int)size < 0 && size >=3D -6) { /* buffer */ > - index =3D -(size + 1); > - barrier_var(index); // Prevent clang (noticed with v18) from removing= the &=3D 7 trick. > - index &=3D 7; // Satisfy the bounds checking with the verifier in= some kernels. > - aug_size =3D args->args[index] > TRACE_AUG_MAX_BUF ? TRACE_AUG_MAX_BU= F : args->args[index]; > - > - if (aug_size > 0) { > - if (!bpf_probe_read_user(((struct augmented_arg *)payload_offset)->v= alue, aug_size, arg)) > - augmented =3D true; > - } > - } > - > - /* Augmented data size is limited to sizeof(augmented_arg->unnamed uni= on with value field) */ > - if (aug_size > value_size) > - aug_size =3D value_size; > - > - /* write data to payload */ > - if (augmented) { > - int written =3D offsetof(struct augmented_arg, value) + aug_size; > - > - if (written < 0 || written > sizeof(struct augmented_arg)) > - return 1; > - > - ((struct augmented_arg *)payload_offset)->size =3D aug_size; > - output +=3D written; > - payload_offset +=3D written; > - do_output =3D true; > - } > - } > + struct args_loop_ctx loop_ctx =3D { > + .args =3D args, > + .beauty_map =3D beauty_map, > + .payload_offset =3D payload_offset, > + .value_size =3D value_size, > + .output =3D &output, > + .do_output =3D &do_output > + }; > + iters =3D 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.