Messages in this thread | | | Date | Tue, 12 May 2009 10:54:02 +0200 | From | Ingo Molnar <> | Subject | Re: [RFC PATCH 2/2] kernel/sched.c: VLA in middle of struct |
| |
* Jeff Garzik <jeff@garzik.org> wrote:
> 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).
feel free to send patches for this - i dont have those build modes to test that it's sufficient. I'd suggest to go the simplest path: remove all the vla aliasing tricks: just make struct sched_domain use a plain struct cpumask and eliminate static_sched_domain altogether.
The memory overhead is marginal as most of our sched domains are static allocated (and full size) anyway.
Ingo
| |