Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1205075 > unrolled thread

Re: [RFCv5 PATCH 11/46] sched: Remove blocked load and utilization contributions of dying tasks

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFCv5 PATCH 11/46] sched: Remove blocked load and utilization  contributions of dying tasks Peter Zijlstra <peterz@infradead.org> - 2015-08-11 13:40 +0200
    Re: [RFCv5 PATCH 11/46] sched: Remove blocked load and utilization  contributions of dying tasks Morten Rasmussen <morten.rasmussen@arm.com> - 2015-08-11 17:00 +0200
      Re: [RFCv5 PATCH 11/46] sched: Remove blocked load and utilization  contributions of dying tasks Peter Zijlstra <peterz@infradead.org> - 2015-08-11 19:30 +0200
        Re: [RFCv5 PATCH 11/46] sched: Remove blocked load and utilization  contributions of dying tasks Morten Rasmussen <morten.rasmussen@arm.com> - 2015-08-12 11:10 +0200

#1205075 — Re: [RFCv5 PATCH 11/46] sched: Remove blocked load and utilization contributions of dying tasks

On Tue, Jul 07, 2015 at 07:23:54PM +0100, Morten Rasmussen wrote:
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 775b0c7..fa12ce5 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -3217,6 +3217,8 @@ dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
>  	 * Update run-time statistics of the 'current'.
>  	 */
>  	update_curr(cfs_rq);
> +	if (entity_is_task(se) && task_of(se)->state == TASK_DEAD)
> +		flags &= !DEQUEUE_SLEEP;
>  	dequeue_entity_load_avg(cfs_rq, se, flags & DEQUEUE_SLEEP);
>  
>  	update_stats_dequeue(cfs_rq, se);

I know this is entirely redundant at this point (we took Yuyang's
patches), but this is the wrong way to go about doing this.

You add extra code the hot dequeue path for something that 'never'
happens. We have the sched_class::task_dead call for that.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1205200

On Tue, Aug 11, 2015 at 01:39:27PM +0200, Peter Zijlstra wrote:
> On Tue, Jul 07, 2015 at 07:23:54PM +0100, Morten Rasmussen wrote:
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 775b0c7..fa12ce5 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -3217,6 +3217,8 @@ dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
> >  	 * Update run-time statistics of the 'current'.
> >  	 */
> >  	update_curr(cfs_rq);
> > +	if (entity_is_task(se) && task_of(se)->state == TASK_DEAD)
> > +		flags &= !DEQUEUE_SLEEP;
> >  	dequeue_entity_load_avg(cfs_rq, se, flags & DEQUEUE_SLEEP);
> >  
> >  	update_stats_dequeue(cfs_rq, se);
> 
> I know this is entirely redundant at this point (we took Yuyang's
> patches), but this is the wrong way to go about doing this.

Yes, I'm still working my way through Yuyang's changes.

> You add extra code the hot dequeue path for something that 'never'
> happens. We have the sched_class::task_dead call for that.

I don't mind using sched_class::task_dead() instead. The reason why I
didn't go that way is that we have to retake the rq->lock or mess with
cfs_rq::removed_load instead of just not adding the utilization in
the first place when we have the rq->lock.

Anyway, it is probably redundant by now. I will check Yuyang's code to
see if he already fixed this problem.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1205307

On Tue, Aug 11, 2015 at 03:58:48PM +0100, Morten Rasmussen wrote:
> On Tue, Aug 11, 2015 at 01:39:27PM +0200, Peter Zijlstra wrote:

> > You add extra code the hot dequeue path for something that 'never'
> > happens. We have the sched_class::task_dead call for that.
> 
> I don't mind using sched_class::task_dead() instead. The reason why I
> didn't go that way is that we have to retake the rq->lock or mess with
> cfs_rq::removed_load instead of just not adding the utilization in
> the first place when we have the rq->lock.
> 
> Anyway, it is probably redundant by now. I will check Yuyang's code to
> see if he already fixed this problem.

He did, he used the removed_load stuff, same as migration does.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1205738

On Tue, Aug 11, 2015 at 07:23:44PM +0200, Peter Zijlstra wrote:
> On Tue, Aug 11, 2015 at 03:58:48PM +0100, Morten Rasmussen wrote:
> > On Tue, Aug 11, 2015 at 01:39:27PM +0200, Peter Zijlstra wrote:
> 
> > > You add extra code the hot dequeue path for something that 'never'
> > > happens. We have the sched_class::task_dead call for that.
> > 
> > I don't mind using sched_class::task_dead() instead. The reason why I
> > didn't go that way is that we have to retake the rq->lock or mess with
> > cfs_rq::removed_load instead of just not adding the utilization in
> > the first place when we have the rq->lock.
> > 
> > Anyway, it is probably redundant by now. I will check Yuyang's code to
> > see if he already fixed this problem.
> 
> He did, he used the removed_load stuff, same as migration does.

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web