Linux Trace Kernel
 help / color / mirror / Atom feed
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: "Markus Schneider-Pargmann" <msp@baylibre.com>
Cc: "Steven Rostedt" <rostedt@goodmis.org>,
	"Mathieu Desnoyers" <mathieu.desnoyers@efficios.com>,
	"Heiko Carstens" <hca@linux.ibm.com>,
	<linux-kernel@vger.kernel.org>,
	<linux-trace-kernel@vger.kernel.org>
Subject: Re: [PATCH] tracing: fprobe: Remove __packed from generic __fprobe_header
Date: Mon, 13 Jul 2026 11:23:39 +0900	[thread overview]
Message-ID: <20260713112339.53ed390e1c6f18b2fb1cd901@kernel.org> (raw)
In-Reply-To: <DJU105LESLBJ.1O3UZRY325T2M@baylibre.com>

On Thu, 09 Jul 2026 14:06:32 +0200
"Markus Schneider-Pargmann" <msp@baylibre.com> wrote:

> Hi Masami,
> 
> On Mon Jun 22, 2026 at 6:56 AM CEST, Masami Hiramatsu wrote:
> > On Wed, 10 Jun 2026 11:20:24 +0200
> > "Markus Schneider-Pargmann" <msp@baylibre.com> wrote:
> >
> >> Hi Masami,
> >> 
> >> On Wed Jun 10, 2026 at 10:17 AM CEST, Masami Hiramatsu wrote:
> >> > Hi Markus,
> >> >
> >> > Thanks for ping me.
> >> >
> >> > On Tue, 28 Apr 2026 10:30:29 +0200
> >> > "Markus Schneider-Pargmann (The Capable Hub)" <msp@baylibre.com> wrote:
> >> >
> >> >> fp pointer and unsigned long have the same size on all relevant
> >> >> architectures that build Linux. Furthermore this struct is only used in
> >> >> architectures that do not set ARCH_DEFINE_ENCODE_FPROBE_HEADER which is
> >> >> set only for 64bit architectures (apart from LoongArch).
> >> >> 
> >> >> Both fields are aligned on these architectures so the struct with
> >> >> __packed and without it are the same.
> >> >> 
> >> >> Remove the __packed as it is unnecessary.
> >> >> 
> >> >> Fixes: 4346ba160409 ("fprobe: Rewrite fprobe on function-graph tracer")
> >> >
> >> > NOTE: This is not a Fix, but just cleanup or minor update. Or, you have
> >> > any problem with this __packed attribute?
> >> 
> >> Thanks, yes it is not fixing a bug, I can remove this.
> >> 
> >> >
> >> > Unless there is no problem (or any concern), I would like to keep this
> >> > as it is.
> >> 
> >> There is currently no problem with __packed in the upstream kernel. I
> >> just thought this would be a good cleanup to remove the unnecessary
> >> attribute. I am working on CHERI architectures where pointers have
> >> capabilities. __packed breaks these capability tags and therefore
> >> doesn't work on CHERI. When looking into why this struct has a __packed
> >> attribute I didn't see a reason, so I thought this would be a good patch
> >> for upstream as well even though CHERI is not yet relevant for upstream
> >> linux.
> >
> > Hi Markus,
> >
> > Ah, OK. CHERI makes pointers to non-long value. Are you sure
> > removing __packed makes fprobe working with CHERI? If so,
> > please describe it. Then I can pick it.
> 
> Sorry for the late response.
> 
> No, this patch alone doesn't make fprobe work with CHERI. __packed is
> problematic for CHERI because it sets the alignment to 1 which forces
> the compiler to do unaligned access operations even if the
> struct/pointer is aligned. Unaligned operations do not keep the CHERI
> capabilities. Without __packed the compiler knows everything is aligned
> and can use aligned operations and keep the CHERI capabilities. CHERI
> hardware does not support unaligned pointer capability access.
> 
> On its own it doesn't help CHERI, there are more patches that are more
> CHERI specific that are needed for fprobe to work on CHERI. I just
> thought it would be a great standalone patch for upstream.

OK, anyway, I would like to pick your patch but without fixes tag.
It may make no differences, so should be treated as a cleanup patch.
Let me pick it to probes/for-next.

Thank you,

> 
> Best
> Markus


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

  reply	other threads:[~2026-07-13  2:23 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-28  8:30 [PATCH] tracing: fprobe: Remove __packed from generic __fprobe_header Markus Schneider-Pargmann (The Capable Hub)
2026-06-10  7:17 ` Markus Schneider-Pargmann
2026-06-10  8:17 ` Masami Hiramatsu
2026-06-10  9:20   ` Markus Schneider-Pargmann
2026-06-22  4:56     ` Masami Hiramatsu
2026-07-09 12:06       ` Markus Schneider-Pargmann
2026-07-13  2:23         ` Masami Hiramatsu [this message]
2026-06-10 11:06   ` David Laight
2026-06-10 19:51     ` Steven Rostedt
2026-06-10 20:05       ` Mathieu Desnoyers
2026-06-12 12:51         ` Markus Schneider-Pargmann
2026-06-12 16:36           ` Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260713112339.53ed390e1c6f18b2fb1cd901@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=hca@linux.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=msp@baylibre.com \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox