All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.