Hi, On Wed Jun 10, 2026 at 10:05 PM CEST, Mathieu Desnoyers wrote: > On 2026-06-10 15:51, Steven Rostedt wrote: >> On Wed, 10 Jun 2026 12:06:59 +0100 >> David Laight <[email protected]> wrote: >> >>> 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) >>>>> <[email protected]> >>>>> --- >>>>> 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; >>>>> +}; >>>>> >> >> Does "__packed" really do anything between a pointer and a long? > > If that structure is allocated at a non-void-ptr-aligned address, the > packed attribute will ensure that the compiler don't emit instructions > that require aligned loads/stores when accessing those fields. > > It does not change the layout of the structure per se in this specific > case, but it informs the compiler about the lack of guarantees about > alignment for the entire structure. > > x86 32/64 cannot care less about this, but it's relevant on other > architectures.
Thanks for your feedback. I checked this before submitting the patch.
The struct is always aligned to sizeof(long):
struct __fprobe_header is only ever accessed through
read_fprobe_header() and write_fprobe_header(). Since the read will only
read what we have previously written, only the write part is relevant
here. write_fprobe_header() is only called from fprobe_fgraph_entry():
if (write_fprobe_header(&fgraph_data[used], fp, size_words))
used += FPROBE_HEADER_SIZE_IN_LONG + size_words;
used is always kept aligned to sizeof(long), in fact the above snippet
is the only part where it is actually changed. fgraph_data is assigned
here:
fgraph_data = fgraph_reserve_data(gops->idx, reserved_words * sizeof(long));
fgraph_reserve_data() returns a pointer into an unsigned long array
ret_stack. ret_stack is allocated with
ret_stack = kmem_cache_alloc(fgraph_stack_cachep, GFP_KERNEL);
and fgraph_stack_cachep is allocated with
fgraph_stack_cachep = kmem_cache_create("fgraph_stack",
SHADOW_STACK_SIZE,
SHADOW_STACK_SIZE, 0, NULL);
So as far as I can see everything is sizeof(long) aligned here and it is
not allocated at a non-void-ptr-aligned address.
Best
Markus
signature.asc
Description: PGP signature
