Messages in this thread | | | Date | Tue, 07 Jun 2022 01:25:48 +0000 | From | "Yajun Deng" <> | Subject | Re: [PATCH v4] sched/rt: fix the case where sched_rt_period_us is negative |
| |
Ping.
May 18, 2022 12:08 AM, "Valentin Schneider" <vschneid@redhat.com> wrote:
> 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>
| |