Re: [PATCH] sched/deadline: overrun could happen in start_hrtick_dl

[xiaofeng.yan]

On 2014/7/8 17:33, Peter Zijlstra wrote:
> On Tue, Jul 08, 2014 at 08:53:27AM +0000, xiaofeng.yan wrote:
> >   static void start_hrtick_dl(struct rq *rq, struct task_struct *p)
> >   {
> > -	s64 delta = p->dl.dl_runtime - p->dl.runtime;
> > -
> > -	if (delta > 10000)
> > -		hrtick_start(rq, p->dl.runtime);
> > +	delta = max_t(s64, 10000LL, delta);
> > +	hrtick_start(rq, delta);
> >   }
> no, no, no. I said to unify the test.

I understand your idea after reading the next patch. This is good solution.
I will test it with your patch.

> ---
>   kernel/sched/core.c     | 9 ++++++++-
>   kernel/sched/deadline.c | 3 +--
>   kernel/sched/fair.c     | 7 -------
>   3 files changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index e1a2f31bb0cb..c7b8a6fbac66 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -444,7 +444,14 @@ static void __hrtick_start(void *arg)
>   void hrtick_start(struct rq *rq, u64 delay)
>   {
>   	struct hrtimer *timer = &rq->hrtick_timer;
> -	ktime_t time = ktime_add_ns(timer->base->get_time(), delay);
> +	ktime_t time;
> +
> +	/*
> +	 * Don't schedule slices shorter than 10000ns, that just
> +	 * doesn't make sense and can cause timer DoS.
> +	 */
> +	delta = max_t(s64, delta, 10000LL);

transfer the argument delta to delay

> +	time = ktime_add_ns(timer->base->get_time(), delay);
>   
>   	hrtimer_set_expires(timer, time);
>   
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index fc4f98b1258f..e1e24eea8061 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -999,8 +999,7 @@ static void start_hrtick_dl(struct rq *rq, struct task_struct *p)
>   {
>   	s64 delta = p->dl.dl_runtime - p->dl.runtime;
>   
> -	if (delta > 10000)
> -		hrtick_start(rq, p->dl.runtime);
> +	hrtick_start(rq, p->dl.runtime);
>   }
>   #endif
>   
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 923fe32db6b3..713c58d2a7b0 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -3901,13 +3901,6 @@ static void hrtick_start_fair(struct rq *rq, struct task_struct *p)
>   			return;
>   		}
>   
> -		/*
> -		 * Don't schedule slices shorter than 10000ns, that just
> -		 * doesn't make sense. Rely on vruntime for fairness.
> -		 */
> -		if (rq->curr != p)
> -			delta = max_t(s64, 10000LL, delta);
> -
>   		hrtick_start(rq, delta);
>   	}
>   }


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/