linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] sched/deadline: remove useless param from setup_new_dl_entity
@ 2016-06-17  9:48 Juri Lelli
  2016-06-17  9:58 ` luca abeni
  2016-06-17 13:49 ` Steven Rostedt
  0 siblings, 2 replies; 9+ messages in thread
From: Juri Lelli @ 2016-06-17  9:48 UTC (permalink / raw)
  To: linux-kernel; +Cc: peterz, mingo, luca.abeni, rostedt, juri.lelli

setup_new_dl_entity() takes two parameters, but it only actually uses
one of them to setup a new dl_entity.

Remove the second, useless, parameter.

Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Luca Abeni <luca.abeni@unitn.it>
Signed-off-by: Juri Lelli <juri.lelli@arm.com>
---
 kernel/sched/deadline.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index fcb7f0217ff4..5229788a4765 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -346,8 +346,7 @@ static void check_preempt_curr_dl(struct rq *rq, struct task_struct *p,
  * one, and to (try to!) reconcile itself with its own scheduling
  * parameters.
  */
-static inline void setup_new_dl_entity(struct sched_dl_entity *dl_se,
-				       struct sched_dl_entity *pi_se)
+static inline void setup_new_dl_entity(struct sched_dl_entity *dl_se)
 {
 	struct dl_rq *dl_rq = dl_rq_of_se(dl_se);
 	struct rq *rq = rq_of_dl_rq(dl_rq);
@@ -367,8 +366,8 @@ static inline void setup_new_dl_entity(struct sched_dl_entity *dl_se,
 	 * future; in fact, we must consider execution overheads (time
 	 * spent on hardirq context, etc.).
 	 */
-	dl_se->deadline = rq_clock(rq) + pi_se->dl_deadline;
-	dl_se->runtime = pi_se->dl_runtime;
+	dl_se->deadline = rq_clock(rq) + dl_se->dl_deadline;
+	dl_se->runtime = dl_se->dl_runtime;
 }
 
 /*
@@ -1721,7 +1720,7 @@ static void switched_from_dl(struct rq *rq, struct task_struct *p)
 static void switched_to_dl(struct rq *rq, struct task_struct *p)
 {
 	if (dl_time_before(p->dl.deadline, rq_clock(rq)))
-		setup_new_dl_entity(&p->dl, &p->dl);
+		setup_new_dl_entity(&p->dl);
 
 	if (task_on_rq_queued(p) && rq->curr != p) {
 #ifdef CONFIG_SMP
-- 
2.7.0

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

* Re: [PATCH] sched/deadline: remove useless param from setup_new_dl_entity
  2016-06-17  9:48 [PATCH] sched/deadline: remove useless param from setup_new_dl_entity Juri Lelli
@ 2016-06-17  9:58 ` luca abeni
  2016-06-17 10:08   ` Juri Lelli
  2016-06-17 13:49 ` Steven Rostedt
  1 sibling, 1 reply; 9+ messages in thread
From: luca abeni @ 2016-06-17  9:58 UTC (permalink / raw)
  To: Juri Lelli; +Cc: linux-kernel, peterz, mingo, rostedt

Hi,

On Fri, 17 Jun 2016 10:48:41 +0100
Juri Lelli <juri.lelli@arm.com> wrote:

> setup_new_dl_entity() takes two parameters, but it only actually uses
> one of them to setup a new dl_entity.
> 
> Remove the second, useless, parameter.

Funnily enough, I was adding something similar in my local queue :)
Looks good to me



			Thanks,
				Luca


> 
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Luca Abeni <luca.abeni@unitn.it>
> Signed-off-by: Juri Lelli <juri.lelli@arm.com>
> ---
>  kernel/sched/deadline.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index fcb7f0217ff4..5229788a4765 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -346,8 +346,7 @@ static void check_preempt_curr_dl(struct rq *rq,
> struct task_struct *p,
>   * one, and to (try to!) reconcile itself with its own scheduling
>   * parameters.
>   */
> -static inline void setup_new_dl_entity(struct sched_dl_entity *dl_se,
> -				       struct sched_dl_entity *pi_se)
> +static inline void setup_new_dl_entity(struct sched_dl_entity *dl_se)
>  {
>  	struct dl_rq *dl_rq = dl_rq_of_se(dl_se);
>  	struct rq *rq = rq_of_dl_rq(dl_rq);
> @@ -367,8 +366,8 @@ static inline void setup_new_dl_entity(struct
> sched_dl_entity *dl_se,
>  	 * future; in fact, we must consider execution overheads
> (time
>  	 * spent on hardirq context, etc.).
>  	 */
> -	dl_se->deadline = rq_clock(rq) + pi_se->dl_deadline;
> -	dl_se->runtime = pi_se->dl_runtime;
> +	dl_se->deadline = rq_clock(rq) + dl_se->dl_deadline;
> +	dl_se->runtime = dl_se->dl_runtime;
>  }
>  
>  /*
> @@ -1721,7 +1720,7 @@ static void switched_from_dl(struct rq *rq,
> struct task_struct *p) static void switched_to_dl(struct rq *rq,
> struct task_struct *p) {
>  	if (dl_time_before(p->dl.deadline, rq_clock(rq)))
> -		setup_new_dl_entity(&p->dl, &p->dl);
> +		setup_new_dl_entity(&p->dl);
>  
>  	if (task_on_rq_queued(p) && rq->curr != p) {
>  #ifdef CONFIG_SMP

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

* Re: [PATCH] sched/deadline: remove useless param from setup_new_dl_entity
  2016-06-17  9:58 ` luca abeni
@ 2016-06-17 10:08   ` Juri Lelli
  0 siblings, 0 replies; 9+ messages in thread
From: Juri Lelli @ 2016-06-17 10:08 UTC (permalink / raw)
  To: luca abeni; +Cc: linux-kernel, peterz, mingo, rostedt

On 17/06/16 11:58, Luca Abeni wrote:
> Hi,
> 
> On Fri, 17 Jun 2016 10:48:41 +0100
> Juri Lelli <juri.lelli@arm.com> wrote:
> 
> > setup_new_dl_entity() takes two parameters, but it only actually uses
> > one of them to setup a new dl_entity.
> > 
> > Remove the second, useless, parameter.
> 
> Funnily enough, I was adding something similar in my local queue :)
> Looks good to me
> 

Yeah, noticed this while looking at your reclaiming patches. :)

Thanks for reviewing,

- Juri

> 
> > 
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Luca Abeni <luca.abeni@unitn.it>
> > Signed-off-by: Juri Lelli <juri.lelli@arm.com>
> > ---
> >  kernel/sched/deadline.c | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> > index fcb7f0217ff4..5229788a4765 100644
> > --- a/kernel/sched/deadline.c
> > +++ b/kernel/sched/deadline.c
> > @@ -346,8 +346,7 @@ static void check_preempt_curr_dl(struct rq *rq,
> > struct task_struct *p,
> >   * one, and to (try to!) reconcile itself with its own scheduling
> >   * parameters.
> >   */
> > -static inline void setup_new_dl_entity(struct sched_dl_entity *dl_se,
> > -				       struct sched_dl_entity *pi_se)
> > +static inline void setup_new_dl_entity(struct sched_dl_entity *dl_se)
> >  {
> >  	struct dl_rq *dl_rq = dl_rq_of_se(dl_se);
> >  	struct rq *rq = rq_of_dl_rq(dl_rq);
> > @@ -367,8 +366,8 @@ static inline void setup_new_dl_entity(struct
> > sched_dl_entity *dl_se,
> >  	 * future; in fact, we must consider execution overheads
> > (time
> >  	 * spent on hardirq context, etc.).
> >  	 */
> > -	dl_se->deadline = rq_clock(rq) + pi_se->dl_deadline;
> > -	dl_se->runtime = pi_se->dl_runtime;
> > +	dl_se->deadline = rq_clock(rq) + dl_se->dl_deadline;
> > +	dl_se->runtime = dl_se->dl_runtime;
> >  }
> >  
> >  /*
> > @@ -1721,7 +1720,7 @@ static void switched_from_dl(struct rq *rq,
> > struct task_struct *p) static void switched_to_dl(struct rq *rq,
> > struct task_struct *p) {
> >  	if (dl_time_before(p->dl.deadline, rq_clock(rq)))
> > -		setup_new_dl_entity(&p->dl, &p->dl);
> > +		setup_new_dl_entity(&p->dl);
> >  
> >  	if (task_on_rq_queued(p) && rq->curr != p) {
> >  #ifdef CONFIG_SMP
> 

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

* Re: [PATCH] sched/deadline: remove useless param from setup_new_dl_entity
  2016-06-17  9:48 [PATCH] sched/deadline: remove useless param from setup_new_dl_entity Juri Lelli
  2016-06-17  9:58 ` luca abeni
@ 2016-06-17 13:49 ` Steven Rostedt
  2016-06-17 16:28   ` Juri Lelli
  1 sibling, 1 reply; 9+ messages in thread
From: Steven Rostedt @ 2016-06-17 13:49 UTC (permalink / raw)
  To: Juri Lelli; +Cc: linux-kernel, peterz, mingo, luca.abeni

On Fri, 17 Jun 2016 10:48:41 +0100
Juri Lelli <juri.lelli@arm.com> wrote:

> setup_new_dl_entity() takes two parameters, but it only actually uses
> one of them to setup a new dl_entity.
> 

Actually this patch is making it so that setup_new_dl_entity() only
uses one of the parameters. Can you note why that change happened.
Because this change log implies that the second parameter wasn't used
before this patch, and that is incorrect.

This patch reverts part of the change done in
commit 2d3d891d334 "sched/deadline: Add SCHED_DEADLINE inheritance
logic"

It would be nice to have the reason in the change log.

Thanks,

-- Steve


> Remove the second, useless, parameter.
> 
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Luca Abeni <luca.abeni@unitn.it>
> Signed-off-by: Juri Lelli <juri.lelli@arm.com>
> ---
>  kernel/sched/deadline.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index fcb7f0217ff4..5229788a4765 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -346,8 +346,7 @@ static void check_preempt_curr_dl(struct rq *rq, struct task_struct *p,
>   * one, and to (try to!) reconcile itself with its own scheduling
>   * parameters.
>   */
> -static inline void setup_new_dl_entity(struct sched_dl_entity *dl_se,
> -				       struct sched_dl_entity *pi_se)
> +static inline void setup_new_dl_entity(struct sched_dl_entity *dl_se)
>  {
>  	struct dl_rq *dl_rq = dl_rq_of_se(dl_se);
>  	struct rq *rq = rq_of_dl_rq(dl_rq);
> @@ -367,8 +366,8 @@ static inline void setup_new_dl_entity(struct sched_dl_entity *dl_se,
>  	 * future; in fact, we must consider execution overheads (time
>  	 * spent on hardirq context, etc.).
>  	 */
> -	dl_se->deadline = rq_clock(rq) + pi_se->dl_deadline;
> -	dl_se->runtime = pi_se->dl_runtime;
> +	dl_se->deadline = rq_clock(rq) + dl_se->dl_deadline;
> +	dl_se->runtime = dl_se->dl_runtime;
>  }
>  
>  /*
> @@ -1721,7 +1720,7 @@ static void switched_from_dl(struct rq *rq, struct task_struct *p)
>  static void switched_to_dl(struct rq *rq, struct task_struct *p)
>  {
>  	if (dl_time_before(p->dl.deadline, rq_clock(rq)))
> -		setup_new_dl_entity(&p->dl, &p->dl);
> +		setup_new_dl_entity(&p->dl);
>  
>  	if (task_on_rq_queued(p) && rq->curr != p) {
>  #ifdef CONFIG_SMP

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

* Re: [PATCH] sched/deadline: remove useless param from setup_new_dl_entity
  2016-06-17 13:49 ` Steven Rostedt
@ 2016-06-17 16:28   ` Juri Lelli
  2016-06-17 20:15     ` luca abeni
  2016-06-27 15:52     ` Peter Zijlstra
  0 siblings, 2 replies; 9+ messages in thread
From: Juri Lelli @ 2016-06-17 16:28 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-kernel, peterz, mingo, luca.abeni

On 17/06/16 09:49, Steven Rostedt wrote:
> On Fri, 17 Jun 2016 10:48:41 +0100
> Juri Lelli <juri.lelli@arm.com> wrote:
> 
> > setup_new_dl_entity() takes two parameters, but it only actually uses
> > one of them to setup a new dl_entity.
> > 
> 
> Actually this patch is making it so that setup_new_dl_entity() only
> uses one of the parameters. Can you note why that change happened.
> Because this change log implies that the second parameter wasn't used
> before this patch, and that is incorrect.
> 

True, but we were practically already using the same parameter, under a
different name though, after

2f9f3fdc928 "sched/deadline: Remove dl_new from struct sched_dl_entity"

as we currently do:

  setup_new_dl_entity(&p->dl, &p->dl)

> This patch reverts part of the change done in
> commit 2d3d891d334 "sched/deadline: Add SCHED_DEADLINE inheritance
> logic"
> 

Before Luca's change we were doing

 setup_new_dl_entity(dl_se, pi_se)

in update_dl_entity() for a dl_se->new entity. So, I guess the question
is actually why we wanted to use pi_se's parameters (the potential PI
donor) for setting up a new entity? Maybe we broke the situation where a
task is currently boosted by a DEADLINE waiter and we swich the holder
to DEADLINE?

> It would be nice to have the reason in the change log.
> 

Thanks a lot for pointing out what might be more than inaccuracy in the
changelog.

Best,

- Juri

> 
> > Remove the second, useless, parameter.
> > 
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Luca Abeni <luca.abeni@unitn.it>
> > Signed-off-by: Juri Lelli <juri.lelli@arm.com>
> > ---
> >  kernel/sched/deadline.c | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> > index fcb7f0217ff4..5229788a4765 100644
> > --- a/kernel/sched/deadline.c
> > +++ b/kernel/sched/deadline.c
> > @@ -346,8 +346,7 @@ static void check_preempt_curr_dl(struct rq *rq, struct task_struct *p,
> >   * one, and to (try to!) reconcile itself with its own scheduling
> >   * parameters.
> >   */
> > -static inline void setup_new_dl_entity(struct sched_dl_entity *dl_se,
> > -				       struct sched_dl_entity *pi_se)
> > +static inline void setup_new_dl_entity(struct sched_dl_entity *dl_se)
> >  {
> >  	struct dl_rq *dl_rq = dl_rq_of_se(dl_se);
> >  	struct rq *rq = rq_of_dl_rq(dl_rq);
> > @@ -367,8 +366,8 @@ static inline void setup_new_dl_entity(struct sched_dl_entity *dl_se,
> >  	 * future; in fact, we must consider execution overheads (time
> >  	 * spent on hardirq context, etc.).
> >  	 */
> > -	dl_se->deadline = rq_clock(rq) + pi_se->dl_deadline;
> > -	dl_se->runtime = pi_se->dl_runtime;
> > +	dl_se->deadline = rq_clock(rq) + dl_se->dl_deadline;
> > +	dl_se->runtime = dl_se->dl_runtime;
> >  }
> >  
> >  /*
> > @@ -1721,7 +1720,7 @@ static void switched_from_dl(struct rq *rq, struct task_struct *p)
> >  static void switched_to_dl(struct rq *rq, struct task_struct *p)
> >  {
> >  	if (dl_time_before(p->dl.deadline, rq_clock(rq)))
> > -		setup_new_dl_entity(&p->dl, &p->dl);
> > +		setup_new_dl_entity(&p->dl);
> >  
> >  	if (task_on_rq_queued(p) && rq->curr != p) {
> >  #ifdef CONFIG_SMP
> 

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

* Re: [PATCH] sched/deadline: remove useless param from setup_new_dl_entity
  2016-06-17 16:28   ` Juri Lelli
@ 2016-06-17 20:15     ` luca abeni
  2016-06-17 20:36       ` luca abeni
  2016-06-27 15:52     ` Peter Zijlstra
  1 sibling, 1 reply; 9+ messages in thread
From: luca abeni @ 2016-06-17 20:15 UTC (permalink / raw)
  To: Juri Lelli; +Cc: Steven Rostedt, linux-kernel, peterz, mingo

On Fri, 17 Jun 2016 17:28:37 +0100
Juri Lelli <juri.lelli@arm.com> wrote:
[...]
> True, but we were practically already using the same parameter, under a
> different name though, after
> 
> 2f9f3fdc928 "sched/deadline: Remove dl_new from struct sched_dl_entity"
> 
> as we currently do:
> 
>   setup_new_dl_entity(&p->dl, &p->dl)
> 
> > This patch reverts part of the change done in
> > commit 2d3d891d334 "sched/deadline: Add SCHED_DEADLINE inheritance
> > logic"
> >   
> 
> Before Luca's change we were doing
> 
>  setup_new_dl_entity(dl_se, pi_se)
> 
> in update_dl_entity() for a dl_se->new entity. So, I guess the question
> is actually why we wanted to use pi_se's parameters (the potential PI
> donor) for setting up a new entity?
That's a good question :)

> Maybe we broke the situation where a
> task is currently boosted by a DEADLINE waiter and we swich the holder
> to DEADLINE?
I remember I tested this setup (using linaro's version of rt-app), and
it seemed to work correctly...

Re-reading the code now, I actually wonder why my patch did not break
inheritance in this situation...


			Luca
> 
> > It would be nice to have the reason in the change log.
> >   
> 
> Thanks a lot for pointing out what might be more than inaccuracy in the
> changelog.
> 
> Best,
> 
> - Juri
> 
> >   
> > > Remove the second, useless, parameter.
> > > 
> > > Cc: Ingo Molnar <mingo@redhat.com>
> > > Cc: Peter Zijlstra <peterz@infradead.org>
> > > Cc: Steven Rostedt <rostedt@goodmis.org>
> > > Cc: Luca Abeni <luca.abeni@unitn.it>
> > > Signed-off-by: Juri Lelli <juri.lelli@arm.com>
> > > ---
> > >  kernel/sched/deadline.c | 9 ++++-----
> > >  1 file changed, 4 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> > > index fcb7f0217ff4..5229788a4765 100644
> > > --- a/kernel/sched/deadline.c
> > > +++ b/kernel/sched/deadline.c
> > > @@ -346,8 +346,7 @@ static void check_preempt_curr_dl(struct rq *rq, struct task_struct *p,
> > >   * one, and to (try to!) reconcile itself with its own scheduling
> > >   * parameters.
> > >   */
> > > -static inline void setup_new_dl_entity(struct sched_dl_entity *dl_se,
> > > -				       struct sched_dl_entity *pi_se)
> > > +static inline void setup_new_dl_entity(struct sched_dl_entity *dl_se)
> > >  {
> > >  	struct dl_rq *dl_rq = dl_rq_of_se(dl_se);
> > >  	struct rq *rq = rq_of_dl_rq(dl_rq);
> > > @@ -367,8 +366,8 @@ static inline void setup_new_dl_entity(struct sched_dl_entity *dl_se,
> > >  	 * future; in fact, we must consider execution overheads (time
> > >  	 * spent on hardirq context, etc.).
> > >  	 */
> > > -	dl_se->deadline = rq_clock(rq) + pi_se->dl_deadline;
> > > -	dl_se->runtime = pi_se->dl_runtime;
> > > +	dl_se->deadline = rq_clock(rq) + dl_se->dl_deadline;
> > > +	dl_se->runtime = dl_se->dl_runtime;
> > >  }
> > >  
> > >  /*
> > > @@ -1721,7 +1720,7 @@ static void switched_from_dl(struct rq *rq, struct task_struct *p)
> > >  static void switched_to_dl(struct rq *rq, struct task_struct *p)
> > >  {
> > >  	if (dl_time_before(p->dl.deadline, rq_clock(rq)))
> > > -		setup_new_dl_entity(&p->dl, &p->dl);
> > > +		setup_new_dl_entity(&p->dl);
> > >  
> > >  	if (task_on_rq_queued(p) && rq->curr != p) {
> > >  #ifdef CONFIG_SMP  
> >   

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

* Re: [PATCH] sched/deadline: remove useless param from setup_new_dl_entity
  2016-06-17 20:15     ` luca abeni
@ 2016-06-17 20:36       ` luca abeni
  0 siblings, 0 replies; 9+ messages in thread
From: luca abeni @ 2016-06-17 20:36 UTC (permalink / raw)
  To: Juri Lelli; +Cc: Steven Rostedt, linux-kernel, peterz, mingo

On Fri, 17 Jun 2016 22:15:18 +0200
luca abeni <luca.abeni@unitn.it> wrote:

> On Fri, 17 Jun 2016 17:28:37 +0100
> Juri Lelli <juri.lelli@arm.com> wrote:
> [...]
> > True, but we were practically already using the same parameter, under a
> > different name though, after
> > 
> > 2f9f3fdc928 "sched/deadline: Remove dl_new from struct sched_dl_entity"
> > 
> > as we currently do:
> > 
> >   setup_new_dl_entity(&p->dl, &p->dl)
> >   
> > > This patch reverts part of the change done in
> > > commit 2d3d891d334 "sched/deadline: Add SCHED_DEADLINE inheritance
> > > logic"
> > >     
> > 
> > Before Luca's change we were doing
> > 
> >  setup_new_dl_entity(dl_se, pi_se)
> > 
> > in update_dl_entity() for a dl_se->new entity. So, I guess the question
> > is actually why we wanted to use pi_se's parameters (the potential PI
> > donor) for setting up a new entity?  
> That's a good question :)
> 
> > Maybe we broke the situation where a
> > task is currently boosted by a DEADLINE waiter and we swich the holder
> > to DEADLINE?  
> I remember I tested this setup (using linaro's version of rt-app), and
> it seemed to work correctly...
> 
> Re-reading the code now, I actually wonder why my patch did not break
> inheritance in this situation...
Ok; I think I know why inheritance is not broken (or, at least, it does
not appear to be broken when testing it with rt-app):
- When a -deadline task blocks on a mutex that is held by a SCHED_OTHER
  or SCHED_FIFO task, such a task is promoted to -deadline
- setup_new_dl_entity() is invoked, and it sets the tasks' deadline to
  rq_clock(rq) (+ 0), so the task holding the lock is immediately
  scheduled
- as soon as update_curr_dl() is invoked (in the worst case at the next
  tick), the task's deadline and runtime are set to the "desired values"
  (using pi_se)

So, the behaviour is not changed too much respect to the previous one.



				Luca

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

* Re: [PATCH] sched/deadline: remove useless param from setup_new_dl_entity
  2016-06-17 16:28   ` Juri Lelli
  2016-06-17 20:15     ` luca abeni
@ 2016-06-27 15:52     ` Peter Zijlstra
  2016-06-27 17:24       ` Juri Lelli
  1 sibling, 1 reply; 9+ messages in thread
From: Peter Zijlstra @ 2016-06-27 15:52 UTC (permalink / raw)
  To: Juri Lelli; +Cc: Steven Rostedt, linux-kernel, mingo, luca.abeni

On Fri, Jun 17, 2016 at 05:28:37PM +0100, Juri Lelli wrote:
> On 17/06/16 09:49, Steven Rostedt wrote:
> > On Fri, 17 Jun 2016 10:48:41 +0100
> > Juri Lelli <juri.lelli@arm.com> wrote:
> > 
> > > setup_new_dl_entity() takes two parameters, but it only actually uses
> > > one of them to setup a new dl_entity.
> > > 
> > 
> > Actually this patch is making it so that setup_new_dl_entity() only
> > uses one of the parameters. Can you note why that change happened.
> > Because this change log implies that the second parameter wasn't used
> > before this patch, and that is incorrect.
> > 
> 
> True, but we were practically already using the same parameter, under a
> different name though, after
> 
> 2f9f3fdc928 "sched/deadline: Remove dl_new from struct sched_dl_entity"
> 
> as we currently do:
> 
>   setup_new_dl_entity(&p->dl, &p->dl)
> 
> > This patch reverts part of the change done in
> > commit 2d3d891d334 "sched/deadline: Add SCHED_DEADLINE inheritance
> > logic"
> > 
> 
> Before Luca's change we were doing
> 
>  setup_new_dl_entity(dl_se, pi_se)
> 
> in update_dl_entity() for a dl_se->new entity. So, I guess the question
> is actually why we wanted to use pi_se's parameters (the potential PI
> donor) for setting up a new entity? Maybe we broke the situation where a
> task is currently boosted by a DEADLINE waiter and we swich the holder
> to DEADLINE?
> 
> > It would be nice to have the reason in the change log.
> > 
> 
> Thanks a lot for pointing out what might be more than inaccuracy in the
> changelog.

Will you be reposting with a new Changelog?

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

* Re: [PATCH] sched/deadline: remove useless param from setup_new_dl_entity
  2016-06-27 15:52     ` Peter Zijlstra
@ 2016-06-27 17:24       ` Juri Lelli
  0 siblings, 0 replies; 9+ messages in thread
From: Juri Lelli @ 2016-06-27 17:24 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: Steven Rostedt, linux-kernel, mingo, luca.abeni

On 27/06/16 17:52, Peter Zijlstra wrote:
> On Fri, Jun 17, 2016 at 05:28:37PM +0100, Juri Lelli wrote:
> > On 17/06/16 09:49, Steven Rostedt wrote:
> > > On Fri, 17 Jun 2016 10:48:41 +0100
> > > Juri Lelli <juri.lelli@arm.com> wrote:
> > > 
> > > > setup_new_dl_entity() takes two parameters, but it only actually uses
> > > > one of them to setup a new dl_entity.
> > > > 
> > > 
> > > Actually this patch is making it so that setup_new_dl_entity() only
> > > uses one of the parameters. Can you note why that change happened.
> > > Because this change log implies that the second parameter wasn't used
> > > before this patch, and that is incorrect.
> > > 
> > 
> > True, but we were practically already using the same parameter, under a
> > different name though, after
> > 
> > 2f9f3fdc928 "sched/deadline: Remove dl_new from struct sched_dl_entity"
> > 
> > as we currently do:
> > 
> >   setup_new_dl_entity(&p->dl, &p->dl)
> > 
> > > This patch reverts part of the change done in
> > > commit 2d3d891d334 "sched/deadline: Add SCHED_DEADLINE inheritance
> > > logic"
> > > 
> > 
> > Before Luca's change we were doing
> > 
> >  setup_new_dl_entity(dl_se, pi_se)
> > 
> > in update_dl_entity() for a dl_se->new entity. So, I guess the question
> > is actually why we wanted to use pi_se's parameters (the potential PI
> > donor) for setting up a new entity? Maybe we broke the situation where a
> > task is currently boosted by a DEADLINE waiter and we swich the holder
> > to DEADLINE?
> > 
> > > It would be nice to have the reason in the change log.
> > > 
> > 
> > Thanks a lot for pointing out what might be more than inaccuracy in the
> > changelog.
> 
> Will you be reposting with a new Changelog?
> 

Yes. Sorry, I didn't have much time to follow up on this. I actually
think that a different change is required. Let's discuss that on v2.

Best,

- Juri

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

end of thread, other threads:[~2016-06-27 17:24 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-17  9:48 [PATCH] sched/deadline: remove useless param from setup_new_dl_entity Juri Lelli
2016-06-17  9:58 ` luca abeni
2016-06-17 10:08   ` Juri Lelli
2016-06-17 13:49 ` Steven Rostedt
2016-06-17 16:28   ` Juri Lelli
2016-06-17 20:15     ` luca abeni
2016-06-17 20:36       ` luca abeni
2016-06-27 15:52     ` Peter Zijlstra
2016-06-27 17:24       ` Juri Lelli

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).