lkml.org 
[lkml]   [2019]   [Aug]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC][PATCH 01/13] sched/deadline: Impose global limits on sched_attr::sched_period
Hi Peter,

This is indeed an important missing piece.

I think it would be worth having some simple checks, e.g.:
- period_min <= period_max;
- period_min >= (1ULL << DL_SCALE).

To be even more picky, I'm wondering if it would be a good practice to
fail the write operation if there are already dl-tasks in the system
that violate the new constraint. I'm afraid there is no cheap way of
achieving such check, so, I think we can skip this last tricky thing for
now.

Thanks,
Alessio

On Fri, Jul 26, 2019 at 04:54:10PM +0200, Peter Zijlstra wrote:
>
> Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
> Cc: Luca Abeni <luca.abeni@santannapisa.it>
> Cc: Juri Lelli <juri.lelli@redhat.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> include/linux/sched/sysctl.h | 3 +++
> kernel/sched/deadline.c | 23 +++++++++++++++++++++--
> kernel/sysctl.c | 14 ++++++++++++++
> 3 files changed, 38 insertions(+), 2 deletions(-)
>
> --- a/include/linux/sched/sysctl.h
> +++ b/include/linux/sched/sysctl.h
> @@ -56,6 +56,9 @@ int sched_proc_update_handler(struct ctl
> extern unsigned int sysctl_sched_rt_period;
> extern int sysctl_sched_rt_runtime;
>
> +extern unsigned int sysctl_sched_dl_period_max;
> +extern unsigned int sysctl_sched_dl_period_min;
> +
> #ifdef CONFIG_UCLAMP_TASK
> extern unsigned int sysctl_sched_uclamp_util_min;
> extern unsigned int sysctl_sched_uclamp_util_max;
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -2597,6 +2597,14 @@ void __getparam_dl(struct task_struct *p
> }
>
> /*
> + * Default limits for DL period; on the top end we guard against small util
> + * tasks still getting rediculous long effective runtimes, on the bottom end we
> + * guard against timer DoS.
> + */
> +unsigned int sysctl_sched_dl_period_max = 1 << 22; /* ~4 seconds */
> +unsigned int sysctl_sched_dl_period_min = 100; /* 100 us */
> +
> +/*
> * This function validates the new parameters of a -deadline task.
> * We ask for the deadline not being zero, and greater or equal
> * than the runtime, as well as the period of being zero or
> @@ -2608,6 +2616,8 @@ void __getparam_dl(struct task_struct *p
> */
> bool __checkparam_dl(const struct sched_attr *attr)
> {
> + u64 period, max, min;
> +
> /* special dl tasks don't actually use any parameter */
> if (attr->sched_flags & SCHED_FLAG_SUGOV)
> return true;
> @@ -2631,12 +2641,21 @@ bool __checkparam_dl(const struct sched_
> attr->sched_period & (1ULL << 63))
> return false;
>
> + period = attr->sched_period;
> + if (!period)
> + period = attr->sched_deadline;
> +
> /* runtime <= deadline <= period (if period != 0) */
> - if ((attr->sched_period != 0 &&
> - attr->sched_period < attr->sched_deadline) ||
> + if (period < attr->sched_deadline ||
> attr->sched_deadline < attr->sched_runtime)
> return false;
>
> + max = (u64)READ_ONCE(sysctl_sched_dl_period_max) * NSEC_PER_USEC;
> + min = (u64)READ_ONCE(sysctl_sched_dl_period_min) * NSEC_PER_USEC;
> +
> + if (period < min || period > max)
> + return false;
> +
> return true;
> }
>
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -443,6 +443,20 @@ static struct ctl_table kern_table[] = {
> .proc_handler = sched_rt_handler,
> },
> {
> + .procname = "sched_deadline_period_max_us",
> + .data = &sysctl_sched_dl_period_max,
> + .maxlen = sizeof(unsigned int),
> + .mode = 0644,
> + .proc_handler = proc_dointvec,
> + },
> + {
> + .procname = "sched_deadline_period_min_us",
> + .data = &sysctl_sched_dl_period_min,
> + .maxlen = sizeof(unsigned int),
> + .mode = 0644,
> + .proc_handler = proc_dointvec,
> + },
> + {
> .procname = "sched_rr_timeslice_ms",
> .data = &sysctl_sched_rr_timeslice,
> .maxlen = sizeof(int),
>
>

\
 
 \ /
  Last update: 2019-08-02 19:22    [W:0.200 / U:9.696 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site