From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f44.google.com (mail-wm1-f44.google.com [209.85.128.44]) (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 69DB8309DDD for ; Wed, 3 Dec 2025 20:23:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.44 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764793438; cv=none; b=biHo4N1oF3iNg4EmlqEZDBPEfgu3Z9xlGTtXfIBJeiPEC2gB4053gA5BkbHGY7LW3Gk+LBTToK55ZxfWaIlfxEhjWYjuzn2HzRsg5D52TF+iorv/zrHrbX42bSslb91L+0AaQoXc2tDfaMIenVJZgVJREFeidLaJmwcvolit7Q4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764793438; c=relaxed/simple; bh=H3qq8NXAbCoDl1lI9pvTZNbJD7FkSGRSasmvrrHI8nw=; h=From:Date:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Lzw6YHf+Ib/OmnIWagDqCobpWkW1rFd3LXxMIwGP4wru2zjNcuHcqBG8oI8CBnkGtK+ZXaYgGq2qTgp53CTRStC9+f37xVO8sa3ndX2qBKpCif8m0Q6Gg9W0xAEHpBwE67zIOc/9OVssKHdnwSwojOOOTMJ51TsiFxJJOyhxKrY= 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=dAecPv5Q; arc=none smtp.client-ip=209.85.128.44 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="dAecPv5Q" Received: by mail-wm1-f44.google.com with SMTP id 5b1f17b1804b1-4779ce2a624so1880645e9.2 for ; Wed, 03 Dec 2025 12:23:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1764793430; x=1765398230; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:date:from:from:to :cc:subject:date:message-id:reply-to; bh=zi2Tym97Iw47XsW0KDcn1kY+fahoY4a+WRZ6zbXd8IA=; b=dAecPv5QWr1qwfgxSNEAoxGWtuV7+93+QdDtLpVgzPQsow/jkn8r0v6gA4nMB3WAQd yUqLIpD9qHcTSBPKecqpL0lCurp1rs4XvZTvtqW16yF20Dz+/5SPFjtEdrxXM+UyUl/a 6poF8y0U5bKA5DfMcbEFaTTHNpjhtDfUV3G0xfKckvlfjZDcNJQAJrna2uBldc8H8rh1 NOQaHEExkPrjh8Qx1O5x9JXxPbkncTuE+FC1Br3V7viWLzNS2HiV8txxTrETIZqqawFQ VeMK1I0CJTShm1LXIYT0K9KM6ACvHSJMLeboDNiJlpMTPowM53F8SG2jsJ/kfRrmra5t RXtg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1764793430; x=1765398230; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:date:from:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=zi2Tym97Iw47XsW0KDcn1kY+fahoY4a+WRZ6zbXd8IA=; b=ZXXykq9++VfljmJ9RpMsgJ0NuK+4AJnI0NhObFHggeS0lUGcKW/SY5HlbK+O18vXr2 7RebiJlX/o8zQV+bi9cftiYA+jGPwSqxdoCjxGipT6HD3FfGHuy34O7AymqzoU3jA45M 5yAkVyDgPyvlqAJcJsQ6Ec+qo9R1oHdUmHQv0V5tJ3MjjxIdjIe8PDDqlccGix9Higp6 6GiXU6AK/76gAYFimOw7bklIMz+SLAEUct/tCo2XsNnDBNZZwed3Bx08f7O9lk/pK9BI KK0HTymBU2RZEd4yWYcCfR7Z4kLrsMMcwu9cJGm+kZ8k/FbfiRhqfOjgOBp3428P4PuH mCeA== X-Forwarded-Encrypted: i=1; AJvYcCXINc9VZ5xE3u6wgpGR53K8HM/evFEx9RGAJrri/8JVzbrswxm0SMP31f2HfmHCuCa5eyXzU8LxiYXuDbs=@vger.kernel.org X-Gm-Message-State: AOJu0Yxaz1hvOX/DtuhV9X+4Iw//vCLpPr+bUsaHxOepjamfwQqA7KmH SqoVacFN9L8EPpY+mbV9BwDCvrb6aT0lTqBPV5VExzMTcPNfnBD0NV6O X-Gm-Gg: ASbGncsvTJqkH+Ew2VzL6HufRKeLSD7pV4PYqnpzaBcN+M9FkF2kCpI90r2mk1GLRgT JuKbQ9tGNIg5Kom3Wp3yyN4awk0IzRVIbKi2NtgyWQv0K3lKtE7X9MPcWPJ6YMjyA/8S4dJTz5G Nz7bEpTDadxdwKtRNCrHUJRFQ+oON8046wj6bi0MM1adDrdDWgMeRziIMTi6SjwFG2GQBnfsny2 VRTTX1lX62ohrdYV98JVKpwT3TMQ/qhS+Xwnitq8AvnGvQyBFSrP1FBnZ3A3vgoZFC2kr1bquXd +ri7qc3QSCrE999kcGvlRRl1/fEtLiOQP83JV8Ig1mWpB3lzm1Hu0gz+jXs4/WGX8ueJu1r5E3i S31dKrzDefWk0whXaY4udtCRYpF72QY3Uu5P2ph4icU3Q7h8z6VTkNbn2CSVcfn6iAcEwuQKW1Q o= X-Google-Smtp-Source: AGHT+IHAWH1Cpvq5B81/xkivbTXgTXuutxLETKY/29bdgzeJHM6sjIXHH155EYl31r8m6eHuaFG7fg== X-Received: by 2002:a05:6000:22c2:b0:428:4004:8241 with SMTP id ffacd0b85a97d-42f79851c93mr237747f8f.40.1764793429799; Wed, 03 Dec 2025 12:23:49 -0800 (PST) Received: from krava ([176.74.159.170]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-42e1c5d614asm40900203f8f.12.2025.12.03.12.23.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 03 Dec 2025 12:23:49 -0800 (PST) From: Jiri Olsa X-Google-Original-From: Jiri Olsa Date: Wed, 3 Dec 2025 21:23:47 +0100 To: Menglong Dong Cc: Steven Rostedt , Florent Revest , Mark Rutland , bpf@vger.kernel.org, linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Song Liu Subject: Re: [PATCHv4 bpf-next 1/9] ftrace,bpf: Remove FTRACE_OPS_FL_JMP ftrace_ops flag Message-ID: References: <20251203082402.78816-1-jolsa@kernel.org> <20251203082402.78816-2-jolsa@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Wed, Dec 03, 2025 at 05:15:52PM +0800, Menglong Dong wrote: > On Wed, Dec 3, 2025 at 4:24 PM Jiri Olsa wrote: > > > > At the moment the we allow the jmp attach only for ftrace_ops that > > has FTRACE_OPS_FL_JMP set. This conflicts with following changes > > where we use single ftrace_ops object for all direct call sites, > > so all could be be attached via just call or jmp. > > > > We already limit the jmp attach support with config option and bit > > (LSB) set on the trampoline address. It turns out that's actually > > enough to limit the jmp attach for architecture and only for chosen > > addresses (with LSB bit set). > > > > Each user of register_ftrace_direct or modify_ftrace_direct can set > > the trampoline bit (LSB) to indicate it has to be attached by jmp. > > > > The bpf trampoline generation code uses trampoline flags to generate > > jmp-attach specific code and ftrace inner code uses the trampoline > > bit (LSB) to handle return from jmp attachment, so there's no harm > > to remove the FTRACE_OPS_FL_JMP bit. > > > > The fexit/fmodret performance stays the same (did not drop), > > current code: > > > > fentry : 77.904 ± 0.546M/s > > fexit : 62.430 ± 0.554M/s > > fmodret : 66.503 ± 0.902M/s > > > > with this change: > > > > fentry : 80.472 ± 0.061M/s > > fexit : 63.995 ± 0.127M/s > > fmodret : 67.362 ± 0.175M/s > > > > Fixes: 25e4e3565d45 ("ftrace: Introduce FTRACE_OPS_FL_JMP") > > Signed-off-by: Jiri Olsa > > --- > > include/linux/ftrace.h | 1 - > > kernel/bpf/trampoline.c | 32 ++++++++++++++------------------ > > kernel/trace/ftrace.c | 14 -------------- > > 3 files changed, 14 insertions(+), 33 deletions(-) > > > > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h > > index 015dd1049bea..505b7d3f5641 100644 > > --- a/include/linux/ftrace.h > > +++ b/include/linux/ftrace.h > > @@ -359,7 +359,6 @@ enum { > > FTRACE_OPS_FL_DIRECT = BIT(17), > > FTRACE_OPS_FL_SUBOP = BIT(18), > > FTRACE_OPS_FL_GRAPH = BIT(19), > > - FTRACE_OPS_FL_JMP = BIT(20), > > Yeah, the FTRACE_OPS_FL_JMP is not necessary. I added > it in case that we maybe want to implement such "jmp" for > ftrace trampoline in the feature. But it's OK to remove it now. > > > }; > > > > #ifndef CONFIG_DYNAMIC_FTRACE_WITH_ARGS > > diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c > > index 976d89011b15..b9a358d7a78f 100644 > > --- a/kernel/bpf/trampoline.c > > +++ b/kernel/bpf/trampoline.c > > @@ -214,10 +214,15 @@ static int modify_fentry(struct bpf_trampoline *tr, u32 orig_flags, > > int ret; > > > > if (tr->func.ftrace_managed) { > > + unsigned long addr = (unsigned long) new_addr; > > + > > + if (bpf_trampoline_use_jmp(tr->flags)) > > + addr = ftrace_jmp_set(addr); I wanted to get rid of the void * -> unsigned long casting in all the places.. this way it has to be just on one place above, but maybe we could have already direct_ops_add with unsigned long addr, will check jirka > > nit: It seems that we can remove the variable "addr" can use > the "new_addr" directly? > > > + > > if (lock_direct_mutex) > > - ret = modify_ftrace_direct(tr->fops, (long)new_addr); > > + ret = modify_ftrace_direct(tr->fops, addr); > > else > > - ret = modify_ftrace_direct_nolock(tr->fops, (long)new_addr); > > + ret = modify_ftrace_direct_nolock(tr->fops, addr); > > } else { > > ret = bpf_trampoline_update_fentry(tr, orig_flags, old_addr, > > new_addr); > > @@ -240,10 +245,15 @@ static int register_fentry(struct bpf_trampoline *tr, void *new_addr) > > } > > > > if (tr->func.ftrace_managed) { > > + unsigned long addr = (unsigned long) new_addr; > > + > > + if (bpf_trampoline_use_jmp(tr->flags)) > > + addr = ftrace_jmp_set(addr); > > And here. > > Thanks! > Menglong Dong > > > + > [...] > >