From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: multipart/mixed; boundary="===============2362638093594806641==" MIME-Version: 1.0 From: Gustavo A. R. Silva To: kbuild-all@lists.01.org Subject: Re: [peterz-queue:perf/core 26/39] arch/x86/include/asm/perf_event.h:290:19: error: flexible array member in a struct with no named members Date: Tue, 07 Jul 2020 10:05:35 -0500 Message-ID: <20200707150535.GB11125@embeddedor> In-Reply-To: <781e0ba9-0817-88c2-af20-6b743c225805@linux.intel.com> List-Id: --===============2362638093594806641== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable On Tue, Jul 07, 2020 at 10:50:47AM -0400, Liang, Kan wrote: > = > = > On 7/7/2020 9:51 AM, Gustavo A. R. Silva wrote: > > On Tue, Jul 07, 2020 at 12:24:43PM +0200, Peter Zijlstra wrote: > > > On Mon, Jul 06, 2020 at 09:08:41PM -0400, Liang, Kan wrote: > > > = > > > > > 288 = > > > > > 289 struct pebs_lbr { > > > > > > 290 struct lbr_entry lbr[]; /* Variable length */ > > > > > 291 }; > > > > > 292 = > > > > = > > > > The struct pebs_lbr only has one member. To fix the warning, it see= ms we > > > > have to define the struct as below. > > > > = > > > > = > > > > diff --git a/arch/x86/include/asm/perf_event.h > > > > b/arch/x86/include/asm/perf_event.h > > > > index 2338200..a387e14 100644 > > > > --- a/arch/x86/include/asm/perf_event.h > > > > +++ b/arch/x86/include/asm/perf_event.h > > > > @@ -283,7 +283,7 @@ struct pebs_xmm { > > > > }; > > > > = > > > > struct pebs_lbr { > > > > - struct lbr_entry lbr[]; /* Variable length */ > > > > + struct lbr_entry lbr[0]; /* Variable length */ > > > > }; > > > > = > > > = > > > Right, but then we get trouble like the below again. There's basically > > = > > Yep, zero-length and one-element arrays are being deprecated[1]. > > >> no good solution here :-( > > > = > > > Gustavo, any clues? > > > = > > = > > Yep; the issue is that "Flexible array members may only appear as the > > last member of a struct that is otherwise non-empty"[2]. > > = > > The solution is to declare something like this: > > = > > struct pebs_lbr { > > size_t num_lbr; /* count for number of lbr entries */ > > struct lbr_entry lbr[]; > > }; > > = > > and allocate memory as follows: > = > It's kind different for PEBS LBR. We don't allocate a buffer for struct > pebs_lbr. We just point it to a big memory (PEBS records) at run time whi= ch > includes not only LBR data but also other data. > = > With the above structure, there is an error when I assign the pointer via > lbr.lbr =3D (struct lbr_entry *) next_record;. > "error: invalid use of flexible array member" > = > I think we may want to drop the struct pebs_lbr and directly pass the > pointer to intel_pmu_store_lbr(). > (The patch as below hasn't been tested yet.) > = > diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c > index 0d33f85..13f18f6 100644 > --- a/arch/x86/events/intel/ds.c > +++ b/arch/x86/events/intel/ds.c > @@ -1595,13 +1595,12 @@ static void setup_pebs_adaptive_sample_data(struct > perf_event *event, > } > = > if (format_size & PEBS_DATACFG_LBRS) { > - struct pebs_lbr *lbr =3D next_record; ^^^^^^ Why don't you just directly use lbr_entry here instead of pebs_lbr and avoid the cast below... > int num_lbr =3D ((format_size >> PEBS_DATACFG_LBR_SHIFT) > & 0xff) + 1; > next_record =3D next_record + num_lbr * sizeof(struct lbr_entry); > = > if (has_branch_stack(event)) { > - intel_pmu_store_pebs_lbrs(lbr); > + intel_pmu_store_pebs_lbrs((struct lbr_entry *)next_record); > data->br_stack =3D &cpuc->lbr_stack; > } > } > diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c > index db4cdce..0f04169 100644 > --- a/arch/x86/events/intel/lbr.c > +++ b/arch/x86/events/intel/lbr.c > @@ -1461,7 +1461,7 @@ intel_pmu_lbr_filter(struct cpu_hw_events *cpuc) > } > } > = > -void intel_pmu_store_pebs_lbrs(struct pebs_lbr *lbr) > +void intel_pmu_store_pebs_lbrs(struct lbr_entry *lbr) > { > struct cpu_hw_events *cpuc =3D this_cpu_ptr(&cpu_hw_events); > = > @@ -1472,7 +1472,7 @@ void intel_pmu_store_pebs_lbrs(struct pebs_lbr *lbr) > else > cpuc->lbr_stack.hw_idx =3D intel_pmu_lbr_tos(); > = > - intel_pmu_store_lbr(cpuc, lbr->lbr, lbr_need_info(cpuc)); > + intel_pmu_store_lbr(cpuc, lbr, lbr_need_info(cpuc)); > = > intel_pmu_lbr_filter(cpuc); > } > diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h > index 466e1f9..7b68ab5 100644 > --- a/arch/x86/events/perf_event.h > +++ b/arch/x86/events/perf_event.h > @@ -1122,7 +1122,7 @@ void intel_pmu_pebs_sched_task(struct > perf_event_context *ctx, bool sched_in); > = > void intel_pmu_auto_reload_read(struct perf_event *event); > = > -void intel_pmu_store_pebs_lbrs(struct pebs_lbr *lbr); > +void intel_pmu_store_pebs_lbrs(struct lbr_entry *lbr); > = > void intel_ds_init(void); > = > diff --git a/arch/x86/include/asm/perf_event.h > b/arch/x86/include/asm/perf_event.h > index a387e14..0c1b137 100644 > --- a/arch/x86/include/asm/perf_event.h > +++ b/arch/x86/include/asm/perf_event.h > @@ -282,10 +282,6 @@ struct pebs_xmm { > u64 xmm[16*2]; /* two entries for each register */ > }; > = > -struct pebs_lbr { > - struct lbr_entry lbr[0]; /* Variable length */ > -}; > - > /* > * IBS cpuid feature detection > */ > = > Thanks, > Kan -- Gustavo --===============2362638093594806641==--