| Message ID | 20220210060831.26689-1-nizhen@uniontech.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | sched: move rr_timeslice sysctls to rt.c | expand |
On Thu, Feb 10, 2022 at 02:08:31PM +0800, Zhen Ni wrote: > move rr_timeslice sysctls to rt.c and use the new > register_sysctl_init() to register the sysctl interface. > > Signed-off-by: Zhen Ni <nizhen@uniontech.com> OK, I've had it with this nonsense. Can you *please* redo all of sched such that: - In the Subject:, the first letter after the subsystem prefix (sched:) is capitalized. - the lot actually applies to tip/sched/core (so far not a single one of these patches applied without needing -- mostly trivial -- fixups). - Do obvious cleanups.. see below. - Don't have more than a single *sysctl_init() per .c file. - It's a full series that does all instead of random little patches that conflict with one another when applied out of turn. > --- > include/linux/sched/sysctl.h | 3 --- > kernel/sched/rt.c | 28 ++++++++++++++++++++++++++-- > kernel/sysctl.c | 7 ------- > 3 files changed, 26 insertions(+), 12 deletions(-) > > diff --git a/include/linux/sched/sysctl.h b/include/linux/sched/sysctl.h > index d416d8f45186..f6466040883c 100644 > --- a/include/linux/sched/sysctl.h > +++ b/include/linux/sched/sysctl.h > @@ -45,11 +45,8 @@ extern unsigned int sysctl_sched_uclamp_util_min_rt_default; > extern unsigned int sysctl_sched_autogroup_enabled; > #endif > > -extern int sysctl_sched_rr_timeslice; > extern int sched_rr_timeslice; Why leave sched_rr_timeslice here? It doesn't belong here. > +#ifdef CONFIG_SYSCTL > +static struct ctl_table sched_rr_sysctls[] = { > + { > + .procname = "sched_rr_timeslice_ms", > + .data = &sysctl_sched_rr_timeslice, > + .maxlen = sizeof(int), > + .mode = 0644, > + .proc_handler = sched_rr_handler, > + }, > + {} > +}; > + > +static void __init sched_rr_sysctl_init(void) > +{ > + register_sysctl_init("kernel", sched_rr_sysctls); > +} > +#else > +#define sched_rr_sysctl_init() do { } while (0) > +#endif > + > static int do_sched_rt_period_timer(struct rt_bandwidth *rt_b, int overrun); > > struct rt_bandwidth def_rt_bandwidth; > @@ -2471,6 +2494,7 @@ void __init init_sched_rt_class(void) > zalloc_cpumask_var_node(&per_cpu(local_cpu_mask, i), > GFP_KERNEL, cpu_to_node(i)); > } > + sched_rr_sysctl_init(); > } When I combine this with: patches/zhen_ni-sched-move_rt_period_runtime_sysctls_to_rt_c.patch That ends up as: @@ -2471,6 +2535,8 @@ void __init init_sched_rt_class(void) zalloc_cpumask_var_node(&per_cpu(local_cpu_mask, i), GFP_KERNEL, cpu_to_node(i)); } + sched_rt_sysctl_init(); + sched_rr_sysctl_init(); } #endif /* CONFIG_SMP */ Like srsly? So I've dropped the whole lot I had: patches/zhen_ni-sched-move_energy_aware_sysctls_to_topology_c.patch patches/zhen_ni-sched-move_cfs_bandwidth_slice_sysctls_to_fair_c.patch patches/zhen_ni-sched-move_uclamp_util_sysctls_to_core_c.patch patches/zhen_ni-sched-move_schedstats_sysctls_to_core_c.patch patches/zhen_ni-sched-move_deadline_period_sysctls_to_deadline_c.patch patches/zhen_ni-sched-move_rt_period_runtime_sysctls_to_rt_c.patch patches/zhen_ni-sched-move_rr_timeslice_sysctls_to_rt_c.patch And I expect a single coherent series or I'll forgo all this.
Cc'ing Andrew for coordination on merging these sorts of patches. On Fri, Feb 11, 2022 at 12:51:14PM +0100, Peter Zijlstra wrote: > So I've dropped the whole lot I had: > > patches/zhen_ni-sched-move_energy_aware_sysctls_to_topology_c.patch > patches/zhen_ni-sched-move_cfs_bandwidth_slice_sysctls_to_fair_c.patch > patches/zhen_ni-sched-move_uclamp_util_sysctls_to_core_c.patch > patches/zhen_ni-sched-move_schedstats_sysctls_to_core_c.patch > patches/zhen_ni-sched-move_deadline_period_sysctls_to_deadline_c.patch > patches/zhen_ni-sched-move_rt_period_runtime_sysctls_to_rt_c.patch > patches/zhen_ni-sched-move_rr_timeslice_sysctls_to_rt_c.patch > > And I expect a single coherent series or I'll forgo all this. I suspect Zhen will follow up accordingly. So Andrew had merged tons of initial cleanups for kernel/sysctl.c. Now that some of the initial grunt work and sysctls for fs are out of kernel/sysctl.c, I'm seeing the next target seems to be the scheduler. I don't think these *need* to go through Andrew's tree since the fs changes are already on Linus' tree. So figured I'd drop this note to just remind ourselves that if accumulate a few of these patches for one subsystem there is a greater risk of a conflict later with another subsystem doing some similar cleanup. So for the next release I think it makes sense to keep this localized somehow if we can. Maybe we just deal with these on Peter's tree? Luis
diff --git a/include/linux/sched/sysctl.h b/include/linux/sched/sysctl.h index d416d8f45186..f6466040883c 100644 --- a/include/linux/sched/sysctl.h +++ b/include/linux/sched/sysctl.h @@ -45,11 +45,8 @@ extern unsigned int sysctl_sched_uclamp_util_min_rt_default; extern unsigned int sysctl_sched_autogroup_enabled; #endif -extern int sysctl_sched_rr_timeslice; extern int sched_rr_timeslice; -int sched_rr_handler(struct ctl_table *table, int write, void *buffer, - size_t *lenp, loff_t *ppos); int sched_rt_handler(struct ctl_table *table, int write, void *buffer, size_t *lenp, loff_t *ppos); int sysctl_sched_uclamp_handler(struct ctl_table *table, int write, diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c index 7b4f4fbbb404..e8316e0307b0 100644 --- a/kernel/sched/rt.c +++ b/kernel/sched/rt.c @@ -8,10 +8,33 @@ #include "pelt.h" int sched_rr_timeslice = RR_TIMESLICE; -int sysctl_sched_rr_timeslice = (MSEC_PER_SEC / HZ) * RR_TIMESLICE; +static int sysctl_sched_rr_timeslice = (MSEC_PER_SEC / HZ) * RR_TIMESLICE; /* More than 4 hours if BW_SHIFT equals 20. */ static const u64 max_rt_runtime = MAX_BW; +static int sched_rr_handler(struct ctl_table *table, int write, void *buffer, + size_t *lenp, loff_t *ppos); + +#ifdef CONFIG_SYSCTL +static struct ctl_table sched_rr_sysctls[] = { + { + .procname = "sched_rr_timeslice_ms", + .data = &sysctl_sched_rr_timeslice, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = sched_rr_handler, + }, + {} +}; + +static void __init sched_rr_sysctl_init(void) +{ + register_sysctl_init("kernel", sched_rr_sysctls); +} +#else +#define sched_rr_sysctl_init() do { } while (0) +#endif + static int do_sched_rt_period_timer(struct rt_bandwidth *rt_b, int overrun); struct rt_bandwidth def_rt_bandwidth; @@ -2471,6 +2494,7 @@ void __init init_sched_rt_class(void) zalloc_cpumask_var_node(&per_cpu(local_cpu_mask, i), GFP_KERNEL, cpu_to_node(i)); } + sched_rr_sysctl_init(); } #endif /* CONFIG_SMP */ @@ -2967,7 +2991,7 @@ int sched_rt_handler(struct ctl_table *table, int write, void *buffer, return ret; } -int sched_rr_handler(struct ctl_table *table, int write, void *buffer, +static int sched_rr_handler(struct ctl_table *table, int write, void *buffer, size_t *lenp, loff_t *ppos) { int ret; diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 981a1902d7a4..d0c45bf6801d 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -1720,13 +1720,6 @@ static struct ctl_table kern_table[] = { .mode = 0644, .proc_handler = proc_dointvec, }, - { - .procname = "sched_rr_timeslice_ms", - .data = &sysctl_sched_rr_timeslice, - .maxlen = sizeof(int), - .mode = 0644, - .proc_handler = sched_rr_handler, - }, #ifdef CONFIG_UCLAMP_TASK { .procname = "sched_util_clamp_min",
move rr_timeslice sysctls to rt.c and use the new register_sysctl_init() to register the sysctl interface. Signed-off-by: Zhen Ni <nizhen@uniontech.com> --- include/linux/sched/sysctl.h | 3 --- kernel/sched/rt.c | 28 ++++++++++++++++++++++++++-- kernel/sysctl.c | 7 ------- 3 files changed, 26 insertions(+), 12 deletions(-)