Re: [PATCH v2 5/8] printk: move printk sysctl to printk/sysctl.c
From: Petr Mladek <pmladek@suse.com>
Date: 2021-11-26 12:53:47
Also in:
linux-fsdevel, lkml
On Wed 2021-11-24 15:14:32, Luis Chamberlain wrote:
quoted hunk ↗ jump to hunk
From: Xiaoming Ni <redacted> The kernel/sysctl.c is a kitchen sink where everyone leaves their dirty dishes, this makes it very difficult to maintain. 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. So move printk sysctl from kernel/sysctl.c to kernel/printk/sysctl.c. Use register_sysctl() to register the sysctl interface.diff --git a/kernel/printk/Makefile b/kernel/printk/Makefile index d118739874c0..f5b388e810b9 100644 --- a/kernel/printk/Makefile +++ b/kernel/printk/Makefile@@ -2,5 +2,8 @@ obj-y = printk.o obj-$(CONFIG_PRINTK) += printk_safe.o obj-$(CONFIG_A11Y_BRAILLE_CONSOLE) += braille.o -obj-$(CONFIG_PRINTK) += printk_ringbuffer.o obj-$(CONFIG_PRINTK_INDEX) += index.o + +obj-$(CONFIG_PRINTK) += printk_support.o +printk_support-y := printk_ringbuffer.o +printk_support-$(CONFIG_SYSCTL) += sysctl.o
I have never seen this trick. It looks like a dirty hack ;-) Anyway, I do not see it described in the documentation. I wonder if it works only by chance. IMHO, a cleaner solution would be to add the following into init/Kconfig: config BUILD_PRINTK_SYSCTL bool default (PRINTK && SYSCTL) and then use: obj-$(CONFIG_BUILD_PRINTK_SYSCTL) += sysctl.o
quoted hunk ↗ jump to hunk
diff --git a/kernel/printk/sysctl.c b/kernel/printk/sysctl.c new file mode 100644 index 000000000000..653ae04aab7f --- /dev/null +++ b/kernel/printk/sysctl.c@@ -0,0 +1,85 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * sysctl.c: General linux system control interface + */ + +#include <linux/sysctl.h> +#include <linux/printk.h> +#include <linux/capability.h> +#include <linux/ratelimit.h> +#include "internal.h" + +static const int ten_thousand = 10000;
The patch should also remove the variable in kernel/sysctl.c. Otherwise, it looks like a really nice clean up. Best Regards, Petr