Re: [PATCH v5] kernel/time: move timer sysctls to its own file
From: Thomas Gleixner
Date: Wed Feb 02 2022 - 19:21:57 EST
Tangmeng,
On Mon, Jan 31 2022 at 18:22, tangmeng wrote:
> kernel/sysctl.c is a kitchen sink where everyone leaves their dirty
> dishes, this makes it very difficult to maintain.
Sorry. That's just a lame argument. What exactly is hard to maintain on
that file? A large table of ifdeffed sysctl entries which changes once
in a blue moon is hardly a maintenance problem.
Aside of that, sysctl.c is a very conveniant way to look up the zoo of
sysctls which you now spread out all over the source tree.
So you really need to come up with a technical and sensical explanation
for this change.
> To help with this maintenance let's start by moving sysctls to places
> where they actually belong. The proc sysctl maintainers do not want to
> know what sysctl knobs you wish to add for your own piece of code, we
> just care about the core logic.
In other words, invite everyone to add random sysctls as they see fit
w/o a central review authority. That's not an improvement at all. Quite
the contrary.
That aside, I'm tired of this because this is now at V5 and you still
failed to fix the fallout reported by the 0-day infrastructure vs. this
part of the patch:
> +static int __init timer_sysctl_init(void)
> +{
> + register_sysctl_init("kernel", timer_sysctl);
> + return 0;
> +}
kernel/time/timer.c: In function 'timer_sysctl_init':
>> kernel/time/timer.c:284:9: error: implicit declaration of function 'register_sysctl_init'; did you mean 'timer_sysctl_init'? [-Werror=implicit-function-declaration]
284 | register_sysctl_init("kernel", timer_sysctl);
| ^~~~~~~~~~~~~~~~~~~~
It's pretty damned obvious why this fails to compile and the 0-day
reports have all the information you need to reproduce and address this,
but you prefer to ignore it and just resend yet another incarnation.
Feel free to ignore these reports, but then please do not be surprised
when I ignore your patches. Our development process is well documented
and it's not subject to your personal interpretation.
Thanks,
tglx