[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 6 Jun 2016 11:52:29 +0100
From: Dietmar Eggemann <dietmar.eggemann@....com>
To: Vincent Guittot <vincent.guittot@...aro.org>, peterz@...radead.org,
mingo@...nel.org, linux-kernel@...r.kernel.org, pjt@...gle.com
Cc: yuyang.du@...el.com, bsegall@...gle.com, Morten.Rasmussen@....com
Subject: Re: [RFC PATCH v2] sched: reflect sched_entity movement into
task_group's utilization
On 24/05/16 10:55, Vincent Guittot wrote:
[...]
> +/* Take into account the change of the utilization of a child task group */
> +static void update_tg_cfs_util(struct sched_entity *se, int blocked)
> +{
> + int delta;
> + struct cfs_rq *cfs_rq;
> + long update_util_avg;
> + long last_update_time;
> + long old_util_avg;
> +
> +
> + /*
> + * update_blocked_average will call this function for root cfs_rq
> + * whose se is null. In this case just return
> + */
> + if (!se)
> + return;
> +
> + if (entity_is_task(se))
> + return 0;
void function
> +
> + /* Get sched_entity of cfs rq */
You mean /* Get cfs_rq owned by this task group */?
> + cfs_rq = group_cfs_rq(se);
> +
> + update_util_avg = cfs_rq->diff_util_avg;
> +
> + if (!update_util_avg)
> + return 0;
Couldn't you not just get rid of long update_util_avg and only use
cfs_rq->diff_util_avg here (clearing pending changes after you set
se->avg.util_avg)?
> +
> + /* Clear pending changes */
> + cfs_rq->diff_util_avg = 0;
> +
> + /* Add changes in sched_entity utilizaton */
> + old_util_avg = se->avg.util_avg;
> + se->avg.util_avg = max_t(long, se->avg.util_avg + update_util_avg, 0);
> + se->avg.util_sum = se->avg.util_avg * LOAD_AVG_MAX;
> +
> + /* Get parent cfs_rq */
> + cfs_rq = cfs_rq_of(se);
> +
> + if (blocked) {
> + /*
> + * blocked utilization has to be synchronized with its parent
> + * cfs_rq's timestamp
> + */
We don't have stand-alone blocked utilization any more although the
function is still called update_blocked_averages(). Do you need this for
cpu's going through idle periods?
It's also necessary because there could be other se's which could have
been [en|de]queued at/from this cfs_rq so its last_update_time value is
more recent?
[...]
Powered by blists - more mailing lists