Thread (11 messages) 11 messages, 2 authors, 2021-11-29

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help