Messages in this thread | | | Date | Mon, 11 May 2009 22:24:34 -0400 | From | Jeff Garzik <> | Subject | Re: [RFC PATCH 2/2] kernel/sched.c: VLA in middle of struct |
| |
Ingo Molnar wrote: >>>>> That cpumask[] should probably be cpumask[0], to document the >>>>> aliasing to ->span and ->cpus properly. >>>> If the comment wasn't sufficient documentation, I don't think that >>>> would help :( >>> It's a visual helper: it matches up with how we do these 'zero size >>> array means dynamic structure continuation' tricks generally. >>> >>> I first mis-parsed the code for a second when seeing cpumask[]. >>> cpumask[0] stands out like a sore thumb. And we dont read comments >>> anyway ;-) >>> >>> Jeff, i suspect you found this because you are working on something >>> rather interesting? :) If yes, would it help your project if we did >>> the cpumask[0] cleanup and pushed it upstream immediately? >> I think cpumask[0] would be more clear and consistent with the >> rest of the kernel. >> >> But unfortunately for the twin projects of (a) static analysis and >> checking with 'sparse', and (b) compiling under another compiler, >> VLA-in-middle-of-struct is a killer in either case. > > even if at the end of the struct?
Putting the VLA at the end of the struct would be a huge help, yes.
For example, struct sched_group and struct sched_domain are OK as-is (though "[0]" would be preferred).
It is the definition of struct static_sched_group and struct static_sched_domain that creates the problem, because with the bitmap following cpumask[] and span[], the VLA is no longer at the end of the struct.
VLA-in-the-middle raises the complexity required of the compiler quite a bit. As a result, VLA-in-middle is not implemented in sparse or clang (LLVM's C front-end and static analyzer).
Jeff
| |