linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] SCHED_DEADLINE fixes and cleanups
@ 2017-09-07 10:09 luca abeni
  2017-09-07 10:09 ` [PATCH 1/4] sched/sched.h: remove duplicate prototype of __dl_clear_params() luca abeni
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: luca abeni @ 2017-09-07 10:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Zijlstra, Ingo Molnar, Juri Lelli, Steven Rostedt,
	Daniel Bristot de Oliveira, Mathieu Poirier, luca abeni

Hi,

here are some fixes, cleanups and improvements for SCHED_DEADLINE.

Patch 0001 just removes a duplicate declaration from sched/sched.h.

Patch 0002 fixes a bug I introduced some time ago, when removing
the dl_new field from sched_dl_entity in
72f9f3fdc928dc3ecd223e801b32d930b662b6ed.
This is the same patch I already sent some time ago, because at the
end of the discussion it was not clear to me if some changes are needed
or not.

Patch 0003 renames the confusing __dl_clear() function, as suggested
by Peter (actually, this is Peter's patch... I hope I did the SoB thing
correctly)

Patch 0004 saves reduces the size of sched_dl_entity by using bitfields
instead of whole integers for boolean values.


Peter Zijlstra (1):
  sched/deadline: rename __dl_clear() to __dl_sub()

luca abeni (3):
  sched/sched.h: remove duplicate prototype of __dl_clear_params()
  sched/deadline: fix switching to -deadline
  sched/deadline: use C bitfields for the state flags

 include/linux/sched.h   |  8 ++++----
 kernel/sched/deadline.c | 21 +++++++++------------
 kernel/sched/sched.h    |  3 +--
 3 files changed, 14 insertions(+), 18 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/4] sched/sched.h: remove duplicate prototype of __dl_clear_params()
  2017-09-07 10:09 [PATCH 0/4] SCHED_DEADLINE fixes and cleanups luca abeni
@ 2017-09-07 10:09 ` luca abeni
  2017-09-08  8:33   ` Daniel Bristot de Oliveira
  2017-10-10 10:55   ` [tip:sched/core] sched/headers: Remove " tip-bot for luca abeni
  2017-09-07 10:09 ` [PATCH 2/4] sched/deadline: fix switching to -deadline luca abeni
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 13+ messages in thread
From: luca abeni @ 2017-09-07 10:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Zijlstra, Ingo Molnar, Juri Lelli, Steven Rostedt,
	Daniel Bristot de Oliveira, Mathieu Poirier, luca abeni

Signed-off-by: luca abeni <luca.abeni@santannapisa.it>
---
 kernel/sched/sched.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 1043c8b..0b93e4b 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -255,7 +255,6 @@ extern int sched_dl_overflow(struct task_struct *p, int policy,
 extern void __setparam_dl(struct task_struct *p, const struct sched_attr *attr);
 extern void __getparam_dl(struct task_struct *p, struct sched_attr *attr);
 extern bool __checkparam_dl(const struct sched_attr *attr);
-extern void __dl_clear_params(struct task_struct *p);
 extern bool dl_param_changed(struct task_struct *p, const struct sched_attr *attr);
 extern int dl_task_can_attach(struct task_struct *p,
 			      const struct cpumask *cs_cpus_allowed);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/4] sched/deadline: fix switching to -deadline
  2017-09-07 10:09 [PATCH 0/4] SCHED_DEADLINE fixes and cleanups luca abeni
  2017-09-07 10:09 ` [PATCH 1/4] sched/sched.h: remove duplicate prototype of __dl_clear_params() luca abeni
@ 2017-09-07 10:09 ` luca abeni
  2017-10-10 10:56   ` [tip:sched/core] sched/deadline: Fix " tip-bot for Luca Abeni
  2017-09-07 10:09 ` [PATCH 3/4] sched/deadline: rename __dl_clear() to __dl_sub() luca abeni
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: luca abeni @ 2017-09-07 10:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Zijlstra, Ingo Molnar, Juri Lelli, Steven Rostedt,
	Daniel Bristot de Oliveira, Mathieu Poirier, luca abeni

Fix a bug introduced in 72f9f3fdc928dc3ecd223e801b32d930b662b6ed:
after that commit, when switching to -deadline if the scheduling
deadline of a task is in the past then switched_to_dl() calls
setup_new_entity() to properly initialize the scheduling deadline
and runtime.

The problem is that the task is enqueued _before_ having its parameters
initialized by setup_new_entity(), and this can cause problems.
For example, a task with its out-of-date deadline in the past will
potentially be enqueued as the highest priority one; however, its
adjusted deadline may not be the earliest one.

This patch fixes the problem by initializing the task's parameters before
enqueuing it.

Signed-off-by: luca abeni <luca.abeni@santannapisa.it>
Reviewed-by: Daniel Bristot de Oliveira <bristot@redhat.com>
---
 kernel/sched/deadline.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index d05bd94..d7f9e86 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -1376,6 +1376,10 @@ enqueue_dl_entity(struct sched_dl_entity *dl_se,
 		update_dl_entity(dl_se, pi_se);
 	} else if (flags & ENQUEUE_REPLENISH) {
 		replenish_dl_entity(dl_se, pi_se);
+	} else if ((flags & ENQUEUE_RESTORE) &&
+		  dl_time_before(dl_se->deadline,
+				 rq_clock(rq_of_dl_rq(dl_rq_of_se(dl_se))))) {
+		setup_new_dl_entity(dl_se);
 	}
 
 	__enqueue_dl_entity(dl_se);
@@ -2267,13 +2271,6 @@ static void switched_to_dl(struct rq *rq, struct task_struct *p)
 
 		return;
 	}
-	/*
-	 * If p is boosted we already updated its params in
-	 * rt_mutex_setprio()->enqueue_task(..., ENQUEUE_REPLENISH),
-	 * p's deadline being now already after rq_clock(rq).
-	 */
-	if (dl_time_before(p->dl.deadline, rq_clock(rq)))
-		setup_new_dl_entity(&p->dl);
 
 	if (rq->curr != p) {
 #ifdef CONFIG_SMP
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/4] sched/deadline: rename __dl_clear() to __dl_sub()
  2017-09-07 10:09 [PATCH 0/4] SCHED_DEADLINE fixes and cleanups luca abeni
  2017-09-07 10:09 ` [PATCH 1/4] sched/sched.h: remove duplicate prototype of __dl_clear_params() luca abeni
  2017-09-07 10:09 ` [PATCH 2/4] sched/deadline: fix switching to -deadline luca abeni
@ 2017-09-07 10:09 ` luca abeni
  2017-09-08  8:26   ` Daniel Bristot de Oliveira
  2017-10-10 10:56   ` [tip:sched/core] sched/deadline: Rename " tip-bot for Peter Zijlstra
  2017-09-07 10:09 ` [PATCH 4/4] sched/deadline: use C bitfields for the state flags luca abeni
  2017-10-02  8:18 ` [PATCH 0/4] SCHED_DEADLINE fixes and cleanups Peter Zijlstra
  4 siblings, 2 replies; 13+ messages in thread
From: luca abeni @ 2017-09-07 10:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Zijlstra, Ingo Molnar, Juri Lelli, Steven Rostedt,
	Daniel Bristot de Oliveira, Mathieu Poirier, luca abeni

From: Peter Zijlstra <peterz@infradead.org>

__dl_sub() is more meaningful as a name, and is more consistent
with the naming of the dual function (__dl_add()).

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: luca abeni <luca.abeni@santannapisa.it>
---
 kernel/sched/deadline.c | 10 +++++-----
 kernel/sched/sched.h    |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index d7f9e86..bab11dd 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -242,7 +242,7 @@ static void task_non_contending(struct task_struct *p)
 			if (p->state == TASK_DEAD)
 				sub_rq_bw(p->dl.dl_bw, &rq->dl);
 			raw_spin_lock(&dl_b->lock);
-			__dl_clear(dl_b, p->dl.dl_bw, dl_bw_cpus(task_cpu(p)));
+			__dl_sub(dl_b, p->dl.dl_bw, dl_bw_cpus(task_cpu(p)));
 			__dl_clear_params(p);
 			raw_spin_unlock(&dl_b->lock);
 		}
@@ -1211,7 +1211,7 @@ static enum hrtimer_restart inactive_task_timer(struct hrtimer *timer)
 		}
 
 		raw_spin_lock(&dl_b->lock);
-		__dl_clear(dl_b, p->dl.dl_bw, dl_bw_cpus(task_cpu(p)));
+		__dl_sub(dl_b, p->dl.dl_bw, dl_bw_cpus(task_cpu(p)));
 		raw_spin_unlock(&dl_b->lock);
 		__dl_clear_params(p);
 
@@ -2182,7 +2182,7 @@ static void set_cpus_allowed_dl(struct task_struct *p,
 		 * until we complete the update.
 		 */
 		raw_spin_lock(&src_dl_b->lock);
-		__dl_clear(src_dl_b, p->dl.dl_bw, dl_bw_cpus(task_cpu(p)));
+		__dl_sub(src_dl_b, p->dl.dl_bw, dl_bw_cpus(task_cpu(p)));
 		raw_spin_unlock(&src_dl_b->lock);
 	}
 
@@ -2460,7 +2460,7 @@ int sched_dl_overflow(struct task_struct *p, int policy,
 	if (dl_policy(policy) && !task_has_dl_policy(p) &&
 	    !__dl_overflow(dl_b, cpus, 0, new_bw)) {
 		if (hrtimer_active(&p->dl.inactive_timer))
-			__dl_clear(dl_b, p->dl.dl_bw, cpus);
+			__dl_sub(dl_b, p->dl.dl_bw, cpus);
 		__dl_add(dl_b, new_bw, cpus);
 		err = 0;
 	} else if (dl_policy(policy) && task_has_dl_policy(p) &&
@@ -2472,7 +2472,7 @@ int sched_dl_overflow(struct task_struct *p, int policy,
 		 * But this would require to set the task's "inactive
 		 * timer" when the task is not inactive.
 		 */
-		__dl_clear(dl_b, p->dl.dl_bw, cpus);
+		__dl_sub(dl_b, p->dl.dl_bw, cpus);
 		__dl_add(dl_b, new_bw, cpus);
 		dl_change_utilization(p, new_bw);
 		err = 0;
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 0b93e4b..061ec3b 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -226,7 +226,7 @@ struct dl_bw {
 static inline void __dl_update(struct dl_bw *dl_b, s64 bw);
 
 static inline
-void __dl_clear(struct dl_bw *dl_b, u64 tsk_bw, int cpus)
+void __dl_sub(struct dl_bw *dl_b, u64 tsk_bw, int cpus)
 {
 	dl_b->total_bw -= tsk_bw;
 	__dl_update(dl_b, (s32)tsk_bw / cpus);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/4] sched/deadline: use C bitfields for the state flags
  2017-09-07 10:09 [PATCH 0/4] SCHED_DEADLINE fixes and cleanups luca abeni
                   ` (2 preceding siblings ...)
  2017-09-07 10:09 ` [PATCH 3/4] sched/deadline: rename __dl_clear() to __dl_sub() luca abeni
@ 2017-09-07 10:09 ` luca abeni
  2017-09-08  8:24   ` Daniel Bristot de Oliveira
  2017-10-10 10:57   ` [tip:sched/core] sched/deadline: Use " tip-bot for luca abeni
  2017-10-02  8:18 ` [PATCH 0/4] SCHED_DEADLINE fixes and cleanups Peter Zijlstra
  4 siblings, 2 replies; 13+ messages in thread
From: luca abeni @ 2017-09-07 10:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Zijlstra, Ingo Molnar, Juri Lelli, Steven Rostedt,
	Daniel Bristot de Oliveira, Mathieu Poirier, luca abeni

Ask the compiler to use a single bit for storing true / false values,
instead of wasting the size of a whole int value.
Tested with gcc 5.4.0 on x86_64, and the compiler produces the expected
Assembly (similar to the Assembly code generated when explicitly accessing
the bits with bitmasks, "&" and "|").

Signed-off-by: luca abeni <luca.abeni@santannapisa.it>
---
 include/linux/sched.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 68b3833..e03cc69 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -472,10 +472,10 @@ struct sched_dl_entity {
 	 * conditions between the inactive timer handler and the wakeup
 	 * code.
 	 */
-	int				dl_throttled;
-	int				dl_boosted;
-	int				dl_yielded;
-	int				dl_non_contending;
+	int				dl_throttled      : 1;
+	int				dl_boosted        : 1;
+	int				dl_yielded        : 1;
+	int				dl_non_contending : 1;
 
 	/*
 	 * Bandwidth enforcement timer. Each -deadline task has its
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/4] sched/deadline: use C bitfields for the state flags
  2017-09-07 10:09 ` [PATCH 4/4] sched/deadline: use C bitfields for the state flags luca abeni
@ 2017-09-08  8:24   ` Daniel Bristot de Oliveira
  2017-10-10 10:57   ` [tip:sched/core] sched/deadline: Use " tip-bot for luca abeni
  1 sibling, 0 replies; 13+ messages in thread
From: Daniel Bristot de Oliveira @ 2017-09-08  8:24 UTC (permalink / raw)
  To: luca abeni, linux-kernel
  Cc: Peter Zijlstra, Ingo Molnar, Juri Lelli, Steven Rostedt, Mathieu Poirier

On 09/07/2017 12:09 PM, luca abeni wrote:
> Ask the compiler to use a single bit for storing true / false values,
> instead of wasting the size of a whole int value.
> Tested with gcc 5.4.0 on x86_64, and the compiler produces the expected
> Assembly (similar to the Assembly code generated when explicitly accessing
> the bits with bitmasks, "&" and "|").
> 
> Signed-off-by: luca abeni <luca.abeni@santannapisa.it>

Reviewed-by: Daniel Bristot de Oliveira <bristot@redhat.com>

-- Daniel
> ---
>  include/linux/sched.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 68b3833..e03cc69 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -472,10 +472,10 @@ struct sched_dl_entity {
>  	 * conditions between the inactive timer handler and the wakeup
>  	 * code.
>  	 */
> -	int				dl_throttled;
> -	int				dl_boosted;
> -	int				dl_yielded;
> -	int				dl_non_contending;
> +	int				dl_throttled      : 1;
> +	int				dl_boosted        : 1;
> +	int				dl_yielded        : 1;
> +	int				dl_non_contending : 1;
>  
>  	/*
>  	 * Bandwidth enforcement timer. Each -deadline task has its
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] sched/deadline: rename __dl_clear() to __dl_sub()
  2017-09-07 10:09 ` [PATCH 3/4] sched/deadline: rename __dl_clear() to __dl_sub() luca abeni
@ 2017-09-08  8:26   ` Daniel Bristot de Oliveira
  2017-10-10 10:56   ` [tip:sched/core] sched/deadline: Rename " tip-bot for Peter Zijlstra
  1 sibling, 0 replies; 13+ messages in thread
From: Daniel Bristot de Oliveira @ 2017-09-08  8:26 UTC (permalink / raw)
  To: luca abeni, linux-kernel
  Cc: Peter Zijlstra, Ingo Molnar, Juri Lelli, Steven Rostedt, Mathieu Poirier

On 09/07/2017 12:09 PM, luca abeni wrote:
> From: Peter Zijlstra <peterz@infradead.org>
> 
> __dl_sub() is more meaningful as a name, and is more consistent
> with the naming of the dual function (__dl_add()).
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: luca abeni <luca.abeni@santannapisa.it>

Reviewed-by: Daniel Bristot de Oliveira <bristot@redhat.com>

-- Daniel

> ---
>  kernel/sched/deadline.c | 10 +++++-----
>  kernel/sched/sched.h    |  2 +-
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index d7f9e86..bab11dd 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -242,7 +242,7 @@ static void task_non_contending(struct task_struct *p)
>  			if (p->state == TASK_DEAD)
>  				sub_rq_bw(p->dl.dl_bw, &rq->dl);
>  			raw_spin_lock(&dl_b->lock);
> -			__dl_clear(dl_b, p->dl.dl_bw, dl_bw_cpus(task_cpu(p)));
> +			__dl_sub(dl_b, p->dl.dl_bw, dl_bw_cpus(task_cpu(p)));
>  			__dl_clear_params(p);
>  			raw_spin_unlock(&dl_b->lock);
>  		}
> @@ -1211,7 +1211,7 @@ static enum hrtimer_restart inactive_task_timer(struct hrtimer *timer)
>  		}
>  
>  		raw_spin_lock(&dl_b->lock);
> -		__dl_clear(dl_b, p->dl.dl_bw, dl_bw_cpus(task_cpu(p)));
> +		__dl_sub(dl_b, p->dl.dl_bw, dl_bw_cpus(task_cpu(p)));
>  		raw_spin_unlock(&dl_b->lock);
>  		__dl_clear_params(p);
>  
> @@ -2182,7 +2182,7 @@ static void set_cpus_allowed_dl(struct task_struct *p,
>  		 * until we complete the update.
>  		 */
>  		raw_spin_lock(&src_dl_b->lock);
> -		__dl_clear(src_dl_b, p->dl.dl_bw, dl_bw_cpus(task_cpu(p)));
> +		__dl_sub(src_dl_b, p->dl.dl_bw, dl_bw_cpus(task_cpu(p)));
>  		raw_spin_unlock(&src_dl_b->lock);
>  	}
>  
> @@ -2460,7 +2460,7 @@ int sched_dl_overflow(struct task_struct *p, int policy,
>  	if (dl_policy(policy) && !task_has_dl_policy(p) &&
>  	    !__dl_overflow(dl_b, cpus, 0, new_bw)) {
>  		if (hrtimer_active(&p->dl.inactive_timer))
> -			__dl_clear(dl_b, p->dl.dl_bw, cpus);
> +			__dl_sub(dl_b, p->dl.dl_bw, cpus);
>  		__dl_add(dl_b, new_bw, cpus);
>  		err = 0;
>  	} else if (dl_policy(policy) && task_has_dl_policy(p) &&
> @@ -2472,7 +2472,7 @@ int sched_dl_overflow(struct task_struct *p, int policy,
>  		 * But this would require to set the task's "inactive
>  		 * timer" when the task is not inactive.
>  		 */
> -		__dl_clear(dl_b, p->dl.dl_bw, cpus);
> +		__dl_sub(dl_b, p->dl.dl_bw, cpus);
>  		__dl_add(dl_b, new_bw, cpus);
>  		dl_change_utilization(p, new_bw);
>  		err = 0;
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 0b93e4b..061ec3b 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -226,7 +226,7 @@ struct dl_bw {
>  static inline void __dl_update(struct dl_bw *dl_b, s64 bw);
>  
>  static inline
> -void __dl_clear(struct dl_bw *dl_b, u64 tsk_bw, int cpus)
> +void __dl_sub(struct dl_bw *dl_b, u64 tsk_bw, int cpus)
>  {
>  	dl_b->total_bw -= tsk_bw;
>  	__dl_update(dl_b, (s32)tsk_bw / cpus);
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] sched/sched.h: remove duplicate prototype of __dl_clear_params()
  2017-09-07 10:09 ` [PATCH 1/4] sched/sched.h: remove duplicate prototype of __dl_clear_params() luca abeni
@ 2017-09-08  8:33   ` Daniel Bristot de Oliveira
  2017-10-10 10:55   ` [tip:sched/core] sched/headers: Remove " tip-bot for luca abeni
  1 sibling, 0 replies; 13+ messages in thread
From: Daniel Bristot de Oliveira @ 2017-09-08  8:33 UTC (permalink / raw)
  To: luca abeni, linux-kernel
  Cc: Peter Zijlstra, Ingo Molnar, Juri Lelli, Steven Rostedt, Mathieu Poirier

Hi,

I wonder if a commit log, even an one line comment, explaining that this
function was already declared in the file could help to understand this
patch...

But, the patch is so trivial... that... not sure it is worth... anyway...

On 09/07/2017 12:09 PM, luca abeni wrote:
> Signed-off-by: luca abeni <luca.abeni@santannapisa.it>

Reviewed-by: Daniel Bristot de Oliveira <bristot@redhat.com>

-- Daniel

> ---
>  kernel/sched/sched.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 1043c8b..0b93e4b 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -255,7 +255,6 @@ extern int sched_dl_overflow(struct task_struct *p, int policy,
>  extern void __setparam_dl(struct task_struct *p, const struct sched_attr *attr);
>  extern void __getparam_dl(struct task_struct *p, struct sched_attr *attr);
>  extern bool __checkparam_dl(const struct sched_attr *attr);
> -extern void __dl_clear_params(struct task_struct *p);
>  extern bool dl_param_changed(struct task_struct *p, const struct sched_attr *attr);
>  extern int dl_task_can_attach(struct task_struct *p,
>  			      const struct cpumask *cs_cpus_allowed);
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/4] SCHED_DEADLINE fixes and cleanups
  2017-09-07 10:09 [PATCH 0/4] SCHED_DEADLINE fixes and cleanups luca abeni
                   ` (3 preceding siblings ...)
  2017-09-07 10:09 ` [PATCH 4/4] sched/deadline: use C bitfields for the state flags luca abeni
@ 2017-10-02  8:18 ` Peter Zijlstra
  4 siblings, 0 replies; 13+ messages in thread
From: Peter Zijlstra @ 2017-10-02  8:18 UTC (permalink / raw)
  To: luca abeni
  Cc: linux-kernel, Ingo Molnar, Juri Lelli, Steven Rostedt,
	Daniel Bristot de Oliveira, Mathieu Poirier



Sorry for the delay, but picked them up now.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [tip:sched/core] sched/headers: Remove duplicate prototype of __dl_clear_params()
  2017-09-07 10:09 ` [PATCH 1/4] sched/sched.h: remove duplicate prototype of __dl_clear_params() luca abeni
  2017-09-08  8:33   ` Daniel Bristot de Oliveira
@ 2017-10-10 10:55   ` tip-bot for luca abeni
  1 sibling, 0 replies; 13+ messages in thread
From: tip-bot for luca abeni @ 2017-10-10 10:55 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: torvalds, linux-kernel, juri.lelli, luca.abeni, peterz, mingo,
	bristot, tglx, rostedt, mathieu.poirier, hpa, efault

Commit-ID:  e964d3501b64d6930aaa4dd18955a8cd086ccb92
Gitweb:     https://git.kernel.org/tip/e964d3501b64d6930aaa4dd18955a8cd086ccb92
Author:     luca abeni <luca.abeni@santannapisa.it>
AuthorDate: Thu, 7 Sep 2017 12:09:28 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 10 Oct 2017 11:43:30 +0200

sched/headers: Remove duplicate prototype of __dl_clear_params()

Signed-off-by: luca abeni <luca.abeni@santannapisa.it>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Juri Lelli <juri.lelli@arm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1504778971-13573-2-git-send-email-luca.abeni@santannapisa.it
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/sched.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index e83d1b8..9d5aa18 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -255,7 +255,6 @@ extern int sched_dl_overflow(struct task_struct *p, int policy,
 extern void __setparam_dl(struct task_struct *p, const struct sched_attr *attr);
 extern void __getparam_dl(struct task_struct *p, struct sched_attr *attr);
 extern bool __checkparam_dl(const struct sched_attr *attr);
-extern void __dl_clear_params(struct task_struct *p);
 extern bool dl_param_changed(struct task_struct *p, const struct sched_attr *attr);
 extern int dl_task_can_attach(struct task_struct *p,
 			      const struct cpumask *cs_cpus_allowed);

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [tip:sched/core] sched/deadline: Fix switching to -deadline
  2017-09-07 10:09 ` [PATCH 2/4] sched/deadline: fix switching to -deadline luca abeni
@ 2017-10-10 10:56   ` tip-bot for Luca Abeni
  0 siblings, 0 replies; 13+ messages in thread
From: tip-bot for Luca Abeni @ 2017-10-10 10:56 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: bristot, peterz, mathieu.poirier, linux-kernel, hpa, efault,
	mingo, juri.lelli, rostedt, tglx, luca.abeni, torvalds

Commit-ID:  295d6d5e373607729bcc8182c25afe964655714f
Gitweb:     https://git.kernel.org/tip/295d6d5e373607729bcc8182c25afe964655714f
Author:     Luca Abeni <luca.abeni@santannapisa.it>
AuthorDate: Thu, 7 Sep 2017 12:09:29 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 10 Oct 2017 11:43:30 +0200

sched/deadline: Fix switching to -deadline

Fix a bug introduced in:

  72f9f3fdc928 ("sched/deadline: Remove dl_new from struct sched_dl_entity")

After that commit, when switching to -deadline if the scheduling
deadline of a task is in the past then switched_to_dl() calls
setup_new_entity() to properly initialize the scheduling deadline
and runtime.

The problem is that the task is enqueued _before_ having its parameters
initialized by setup_new_entity(), and this can cause problems.
For example, a task with its out-of-date deadline in the past will
potentially be enqueued as the highest priority one; however, its
adjusted deadline may not be the earliest one.

This patch fixes the problem by initializing the task's parameters before
enqueuing it.

Signed-off-by: luca abeni <luca.abeni@santannapisa.it>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Juri Lelli <juri.lelli@arm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1504778971-13573-3-git-send-email-luca.abeni@santannapisa.it
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/deadline.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 0191ec7..9d45354 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -1364,6 +1364,10 @@ enqueue_dl_entity(struct sched_dl_entity *dl_se,
 		update_dl_entity(dl_se, pi_se);
 	} else if (flags & ENQUEUE_REPLENISH) {
 		replenish_dl_entity(dl_se, pi_se);
+	} else if ((flags & ENQUEUE_RESTORE) &&
+		  dl_time_before(dl_se->deadline,
+				 rq_clock(rq_of_dl_rq(dl_rq_of_se(dl_se))))) {
+		setup_new_dl_entity(dl_se);
 	}
 
 	__enqueue_dl_entity(dl_se);
@@ -2255,13 +2259,6 @@ static void switched_to_dl(struct rq *rq, struct task_struct *p)
 
 		return;
 	}
-	/*
-	 * If p is boosted we already updated its params in
-	 * rt_mutex_setprio()->enqueue_task(..., ENQUEUE_REPLENISH),
-	 * p's deadline being now already after rq_clock(rq).
-	 */
-	if (dl_time_before(p->dl.deadline, rq_clock(rq)))
-		setup_new_dl_entity(&p->dl);
 
 	if (rq->curr != p) {
 #ifdef CONFIG_SMP

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [tip:sched/core] sched/deadline: Rename __dl_clear() to __dl_sub()
  2017-09-07 10:09 ` [PATCH 3/4] sched/deadline: rename __dl_clear() to __dl_sub() luca abeni
  2017-09-08  8:26   ` Daniel Bristot de Oliveira
@ 2017-10-10 10:56   ` tip-bot for Peter Zijlstra
  1 sibling, 0 replies; 13+ messages in thread
From: tip-bot for Peter Zijlstra @ 2017-10-10 10:56 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: efault, mathieu.poirier, hpa, rostedt, luca.abeni, tglx, bristot,
	linux-kernel, torvalds, peterz, juri.lelli, mingo

Commit-ID:  8c0944cee7af55291df0b28e6e2eeac0930e93c9
Gitweb:     https://git.kernel.org/tip/8c0944cee7af55291df0b28e6e2eeac0930e93c9
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Thu, 7 Sep 2017 12:09:30 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 10 Oct 2017 11:45:26 +0200

sched/deadline: Rename __dl_clear() to __dl_sub()

__dl_sub() is more meaningful as a name, and is more consistent
with the naming of the dual function (__dl_add()).

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Luca Abeni <luca.abeni@santannapisa.it>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Juri Lelli <juri.lelli@arm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1504778971-13573-4-git-send-email-luca.abeni@santannapisa.it
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/deadline.c | 10 +++++-----
 kernel/sched/sched.h    |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 9d45354..8d1b946 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -242,7 +242,7 @@ static void task_non_contending(struct task_struct *p)
 			if (p->state == TASK_DEAD)
 				sub_rq_bw(p->dl.dl_bw, &rq->dl);
 			raw_spin_lock(&dl_b->lock);
-			__dl_clear(dl_b, p->dl.dl_bw, dl_bw_cpus(task_cpu(p)));
+			__dl_sub(dl_b, p->dl.dl_bw, dl_bw_cpus(task_cpu(p)));
 			__dl_clear_params(p);
 			raw_spin_unlock(&dl_b->lock);
 		}
@@ -1209,7 +1209,7 @@ static enum hrtimer_restart inactive_task_timer(struct hrtimer *timer)
 		}
 
 		raw_spin_lock(&dl_b->lock);
-		__dl_clear(dl_b, p->dl.dl_bw, dl_bw_cpus(task_cpu(p)));
+		__dl_sub(dl_b, p->dl.dl_bw, dl_bw_cpus(task_cpu(p)));
 		raw_spin_unlock(&dl_b->lock);
 		__dl_clear_params(p);
 
@@ -2170,7 +2170,7 @@ static void set_cpus_allowed_dl(struct task_struct *p,
 		 * until we complete the update.
 		 */
 		raw_spin_lock(&src_dl_b->lock);
-		__dl_clear(src_dl_b, p->dl.dl_bw, dl_bw_cpus(task_cpu(p)));
+		__dl_sub(src_dl_b, p->dl.dl_bw, dl_bw_cpus(task_cpu(p)));
 		raw_spin_unlock(&src_dl_b->lock);
 	}
 
@@ -2448,7 +2448,7 @@ int sched_dl_overflow(struct task_struct *p, int policy,
 	if (dl_policy(policy) && !task_has_dl_policy(p) &&
 	    !__dl_overflow(dl_b, cpus, 0, new_bw)) {
 		if (hrtimer_active(&p->dl.inactive_timer))
-			__dl_clear(dl_b, p->dl.dl_bw, cpus);
+			__dl_sub(dl_b, p->dl.dl_bw, cpus);
 		__dl_add(dl_b, new_bw, cpus);
 		err = 0;
 	} else if (dl_policy(policy) && task_has_dl_policy(p) &&
@@ -2460,7 +2460,7 @@ int sched_dl_overflow(struct task_struct *p, int policy,
 		 * But this would require to set the task's "inactive
 		 * timer" when the task is not inactive.
 		 */
-		__dl_clear(dl_b, p->dl.dl_bw, cpus);
+		__dl_sub(dl_b, p->dl.dl_bw, cpus);
 		__dl_add(dl_b, new_bw, cpus);
 		dl_change_utilization(p, new_bw);
 		err = 0;
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 9d5aa18..a81c978 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -226,7 +226,7 @@ struct dl_bw {
 static inline void __dl_update(struct dl_bw *dl_b, s64 bw);
 
 static inline
-void __dl_clear(struct dl_bw *dl_b, u64 tsk_bw, int cpus)
+void __dl_sub(struct dl_bw *dl_b, u64 tsk_bw, int cpus)
 {
 	dl_b->total_bw -= tsk_bw;
 	__dl_update(dl_b, (s32)tsk_bw / cpus);

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [tip:sched/core] sched/deadline: Use C bitfields for the state flags
  2017-09-07 10:09 ` [PATCH 4/4] sched/deadline: use C bitfields for the state flags luca abeni
  2017-09-08  8:24   ` Daniel Bristot de Oliveira
@ 2017-10-10 10:57   ` tip-bot for luca abeni
  1 sibling, 0 replies; 13+ messages in thread
From: tip-bot for luca abeni @ 2017-10-10 10:57 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mingo, linux-kernel, peterz, rostedt, juri.lelli,
	mathieu.poirier, bristot, tglx, torvalds, efault, luca.abeni,
	hpa

Commit-ID:  799ba82de01e7543f6b2042e1a739f3a20255f23
Gitweb:     https://git.kernel.org/tip/799ba82de01e7543f6b2042e1a739f3a20255f23
Author:     luca abeni <luca.abeni@santannapisa.it>
AuthorDate: Thu, 7 Sep 2017 12:09:31 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 10 Oct 2017 11:45:26 +0200

sched/deadline: Use C bitfields for the state flags

Ask the compiler to use a single bit for storing true / false values,
instead of wasting the size of a whole int value.
Tested with gcc 5.4.0 on x86_64, and the compiler produces the expected
Assembly (similar to the Assembly code generated when explicitly accessing
the bits with bitmasks, "&" and "|").

Signed-off-by: luca abeni <luca.abeni@santannapisa.it>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Juri Lelli <juri.lelli@arm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1504778971-13573-5-git-send-email-luca.abeni@santannapisa.it
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 include/linux/sched.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 33a01f4..0f897df 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -474,10 +474,10 @@ struct sched_dl_entity {
 	 * conditions between the inactive timer handler and the wakeup
 	 * code.
 	 */
-	int				dl_throttled;
-	int				dl_boosted;
-	int				dl_yielded;
-	int				dl_non_contending;
+	int				dl_throttled      : 1;
+	int				dl_boosted        : 1;
+	int				dl_yielded        : 1;
+	int				dl_non_contending : 1;
 
 	/*
 	 * Bandwidth enforcement timer. Each -deadline task has its

^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2017-10-10 11:01 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-07 10:09 [PATCH 0/4] SCHED_DEADLINE fixes and cleanups luca abeni
2017-09-07 10:09 ` [PATCH 1/4] sched/sched.h: remove duplicate prototype of __dl_clear_params() luca abeni
2017-09-08  8:33   ` Daniel Bristot de Oliveira
2017-10-10 10:55   ` [tip:sched/core] sched/headers: Remove " tip-bot for luca abeni
2017-09-07 10:09 ` [PATCH 2/4] sched/deadline: fix switching to -deadline luca abeni
2017-10-10 10:56   ` [tip:sched/core] sched/deadline: Fix " tip-bot for Luca Abeni
2017-09-07 10:09 ` [PATCH 3/4] sched/deadline: rename __dl_clear() to __dl_sub() luca abeni
2017-09-08  8:26   ` Daniel Bristot de Oliveira
2017-10-10 10:56   ` [tip:sched/core] sched/deadline: Rename " tip-bot for Peter Zijlstra
2017-09-07 10:09 ` [PATCH 4/4] sched/deadline: use C bitfields for the state flags luca abeni
2017-09-08  8:24   ` Daniel Bristot de Oliveira
2017-10-10 10:57   ` [tip:sched/core] sched/deadline: Use " tip-bot for luca abeni
2017-10-02  8:18 ` [PATCH 0/4] SCHED_DEADLINE fixes and cleanups Peter Zijlstra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).