Messages in this thread | | | From | Valentin Schneider <> | Subject | Re: [PATCH v4] sched/rt: fix the case where sched_rt_period_us is negative | Date | Tue, 17 May 2022 17:08:23 +0100 |
| |
On 17/05/22 14:29, Yajun Deng wrote: > The proc_dointvec() is for integer, but sysctl_sched_rt_period is a > unsigned integer, proc_dointvec() would convert negative number into > positive number. So both proc_dointvec() and sched_rt_global_validate() > aren't return error even if we set a negative number. > > Use proc_dointvec_minmax() instead of proc_dointvec() and use extra1 > limit the minimum value for sched_rt_period_us/sched_rt_runtime_us. > > Make sysctl_sched_rt_period integer to match proc_dointvec_minmax(). >
How about:
While sysctl_sched_rt_runtime is a signed integer and sysctl_sched_rt_period is unsigned, both are handled in sched_rt_handler() via proc_dointvec(), so negative inputs can be fed into sysctl_sched_rt_period. However, per sched-rt-group.rst:
* sched_rt_period_us takes values from 1 to INT_MAX. * sched_rt_runtime_us takes values from -1 to (INT_MAX - 1).
Use proc_dointvec_minmax() instead of proc_dointvec() and use the .extra1 parameter to enforce a minimum value.
Make sysctl_sched_rt_period a signed integer as this matches the expected upper boundary and the expected type of proc_dointvec_minmax().
> v4: > - Make sysctl_sched_rt_period integer (Valentin Schneider)
Even if v3 was bogus, it's good not to skip it in the version log. Also, the version logs should be after the "---" marker line:
Documentation/process/submitting-patches.rt """ Please put this information **after** the ``---`` line which separates the changelog from the rest of the patch. The version information is not part of the changelog which gets committed to the git tree. It is additional information for the reviewers. If it's placed above the commit tags, it needs manual interaction to remove it. If it is below the separator line, it gets automatically stripped off when applying the patch """
> v2: > - Remove sched_rr_timeslice_ms related changes (Valentin Schneider) > > Fixes: d0b27fa77854 ("sched: rt-group: synchonised bandwidth period") > Signed-off-by: Yajun Deng <yajun.deng@linux.dev>
Reviewed-by: Valentin Schneider <vschneid@redhat.com>
| |