On Tue, Sep 10, 2024 at 09:00:43AM -0400, Theodore Ts'o wrote: > On Tue, Sep 10, 2024 at 12:48:16PM +0200, Mateusz Guzik wrote: > > May I suggest adding a compile-time assert on the size? While it may be > > growing it will be unavoidable at some point, it at least wont happen > > unintentionally. > > That should be fine for this structure since everything is defined in > terms of types that should be fixed across architectures, and they > aren't using any types that might change depending on the kernel > config, but as a general matter, we should be a bit careful when > rearranging structrues to avoid holes and to keep things on the same > cache line. > > I recently had a patch submission which was rearranging structure > order for an ext4 data structure, and what worked for the patch > submitter didn't work for me, because of differences between kernel > configs and/or architecture types. > > So it's been on my todo list to do a sanity check of various ext4 > structuers, but to do so checking a number of different architectures > and common production kernel configs (I don't really care if enabling > lockdep results in more holes, because performance is going to be > impacted way more for reasons other than cache lines :-). > While I agree all bets are off for an arbitrarily b0rked kernel, a lot can be done and for more than structs of constant sizes like the one at hand. General note is that things are definitely oversized, with semi-arbitrary field placement and most notably avoidably go past a magic threshold like a multiply of 64. Cache misses aside this also results in memory waste in the allocator, which is my primary concern here. If people did sweeps over structs in code they maintain (and code which is not maintained at all) that would be great (tm), realistically wont happen for vast majority of the kernel. Even so, for heavily used stuff interested maintainers should be able to assert that some baseline does not exceed a certain size -- there is a huge overlap in *important* distro configs. Perhaps configs used by the oe-lkp folk would be fine? Bonus points if relative field placement is also checked for false-sharing reduction. So for example *some* stuff could be always statically asserted, like the size of the struct above and many others. Other stuff could be conditional on lockdep or whatever other debug bloater not being enabled. Stuff of importance which is too messy to be treated this way can have the check be made on demand -- interested maintainers would compile with "make CHECK_STRUCT_SIZES=1" based on sensible(tm) config and get the info, while random users messing with their configs remain unaffected. If there is a random-ass distro with a config which suffers a size problem for a given struct they can find out thanks to optional size tests and try to do something about. As is nobody knows squat unless they explicitly look at stuff one by one. I did an exercise of the sort elsewhere and managed to shrink quite a few 136 byters back to 128 etc. I have some wip here and there, but I'm not signing up for any such work. I would argue everyone maintaining a subsystem should be able to sort this out over time, if they have interest. > Hmm, maybe fodder for a GSOC or intern project would be creating some > kind of automation to check for optimal structure layouts across > multiple configs/architectures? > I'm going to stop ranting here. I do think what I outlined above is easily doable over time and is a nice to have before anyone even attempts this one.
