Messages in this thread | | | Date | Fri, 26 Nov 2021 13:51:38 +0100 | From | Petr Mladek <> | Subject | Re: [PATCH v2 5/8] printk: move printk sysctl to printk/sysctl.c |
| |
On Wed 2021-11-24 15:14:32, Luis Chamberlain wrote: > From: Xiaoming Ni <nixiaoming@huawei.com> > > 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
> 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
| |