On 2020/5/13 6:03, Luis Chamberlain wrote:
On Tue, May 12, 2020 at 12:40:55PM -0500, Eric W. Biederman wrote:
Luis Chamberlain <mcgrof@xxxxxxxxxx> writes:
On Tue, May 12, 2020 at 06:52:35AM -0500, Eric W. Biederman wrote:
Luis Chamberlain <mcgrof@xxxxxxxxxx> writes:
+static struct ctl_table fs_base_table[] = {
+ {
+ .procname = "fs",
+ .mode = 0555,
+ .child = fs_table,
+ },
+ { }
+};
^^^^^^^^^^^^^^^^^^^^^^^^ You don't need this at all.
+static int __init fs_procsys_init(void)
+{
+ struct ctl_table_header *hdr;
+
+ hdr = register_sysctl_table(fs_base_table);
^^^^^^^^^^^^^^^^^^^^^ Please use register_sysctl instead.
AKA
hdr = register_sysctl("fs", fs_table);
Ah, much cleaner thanks!
It is my hope you we can get rid of register_sysctl_table one of these
days. It was the original interface but today it is just a
compatibility wrapper.
I unfortunately ran out of steam last time before I finished converting
everything over.
Let's give it one more go. I'll start with the fs stuff.
Luis
.
If we register each feature in its own feature code file using
register() to register the sysctl interface. To avoid merge conflicts
when different features modify sysctl.c at the same time.
that is, try to Avoid mixing code with multiple features in the same
code file.
For example, the multiple file interfaces defined in sysctl.c by the
hung_task feature can be moved to hung_task.c.
Perhaps later, without centralized sysctl.c ?
Is this better?
Thanks
Xiaoming Ni
---
include/linux/sched/sysctl.h | 8 +----
kernel/hung_task.c | 78
+++++++++++++++++++++++++++++++++++++++++++-
kernel/sysctl.c | 50 ----------------------------
3 files changed, 78 insertions(+), 58 deletions(-)
diff --git a/include/linux/sched/sysctl.h b/include/linux/sched/sysctl.h
index d4f6215..bb4e0d3 100644
--- a/include/linux/sched/sysctl.h
+++ b/include/linux/sched/sysctl.h
@@ -7,14 +7,8 @@
struct ctl_table;
#ifdef CONFIG_DETECT_HUNG_TASK
-extern int sysctl_hung_task_check_count;
-extern unsigned int sysctl_hung_task_panic;
+/* used for block/ */
extern unsigned long sysctl_hung_task_timeout_secs;
-extern unsigned long sysctl_hung_task_check_interval_secs;
-extern int sysctl_hung_task_warnings;
-extern int proc_dohung_task_timeout_secs(struct ctl_table *table, int
write,
- void __user *buffer,
- size_t *lenp, loff_t *ppos);
#else
/* Avoid need for ifdefs elsewhere in the code */
enum { sysctl_hung_task_timeout_secs = 0 };
diff --git a/kernel/hung_task.c b/kernel/hung_task.c
index 14a625c..53589f2 100644
--- a/kernel/hung_task.c
+++ b/kernel/hung_task.c
@@ -20,10 +20,10 @@
#include <linux/utsname.h>
#include <linux/sched/signal.h>
#include <linux/sched/debug.h>
+#include <linux/kmemleak.h>
#include <linux/sched/sysctl.h>
#include <trace/events/sched.h>
-
/*
* The number of tasks checked:
*/
@@ -296,8 +296,84 @@ static int watchdog(void *dummy)
return 0;
}
+/*
+ * This is needed for proc_doulongvec_minmax of
sysctl_hung_task_timeout_secs
+ * and hung_task_check_interval_secs
+ */
+static unsigned long hung_task_timeout_max = (LONG_MAX / HZ);
+static int __maybe_unused neg_one = -1;
+static struct ctl_table hung_task_sysctls[] = {
+ {
+ .procname = "hung_task_panic",
+ .data = &sysctl_hung_task_panic,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = SYSCTL_ZERO,
+ .extra2 = SYSCTL_ONE,
+ },
+ {
+ .procname = "hung_task_check_count",
+ .data = &sysctl_hung_task_check_count,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = SYSCTL_ZERO,
+ },
+ {
+ .procname = "hung_task_timeout_secs",
+ .data = &sysctl_hung_task_timeout_secs,
+ .maxlen = sizeof(unsigned long),
+ .mode = 0644,
+ .proc_handler = proc_dohung_task_timeout_secs,
+ .extra2 = &hung_task_timeout_max,
+ },
+ {
+ .procname = "hung_task_check_interval_secs",
+ .data = &sysctl_hung_task_check_interval_secs,
+ .maxlen = sizeof(unsigned long),
+ .mode = 0644,
+ .proc_handler = proc_dohung_task_timeout_secs,
+ .extra2 = &hung_task_timeout_max,
+ },
+ {
+ .procname = "hung_task_warnings",
+ .data = &sysctl_hung_task_warnings,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = &neg_one,
+ },
+ {}
+};
+
+/* get /proc/sys/kernel root */
+static struct ctl_table sysctls_root[] = {
+ {
+ .procname = "kernel",
+ .mode = 0555,
+ .child = hung_task_sysctls,
+ },
+ {}
+};
+
+static int __init hung_task_sysctl_init(void)
+{
+ struct ctl_table_header *srt = register_sysctl_table(sysctls_root);
+
+ if (!srt)
+ return -ENOMEM;
+ kmemleak_not_leak(srt);
+ return 0;
+}
+
static int __init hung_task_init(void)
{
+ int ret = hung_task_sysctl_init();
+
+ if (ret != 0)
+ return ret;
+
atomic_notifier_chain_register(&panic_notifier_list, &panic_block);
/* Disable hung task detector on suspend */
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 8a176d8..45a1153 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -149,13 +149,6 @@
static int ngroups_max = NGROUPS_MAX;
static const int cap_last_cap = CAP_LAST_CAP;
-/*
- * This is needed for proc_doulongvec_minmax of
sysctl_hung_task_timeout_secs
- * and hung_task_check_interval_secs
- */
-#ifdef CONFIG_DETECT_HUNG_TASK
-static unsigned long hung_task_timeout_max = (LONG_MAX/HZ);
-#endif
#ifdef CONFIG_INOTIFY_USER
#include <linux/inotify.h>
@@ -1085,49 +1078,6 @@ static int sysrq_sysctl_handler(struct ctl_table
*table, int write,
.proc_handler = proc_dointvec,
},
#endif
-#ifdef CONFIG_DETECT_HUNG_TASK
- {
- .procname = "hung_task_panic",
- .data = &sysctl_hung_task_panic,
- .maxlen = sizeof(int),
- .mode = 0644,
- .proc_handler = proc_dointvec_minmax,
- .extra1 = SYSCTL_ZERO,
- .extra2 = SYSCTL_ONE,
- },
- {
- .procname = "hung_task_check_count",
- .data = &sysctl_hung_task_check_count,
- .maxlen = sizeof(int),
- .mode = 0644,
- .proc_handler = proc_dointvec_minmax,
- .extra1 = SYSCTL_ZERO,
- },
- {
- .procname = "hung_task_timeout_secs",
- .data = &sysctl_hung_task_timeout_secs,
- .maxlen = sizeof(unsigned long),
- .mode = 0644,
- .proc_handler = proc_dohung_task_timeout_secs,
- .extra2 = &hung_task_timeout_max,
- },
- {
- .procname = "hung_task_check_interval_secs",
- .data = &sysctl_hung_task_check_interval_secs,
- .maxlen = sizeof(unsigned long),
- .mode = 0644,
- .proc_handler = proc_dohung_task_timeout_secs,
- .extra2 = &hung_task_timeout_max,
- },
- {
- .procname = "hung_task_warnings",
- .data = &sysctl_hung_task_warnings,
- .maxlen = sizeof(int),
- .mode = 0644,
- .proc_handler = proc_dointvec_minmax,
- .extra1 = &neg_one,
- },
-#endif
#ifdef CONFIG_RT_MUTEXES
{
.procname = "max_lock_depth",
--
1.8.5.6
