Re: [PATCH v2 1/3] sched/fair: Clean up attach_entity_load_avg()

From	Peter Zijlstra <peterz@infradead.org>
Newsgroups	linux.kernel
Subject	Re: [PATCH v2 1/3] sched/fair: Clean up attach_entity_load_avg()
Date	2016-05-31 11:20 +0200
Message-ID	<rEO0a-NP-27@gated-at.bofh.it> (permalink)
References	<rELvj-7hB-3@gated-at.bofh.it> <rELvj-7hB-1@gated-at.bofh.it> <rENns-iy-29@gated-at.bofh.it> <rENx7-m9-23@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Tue, May 31, 2016 at 08:49:54AM +0800, Yuyang Du wrote:
> On Tue, May 31, 2016 at 10:34:02AM +0200, Peter Zijlstra wrote:
> > On Tue, May 31, 2016 at 06:32:54AM +0800, Yuyang Du wrote:
> > > +++ b/kernel/sched/fair.c
> > > @@ -2961,24 +2961,6 @@ static inline void update_load_avg(struct sched_entity *se, int update_tg)
> > >  
> > >  static void attach_entity_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se)
> > >  {
> > > -	if (!sched_feat(ATTACH_AGE_LOAD))
> > > -		goto skip_aging;
> > > -
> > > -	/*
> > > -	 * If we got migrated (either between CPUs or between cgroups) we'll
> > > -	 * have aged the average right before clearing @last_update_time.
> > > -	 */
> > > -	if (se->avg.last_update_time) {
> > > -		__update_load_avg(cfs_rq->avg.last_update_time, cpu_of(rq_of(cfs_rq)),
> > > -				  &se->avg, 0, 0, NULL);
> > > -
> > > -		/*
> > > -		 * XXX: we could have just aged the entire load away if we've been
> > > -		 * absent from the fair class for too long.
> > > -		 */
> > > -	}
> > > -
> > > -skip_aging:
> > >  	se->avg.last_update_time = cfs_rq->avg.last_update_time;
> > >  	cfs_rq->avg.load_avg += se->avg.load_avg;
> > >  	cfs_rq->avg.load_sum += se->avg.load_sum;
> > 
> > So I'm not a big fan of this patch; the aging is a conceptual part of
> > attaching the load, the fact that it only happens in one callsite is a
> > mere 'accident'.
> 
> Strictly in concept, it is part of load dealing, maybe not load attaching, :)

Well it deals with the time between detach and attach. Attach is the
obvious point to do that. If you move it elsewhere you run the risk of
forgetting it when the code changes etc..

Thread

[PATCH v2 1/3] sched/fair: Clean up attach_entity_load_avg() Yuyang Du <yuyang.du@intel.com> - 2016-05-31 08:40 +0200
  Re: [PATCH v2 1/3] sched/fair: Clean up attach_entity_load_avg() Peter Zijlstra <peterz@infradead.org> - 2016-05-31 10:40 +0200
    Re: [PATCH v2 1/3] sched/fair: Clean up attach_entity_load_avg() Yuyang Du <yuyang.du@intel.com> - 2016-05-31 10:50 +0200
      Re: [PATCH v2 1/3] sched/fair: Clean up attach_entity_load_avg() Peter Zijlstra <peterz@infradead.org> - 2016-05-31 11:20 +0200
    Re: [PATCH v2 1/3] sched/fair: Clean up attach_entity_load_avg() Yuyang Du <yuyang.du@intel.com> - 2016-06-01 04:40 +0200

csiph-web