* [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
@ 2020-07-06 19:39 kernel test robot
2020-07-07 1:08 ` Liang, Kan
0 siblings, 1 reply; 10+ messages in thread
From: kernel test robot @ 2020-07-06 19:39 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 10230 bytes --]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git perf/core
head: beb0aaf811f551900f60359d0ca5288257f03664
commit: b7e384bec88604ec36df7a4667a09a4749b943d3 [26/39] perf/x86/intel/lbr: Unify the stored format of LBR information
config: i386-tinyconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
reproduce (this is a W=1 build):
git checkout b7e384bec88604ec36df7a4667a09a4749b943d3
# save the attached .config to linux build tree
make W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
In file included from include/linux/perf_event.h:25,
from include/linux/trace_events.h:10,
from include/trace/syscall.h:7,
from include/linux/syscalls.h:85,
from kernel/fork.c:54:
>> arch/x86/include/asm/perf_event.h:290:19: error: flexible array member in a struct with no named members
290 | struct lbr_entry lbr[]; /* Variable length */
| ^~~
--
In file included from include/linux/perf_event.h:25,
from include/linux/trace_events.h:10,
from include/trace/syscall.h:7,
from include/linux/syscalls.h:85,
from kernel/exit.c:42:
>> arch/x86/include/asm/perf_event.h:290:19: error: flexible array member in a struct with no named members
290 | struct lbr_entry lbr[]; /* Variable length */
| ^~~
kernel/exit.c:1714:13: warning: no previous prototype for 'abort' [-Wmissing-prototypes]
1714 | __weak void abort(void)
| ^~~~~
--
In file included from include/linux/perf_event.h:25,
from include/linux/trace_events.h:10,
from include/trace/syscall.h:7,
from include/linux/syscalls.h:85,
from kernel/time/hrtimer.c:30:
>> arch/x86/include/asm/perf_event.h:290:19: error: flexible array member in a struct with no named members
290 | struct lbr_entry lbr[]; /* Variable length */
| ^~~
kernel/time/hrtimer.c:120:21: warning: initialized field overwritten [-Woverride-init]
120 | [CLOCK_REALTIME] = HRTIMER_BASE_REALTIME,
| ^~~~~~~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:120:21: note: (near initialization for 'hrtimer_clock_to_base_table[0]')
kernel/time/hrtimer.c:121:22: warning: initialized field overwritten [-Woverride-init]
121 | [CLOCK_MONOTONIC] = HRTIMER_BASE_MONOTONIC,
| ^~~~~~~~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:121:22: note: (near initialization for 'hrtimer_clock_to_base_table[1]')
kernel/time/hrtimer.c:122:21: warning: initialized field overwritten [-Woverride-init]
122 | [CLOCK_BOOTTIME] = HRTIMER_BASE_BOOTTIME,
| ^~~~~~~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:122:21: note: (near initialization for 'hrtimer_clock_to_base_table[7]')
kernel/time/hrtimer.c:123:17: warning: initialized field overwritten [-Woverride-init]
123 | [CLOCK_TAI] = HRTIMER_BASE_TAI,
| ^~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:123:17: note: (near initialization for 'hrtimer_clock_to_base_table[11]')
kernel/time/hrtimer.c: In function '__run_hrtimer':
kernel/time/hrtimer.c:1483:7: warning: variable 'expires_in_hardirq' set but not used [-Wunused-but-set-variable]
1483 | bool expires_in_hardirq;
| ^~~~~~~~~~~~~~~~~~
--
In file included from include/linux/perf_event.h:25,
from include/linux/trace_events.h:10,
from include/trace/syscall.h:7,
from include/linux/syscalls.h:85,
from kernel/time/posix-stubs.c:13:
>> arch/x86/include/asm/perf_event.h:290:19: error: flexible array member in a struct with no named members
290 | struct lbr_entry lbr[]; /* Variable length */
| ^~~
kernel/time/posix-stubs.c:25:17: warning: no previous prototype for 'sys_ni_posix_timers' [-Wmissing-prototypes]
25 | asmlinkage long sys_ni_posix_timers(void)
| ^~~~~~~~~~~~~~~~~~~
kernel/time/posix-stubs.c:73:5: warning: no previous prototype for 'do_clock_gettime' [-Wmissing-prototypes]
73 | int do_clock_gettime(clockid_t which_clock, struct timespec64 *tp)
| ^~~~~~~~~~~~~~~~
--
In file included from include/linux/perf_event.h:25,
from include/linux/trace_events.h:10,
from include/trace/syscall.h:7,
from include/linux/syscalls.h:85,
from kernel/events/core.c:34:
>> arch/x86/include/asm/perf_event.h:290:19: error: flexible array member in a struct with no named members
290 | struct lbr_entry lbr[]; /* Variable length */
| ^~~
kernel/events/core.c:6493:6: warning: no previous prototype for 'perf_pmu_snapshot_aux' [-Wmissing-prototypes]
6493 | long perf_pmu_snapshot_aux(struct perf_buffer *rb,
| ^~~~~~~~~~~~~~~~~~~~~
--
In file included from include/linux/perf_event.h:25,
from include/linux/trace_events.h:10,
from include/trace/syscall.h:7,
from include/linux/syscalls.h:85,
from kernel/printk/printk.c:36:
>> arch/x86/include/asm/perf_event.h:290:19: error: flexible array member in a struct with no named members
290 | struct lbr_entry lbr[]; /* Variable length */
| ^~~
In file included from kernel/printk/printk.c:60:
kernel/printk/internal.h:59:20: warning: no previous prototype for 'vprintk_func' [-Wmissing-prototypes]
59 | __printf(1, 0) int vprintk_func(const char *fmt, va_list args) { return 0; }
| ^~~~~~~~~~~~
kernel/printk/printk.c:174:5: warning: no previous prototype for 'devkmsg_sysctl_set_loglvl' [-Wmissing-prototypes]
174 | int devkmsg_sysctl_set_loglvl(struct ctl_table *table, int write,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
--
In file included from include/linux/perf_event.h:25,
from include/linux/trace_events.h:10,
from include/trace/syscall.h:7,
from include/linux/syscalls.h:85,
from kernel/sched/sched.h:65,
from kernel/sched/core.c:9:
>> arch/x86/include/asm/perf_event.h:290:19: error: flexible array member in a struct with no named members
290 | struct lbr_entry lbr[]; /* Variable length */
| ^~~
kernel/sched/core.c: In function 'ttwu_stat':
kernel/sched/core.c:2160:13: warning: variable 'rq' set but not used [-Wunused-but-set-variable]
2160 | struct rq *rq;
| ^~
--
In file included from include/linux/perf_event.h:25,
from include/linux/trace_events.h:10,
from include/trace/syscall.h:7,
from include/linux/syscalls.h:85,
from kernel/sched/sched.h:65,
from kernel/sched/fair.c:23:
>> arch/x86/include/asm/perf_event.h:290:19: error: flexible array member in a struct with no named members
290 | struct lbr_entry lbr[]; /* Variable length */
| ^~~
kernel/sched/fair.c:5352:6: warning: no previous prototype for 'init_cfs_bandwidth' [-Wmissing-prototypes]
5352 | void init_cfs_bandwidth(struct cfs_bandwidth *cfs_b) {}
| ^~~~~~~~~~~~~~~~~~
kernel/sched/fair.c:11078:6: warning: no previous prototype for 'free_fair_sched_group' [-Wmissing-prototypes]
11078 | void free_fair_sched_group(struct task_group *tg) { }
| ^~~~~~~~~~~~~~~~~~~~~
kernel/sched/fair.c:11080:5: warning: no previous prototype for 'alloc_fair_sched_group' [-Wmissing-prototypes]
11080 | int alloc_fair_sched_group(struct task_group *tg, struct task_group *parent)
| ^~~~~~~~~~~~~~~~~~~~~~
kernel/sched/fair.c:11085:6: warning: no previous prototype for 'online_fair_sched_group' [-Wmissing-prototypes]
11085 | void online_fair_sched_group(struct task_group *tg) { }
| ^~~~~~~~~~~~~~~~~~~~~~~
kernel/sched/fair.c:11087:6: warning: no previous prototype for 'unregister_fair_sched_group' [-Wmissing-prototypes]
11087 | void unregister_fair_sched_group(struct task_group *tg) { }
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
--
In file included from include/linux/perf_event.h:25,
from include/linux/trace_events.h:10,
from include/trace/syscall.h:7,
from include/linux/syscalls.h:85,
from kernel/sched/sched.h:65,
from kernel/sched/rt.c:6:
>> arch/x86/include/asm/perf_event.h:290:19: error: flexible array member in a struct with no named members
290 | struct lbr_entry lbr[]; /* Variable length */
| ^~~
kernel/sched/rt.c:253:6: warning: no previous prototype for 'free_rt_sched_group' [-Wmissing-prototypes]
253 | void free_rt_sched_group(struct task_group *tg) { }
| ^~~~~~~~~~~~~~~~~~~
kernel/sched/rt.c:255:5: warning: no previous prototype for 'alloc_rt_sched_group' [-Wmissing-prototypes]
255 | int alloc_rt_sched_group(struct task_group *tg, struct task_group *parent)
| ^~~~~~~~~~~~~~~~~~~~
kernel/sched/rt.c:668:6: warning: no previous prototype for 'sched_rt_bandwidth_account' [-Wmissing-prototypes]
668 | bool sched_rt_bandwidth_account(struct rt_rq *rt_rq)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
vim +290 arch/x86/include/asm/perf_event.h
288
289 struct pebs_lbr {
> 290 struct lbr_entry lbr[]; /* Variable length */
291 };
292
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 7300 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* 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
2020-07-06 19:39 [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 kernel test robot
@ 2020-07-07 1:08 ` Liang, Kan
2020-07-07 10:24 ` Peter Zijlstra
0 siblings, 1 reply; 10+ messages in thread
From: Liang, Kan @ 2020-07-07 1:08 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 1595 bytes --]
>>> arch/x86/include/asm/perf_event.h:290:19: error: flexible array member in a struct with no named members
> 290 | struct lbr_entry lbr[]; /* Variable length */
> | ^~~
> kernel/sched/rt.c:253:6: warning: no previous prototype for 'free_rt_sched_group' [-Wmissing-prototypes]
> 253 | void free_rt_sched_group(struct task_group *tg) { }
> | ^~~~~~~~~~~~~~~~~~~
> kernel/sched/rt.c:255:5: warning: no previous prototype for 'alloc_rt_sched_group' [-Wmissing-prototypes]
> 255 | int alloc_rt_sched_group(struct task_group *tg, struct task_group *parent)
> | ^~~~~~~~~~~~~~~~~~~~
> kernel/sched/rt.c:668:6: warning: no previous prototype for 'sched_rt_bandwidth_account' [-Wmissing-prototypes]
> 668 | bool sched_rt_bandwidth_account(struct rt_rq *rt_rq)
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~
>
> vim +290 arch/x86/include/asm/perf_event.h
>
> 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 seems 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 */
};
Thanks,
Kan
^ permalink raw reply related [flat|nested] 10+ messages in thread
* 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
2020-07-07 1:08 ` Liang, Kan
@ 2020-07-07 10:24 ` Peter Zijlstra
2020-07-07 13:51 ` Gustavo A. R. Silva
0 siblings, 1 reply; 10+ messages in thread
From: Peter Zijlstra @ 2020-07-07 10:24 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3710 bytes --]
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 seems 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
no good solution here :-(
Gustavo, any clues?
---
commit 04f5c362ec6d3ff0e14f1c05230b550da7f528a4
Author: Gustavo A. R. Silva <gustavoars@kernel.org>
Date: Thu May 7 14:21:41 2020 -0500
sched/fair: Replace zero-length array with flexible-array
The current codebase makes use of the zero-length array language
extension to the C90 standard, but the preferred mechanism to declare
variable-length types such as these ones is a flexible array member[1][2],
introduced in C99:
struct foo {
int stuff;
struct boo array[];
};
By making use of the mechanism above, we will get a compiler warning
in case the flexible array does not occur last in the structure, which
will help us prevent some kind of undefined behavior bugs from being
inadvertently introduced[3] to the codebase from now on.
Also, notice that, dynamic memory allocations won't be affected by
this change:
"Flexible array members have incomplete type, and so the sizeof operator
may not be applied. As a quirk of the original implementation of
zero-length arrays, sizeof evaluates to zero."[1]
sizeof(flexible-array-member) triggers a warning because flexible array
members have incomplete type[1]. There are some instances of code in
which the sizeof operator is being incorrectly/erroneously applied to
zero-length arrays and the result is zero. Such instances may be hiding
some bugs. So, this work (flexible-array member conversions) will also
help to get completely rid of those sorts of issues.
This issue was found with the help of Coccinelle.
[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://github.com/KSPP/linux/issues/21
[3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/20200507192141.GA16183(a)embeddedor
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 44b0c8edc260..01f94cf52783 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1094,7 +1094,7 @@ struct numa_group {
* more by CPU use than by memory faults.
*/
unsigned long *faults_cpu;
- unsigned long faults[0];
+ unsigned long faults[];
};
/*
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 21416b30c520..2bd2a222318a 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1462,7 +1462,7 @@ struct sched_group {
* by attaching extra space to the end of the structure,
* depending on how many CPUs the kernel has booted up with)
*/
- unsigned long cpumask[0];
+ unsigned long cpumask[];
};
static inline struct cpumask *sched_group_span(struct sched_group *sg)
^ permalink raw reply related [flat|nested] 10+ messages in thread
* 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
2020-07-07 10:24 ` Peter Zijlstra
@ 2020-07-07 13:51 ` Gustavo A. R. Silva
2020-07-07 14:44 ` Peter Zijlstra
2020-07-07 14:50 ` Liang, Kan
0 siblings, 2 replies; 10+ messages in thread
From: Gustavo A. R. Silva @ 2020-07-07 13:51 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4960 bytes --]
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 seems 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:
struct pebs_lbr *lbr_entries;
...
lbr_entries = kmalloc(struct_size(lbr_entries, lbr, count), GFP_KERNEL);
lbr_entries->num_lbr = count;
...
and then you can access each entry through the use of an index
delimited by lbr_entries->num_lbr (which is self-contained and
not any other random variable):
for (i = 0; i < lbr_entries->num_lbr; i++)
{
...
whatever = lbr_entries->lbr[i];
...
}
and so forth...
--
Gustavo
[1] https://lore.kernel.org/lkml/20200608213711.GA22271(a)embeddedor/
[2] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> ---
> commit 04f5c362ec6d3ff0e14f1c05230b550da7f528a4
> Author: Gustavo A. R. Silva <gustavoars@kernel.org>
> Date: Thu May 7 14:21:41 2020 -0500
>
> sched/fair: Replace zero-length array with flexible-array
>
> The current codebase makes use of the zero-length array language
> extension to the C90 standard, but the preferred mechanism to declare
> variable-length types such as these ones is a flexible array member[1][2],
> introduced in C99:
>
> struct foo {
> int stuff;
> struct boo array[];
> };
>
> By making use of the mechanism above, we will get a compiler warning
> in case the flexible array does not occur last in the structure, which
> will help us prevent some kind of undefined behavior bugs from being
> inadvertently introduced[3] to the codebase from now on.
>
> Also, notice that, dynamic memory allocations won't be affected by
> this change:
>
> "Flexible array members have incomplete type, and so the sizeof operator
> may not be applied. As a quirk of the original implementation of
> zero-length arrays, sizeof evaluates to zero."[1]
>
> sizeof(flexible-array-member) triggers a warning because flexible array
> members have incomplete type[1]. There are some instances of code in
> which the sizeof operator is being incorrectly/erroneously applied to
> zero-length arrays and the result is zero. Such instances may be hiding
> some bugs. So, this work (flexible-array member conversions) will also
> help to get completely rid of those sorts of issues.
>
> This issue was found with the help of Coccinelle.
>
> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> [2] https://github.com/KSPP/linux/issues/21
> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
>
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Link: https://lkml.kernel.org/r/20200507192141.GA16183(a)embeddedor
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 44b0c8edc260..01f94cf52783 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -1094,7 +1094,7 @@ struct numa_group {
> * more by CPU use than by memory faults.
> */
> unsigned long *faults_cpu;
> - unsigned long faults[0];
> + unsigned long faults[];
> };
>
> /*
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 21416b30c520..2bd2a222318a 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -1462,7 +1462,7 @@ struct sched_group {
> * by attaching extra space to the end of the structure,
> * depending on how many CPUs the kernel has booted up with)
> */
> - unsigned long cpumask[0];
> + unsigned long cpumask[];
> };
>
> static inline struct cpumask *sched_group_span(struct sched_group *sg)
^ permalink raw reply [flat|nested] 10+ messages in thread
* 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
2020-07-07 13:51 ` Gustavo A. R. Silva
@ 2020-07-07 14:44 ` Peter Zijlstra
2020-07-07 14:55 ` Liang, Kan
2020-07-07 14:50 ` Liang, Kan
1 sibling, 1 reply; 10+ messages in thread
From: Peter Zijlstra @ 2020-07-07 14:44 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 1829 bytes --]
On Tue, Jul 07, 2020 at 08:51:10AM -0500, 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 seems 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[];
> };
>
Can't.. this is hardware provided layout. The thing is that the array
size is variable, so [0] or [] would be correct.
That said, Kan, I suppose we can simply remove struct pebs_lbr and use
struct lbr_entry * directly. There is very little actual point in having
struct pebs_lbr.
I'll frob the patches and push out again.
^ permalink raw reply [flat|nested] 10+ messages in thread
* 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
2020-07-07 13:51 ` Gustavo A. R. Silva
2020-07-07 14:44 ` Peter Zijlstra
@ 2020-07-07 14:50 ` Liang, Kan
2020-07-07 15:05 ` Gustavo A. R. Silva
1 sibling, 1 reply; 10+ messages in thread
From: Liang, Kan @ 2020-07-07 14:50 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4358 bytes --]
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 seems 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
which 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 = (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 = next_record;
int num_lbr = ((format_size >> PEBS_DATACFG_LBR_SHIFT)
& 0xff) + 1;
next_record = 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 = &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 = 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 = 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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* 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
2020-07-07 14:44 ` Peter Zijlstra
@ 2020-07-07 14:55 ` Liang, Kan
0 siblings, 0 replies; 10+ messages in thread
From: Liang, Kan @ 2020-07-07 14:55 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 1977 bytes --]
On 7/7/2020 10:44 AM, Peter Zijlstra wrote:
> On Tue, Jul 07, 2020 at 08:51:10AM -0500, 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 seems 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[];
>> };
>>
>
> Can't.. this is hardware provided layout. The thing is that the array
> size is variable, so [0] or [] would be correct.
>
> That said, Kan, I suppose we can simply remove struct pebs_lbr and use
> struct lbr_entry * directly. There is very little actual point in having
> struct pebs_lbr.
>
Yes, I will do more tests with the patch I just sent in another thread.
> I'll frob the patches and push out again.
>
Thanks.
Kan
^ permalink raw reply [flat|nested] 10+ messages in thread
* 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
2020-07-07 14:50 ` Liang, Kan
@ 2020-07-07 15:05 ` Gustavo A. R. Silva
2020-07-07 15:24 ` Liang, Kan
0 siblings, 1 reply; 10+ messages in thread
From: Gustavo A. R. Silva @ 2020-07-07 15:05 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4855 bytes --]
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 seems 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 which
> 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 = (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 = next_record;
^^^^^^
Why don't you just directly use lbr_entry here instead of pebs_lbr and avoid
the cast below...
> int num_lbr = ((format_size >> PEBS_DATACFG_LBR_SHIFT)
> & 0xff) + 1;
> next_record = 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 = &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 = 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 = 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* 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
2020-07-07 15:05 ` Gustavo A. R. Silva
@ 2020-07-07 15:24 ` Liang, Kan
2020-07-07 17:09 ` Peter Zijlstra
0 siblings, 1 reply; 10+ messages in thread
From: Liang, Kan @ 2020-07-07 15:24 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 2990 bytes --]
On 7/7/2020 11:05 AM, Gustavo A. R. Silva wrote:
> 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 seems 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 which
>> 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 = (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 = next_record;
>
> ^^^^^^
> Why don't you just directly use lbr_entry here instead of pebs_lbr and avoid
> the cast below...
Yes, we can directly use the lbr_entry here.
I think Peter just update the branch which also directly use the
lbr_entry here. I will pull the latest code and do more test for the
change of dropping the struct pebs_lbr.
Thanks
Kan
^ permalink raw reply [flat|nested] 10+ messages in thread
* 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
2020-07-07 15:24 ` Liang, Kan
@ 2020-07-07 17:09 ` Peter Zijlstra
0 siblings, 0 replies; 10+ messages in thread
From: Peter Zijlstra @ 2020-07-07 17:09 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 376 bytes --]
On Tue, Jul 07, 2020 at 11:24:59AM -0400, Liang, Kan wrote:
> I think Peter just update the branch which also directly use the lbr_entry
> here. I will pull the latest code and do more test for the change of
> dropping the struct pebs_lbr.
Yes, if you look at:
git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git perf/core
pebs_lbr should be gone now.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-07-07 17:09 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-06 19:39 [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 kernel test robot
2020-07-07 1:08 ` Liang, Kan
2020-07-07 10:24 ` Peter Zijlstra
2020-07-07 13:51 ` Gustavo A. R. Silva
2020-07-07 14:44 ` Peter Zijlstra
2020-07-07 14:55 ` Liang, Kan
2020-07-07 14:50 ` Liang, Kan
2020-07-07 15:05 ` Gustavo A. R. Silva
2020-07-07 15:24 ` Liang, Kan
2020-07-07 17:09 ` Peter Zijlstra
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.