On Mon, Aug 05, 2024 at 04:42:35PM -0600, Gustavo A. R. Silva wrote: > -Wflex-array-member-not-at-end was introduced in GCC-14, and we are > getting ready to enable it, globally. > > So, in order to avoid ending up with a flexible-array member in the > middle of multiple other structs, we use the `struct_group_tagged()` > helper to create a new tagged `struct perf_branch_stack_hdr`. > This structure groups together all the members of the flexible > `struct perf_branch_stack` except the flexible array. > > As a result, the array is effectively separated from the rest of the > members without modifying the memory layout of the flexible structure. > We then change the type of the middle struct members currently causing > trouble from `struct perf_branch_stack` to `struct perf_branch_stack_hdr`. > > We also want to ensure that when new members need to be added to the > flexible structure, they are always included within the newly created > tagged struct. For this, we use `static_assert()`. This ensures that the > memory layout for both the flexible structure and the new tagged struct > is the same after any changes. > > This approach avoids having to implement `struct perf_branch_stack_hdr` > as a completely separate structure, thus preventing having to maintain > two independent but basically identical structures, closing the door > to potential bugs in the future. > > We also use `container_of()` whenever we need to retrieve a pointer to > the flexible structure, through which the flexible-array member can be > accessed, if necessary. > > So, with these changes, fix the following warnings: > arch/x86/events/amd/../perf_event.h:289:41: warning: structure containing a > flexible array member is not at the end of another structure > [-Wflex-array-member-not-at-end] > arch/x86/events/intel/../perf_event.h:289:41: warning: structure containing a > flexible array member is not at the end of another structure > [-Wflex-array-member-not-at-end] > arch/x86/events/perf_event.h:289:41: warning: structure containing a flexible > array member is not at the end of another structure > [-Wflex-array-member-not-at-end] > arch/x86/events/zhaoxin/../perf_event.h:289:41: warning: structure containing > a flexible array member is not at the end of another structure > [-Wflex-array-member-not-at-end] > ./arch/x86/include/generated/../../events/perf_event.h:289:41: warning: > structure containing a flexible array member is not at the end of another > structure [-Wflex-array-member-not-at-end] > arch/x86/xen/../events/perf_event.h:289:41: warning: structure containing a > flexible array member is not at the end of another structure > [-Wflex-array-member-not-at-end]
Urgh, this fugly.. And you're breaking coding style :-( Let me see if this can't be done sanely... > Signed-off-by: Gustavo A. R. Silva <[email protected]> > --- > arch/x86/events/amd/core.c | 4 +++- > arch/x86/events/core.c | 4 +++- > arch/x86/events/intel/ds.c | 4 +++- > arch/x86/events/intel/lbr.c | 8 ++++++-- > arch/x86/events/perf_event.h | 2 +- > include/linux/perf_event.h | 9 +++++++-- > 6 files changed, 23 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c > index 920e3a640cad..37adf2fcf508 100644 > --- a/arch/x86/events/amd/core.c > +++ b/arch/x86/events/amd/core.c > @@ -1001,7 +1001,9 @@ static int amd_pmu_v2_handle_irq(struct pt_regs *regs) > continue; > > if (has_branch_stack(event)) > - perf_sample_save_brstack(&data, event, > &cpuc->lbr_stack, NULL); > + perf_sample_save_brstack(&data, event, > + container_of(&cpuc->lbr_stack, struct > perf_branch_stack, hdr), > + NULL); > > if (perf_event_overflow(event, &data, regs)) > x86_pmu_stop(event, 0); > diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c > index 12f2a0c14d33..e7faf7eedfc4 100644 > --- a/arch/x86/events/core.c > +++ b/arch/x86/events/core.c > @@ -1704,7 +1704,9 @@ int x86_pmu_handle_irq(struct pt_regs *regs) > perf_sample_data_init(&data, 0, event->hw.last_period); > > if (has_branch_stack(event)) > - perf_sample_save_brstack(&data, event, > &cpuc->lbr_stack, NULL); > + perf_sample_save_brstack(&data, event, > + container_of(&cpuc->lbr_stack, struct > perf_branch_stack, hdr), > + NULL); > > if (perf_event_overflow(event, &data, regs)) > x86_pmu_stop(event, 0); > diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c > index fa5ea65de0d0..d5c86edbeaa9 100644 > --- a/arch/x86/events/intel/ds.c > +++ b/arch/x86/events/intel/ds.c > @@ -1869,7 +1869,9 @@ static void setup_pebs_fixed_sample_data(struct > perf_event *event, > setup_pebs_time(event, data, pebs->tsc); > > if (has_branch_stack(event)) > - perf_sample_save_brstack(data, event, &cpuc->lbr_stack, NULL); > + perf_sample_save_brstack(data, event, > + container_of(&cpuc->lbr_stack, struct > perf_branch_stack, hdr), > + NULL); > } > > static void adaptive_pebs_save_regs(struct pt_regs *regs, > diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c > index dc641b50814e..79ca1a2d6ff4 100644 > --- a/arch/x86/events/intel/lbr.c > +++ b/arch/x86/events/intel/lbr.c > @@ -974,11 +974,15 @@ void intel_pmu_lbr_save_brstack(struct perf_sample_data > *data, > { > if (is_branch_counters_group(event)) { > intel_pmu_lbr_counters_reorder(cpuc, event); > - perf_sample_save_brstack(data, event, &cpuc->lbr_stack, > cpuc->lbr_counters); > + perf_sample_save_brstack(data, event, > + container_of(&cpuc->lbr_stack, struct > perf_branch_stack, hdr), > + cpuc->lbr_counters); > return; > } > > - perf_sample_save_brstack(data, event, &cpuc->lbr_stack, NULL); > + perf_sample_save_brstack(data, event, > + container_of(&cpuc->lbr_stack, struct perf_branch_stack, hdr), > + NULL); > } > > static void intel_pmu_arch_lbr_read(struct cpu_hw_events *cpuc) > diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h > index ac1182141bf6..bf05cdc3f22f 100644 > --- a/arch/x86/events/perf_event.h > +++ b/arch/x86/events/perf_event.h > @@ -286,7 +286,7 @@ struct cpu_hw_events { > */ > int lbr_users; > int lbr_pebs_users; > - struct perf_branch_stack lbr_stack; > + struct perf_branch_stack_hdr lbr_stack; > struct perf_branch_entry lbr_entries[MAX_LBR_ENTRIES]; > u64 lbr_counters[MAX_LBR_ENTRIES]; /* > branch stack extra */ > union { > diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h > index 1a8942277dda..eb4413917b39 100644 > --- a/include/linux/perf_event.h > +++ b/include/linux/perf_event.h > @@ -121,10 +121,15 @@ static __always_inline bool perf_raw_frag_last(const > struct perf_raw_frag *frag) > * already stored in age order, the hw_idx should be 0. > */ > struct perf_branch_stack { > - __u64 nr; > - __u64 hw_idx; > + /* New members MUST be added within the struct_group() macro below. */ > + struct_group_tagged(perf_branch_stack_hdr, hdr, > + __u64 nr; > + __u64 hw_idx; > + ); > struct perf_branch_entry entries[]; > }; > +static_assert(offsetof(struct perf_branch_stack, entries) == sizeof(struct > perf_branch_stack_hdr), > + "struct member likely outside of struct_group_tagged()"); > > struct task_struct; > > -- > 2.34.1 >
