Re: [PATCH] tracing: fprobe: Remove __packed from generic __fprobe_header

[David Laight]

On Wed, 10 Jun 2026 17:17:40 +0900
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx> wrote:

> Hi Markus,
> 
> Thanks for ping me.
> 
> On Tue, 28 Apr 2026 10:30:29 +0200
> "Markus Schneider-Pargmann (The Capable Hub)" <msp@xxxxxxxxxxxx> 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?
> 
> Unless there is no problem (or any concern), I would like to keep this
> as it is.

There is likely to be a difference on architectures that fault misaligned
accesses.
On those gcc will use multiple byte-sized accesses (and a log of shifts etc)
for code that accesses those members because it will assume that the
structure itself can be misaligned.

So you only want __packed on structures that might be misaligned and those
that contain misaligned members.

If the structure is only guaranteed to be 32bit aligned then use __packed
__aligned(4) so that two 32bit accesses get used instead of 8 8bit ones.

-- David

> 
> Thank you,
> 
> > Signed-off-by: Markus Schneider-Pargmann (The Capable Hub) <msp@xxxxxxxxxxxx>
> > ---
> >  kernel/trace/fprobe.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
> > index cc49ebd2a773..21751dcdb7b9 100644
> > --- a/kernel/trace/fprobe.c
> > +++ b/kernel/trace/fprobe.c
> > @@ -181,7 +181,7 @@ static inline void read_fprobe_header(unsigned long *stack,
> >  struct __fprobe_header {
> >  	struct fprobe *fp;
> >  	unsigned long size_words;
> > -} __packed;
> > +};
> >  
> >  #define FPROBE_HEADER_SIZE_IN_LONG	SIZE_IN_LONG(sizeof(struct __fprobe_header))
> >  
> > 
> > ---
> > base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
> > change-id: 20260427-topic-fprobe-packed-v7-1-f44f9bbdedf6
> > 
> > Best regards,
> > --  
> > Markus Schneider-Pargmann (The Capable Hub) <msp@xxxxxxxxxxxx>
> >   
> 
>