On Thu, Mar 23, 2023 at 05:19:15PM +0100, Vlastimil Babka wrote: > On 3/22/23 09:35, Christoph Hellwig wrote: > > On Wed, Mar 22, 2023 at 10:46:28AM +0800, ye.xingchen@xxxxxxxxxx wrote: > >> From: Minghao Chi <chi.minghao@xxxxxxxxxx> > >> > >> This moves all compaction sysctls to its own file. > > > > So there's a whole lot of these 'move sysctrls to their own file' > > patches, but no actual explanation of why that is desirable. Please > > I think Luis started this initiative, maybe he can provide the canonical > reasoning :) The kernel/sysctl.c is flooded now with commit log entries which describe a proper rationale, however some folks forget to also include similar rationale in new patches. I try to remind folks when I can, thanks for reminding them to continue to do that. That needs to be fixed in this patch. The summary is its hard to coordiante merge conflicts with all the syctls in one place, best to just put them where they are used. There is a small hidden penalty increase in size to the kernel with the sysctls moves today though and one for which Matthew Wilcox has recently asked for us to pause these moves until we can save more memory. The extra memory is caused by the extra empty struct ctl_table added to the end of the new array. The way to avoid that penalty is to deprecate all APIs in sysctl registation which deal with complex array structures. I have some some of that addressed on my sysctl-next tree (and merged on linux-next) but much more work is required to deprecate the older APIs. I was ready to pause the kernel/sysctl.c moves until those APIs are deprecated and we start having sysctl APIs which allow us to not have the empty array at the end. But as I thought about this just now, the use cases that move the sysctls where __init is used could benefit already in size. For this patch there seems to be a savings of 4 bytes: $ ./scripts/bloat-o-meter vmlinux.old vmlinux add/remove: 1/0 grow/shrink: 1/2 up/down: 346/-350 (-4) Function old new delta vm_compaction - 320 +320 kcompactd_init 167 193 +26 proc_dointvec_minmax_warn_RT_change 104 10 -94 vm_table 2112 1856 -256 Total: Before=19287558, After=19287554, chg -0.00% So I don't think we need to pause this move or others where are have savings. Minghao, can you fix the commit log, and explain how you are also saving 4 bytes as per the above bloat-o-meter results? > > explain why we'd want to split code that is closely related, and now > > requires marking symbols non-static just to create a new tiny source > > file. > > Hmm? I can see the opposite, at least in the compaction patch here. Related > code and variables are moved closer together, made static, declarations > removed from headers. It looks like an improvement to me. Glad this helps. Luis
