Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753987AbbFSGBD (ORCPT ); Fri, 19 Jun 2015 02:01:03 -0400 Received: from mail-pa0-f41.google.com ([209.85.220.41]:36160 "EHLO mail-pa0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753795AbbFSGAl (ORCPT ); Fri, 19 Jun 2015 02:00:41 -0400 Date: Fri, 19 Jun 2015 14:00:38 +0800 From: Boqun Feng To: Yuyang Du Cc: mingo@kernel.org, peterz@infradead.org, linux-kernel@vger.kernel.org, pjt@google.com, bsegall@google.com, morten.rasmussen@arm.com, vincent.guittot@linaro.org, dietmar.eggemann@arm.com, arjan.van.de.ven@intel.com, len.brown@intel.com, rafael.j.wysocki@intel.com, fengguang.wu@intel.com, srikar@linux.vnet.ibm.com Subject: Re: [PATCH v8 2/4] sched: Rewrite runnable load and utilization average tracking Message-ID: <20150619060038.GA1240@fixme-laptop.cn.ibm.com> References: <1434396367-27979-1-git-send-email-yuyang.du@intel.com> <1434396367-27979-3-git-send-email-yuyang.du@intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="d6Gm4EdcadzBjdND" Content-Disposition: inline In-Reply-To: <1434396367-27979-3-git-send-email-yuyang.du@intel.com> User-Agent: Mutt/1.5.23+89 (0255b37be491) (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5147 Lines: 152 --d6Gm4EdcadzBjdND Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi Yuyang, On Tue, Jun 16, 2015 at 03:26:05AM +0800, Yuyang Du wrote: > @@ -5977,36 +5786,6 @@ static void attach_tasks(struct lb_env *env) > } > =20 > #ifdef CONFIG_FAIR_GROUP_SCHED > -/* > - * update tg->load_weight by folding this cpu's load_avg > - */ > -static void __update_blocked_averages_cpu(struct task_group *tg, int cpu) > -{ > - struct sched_entity *se =3D tg->se[cpu]; > - struct cfs_rq *cfs_rq =3D tg->cfs_rq[cpu]; > - > - /* throttled entities do not contribute to load */ > - if (throttled_hierarchy(cfs_rq)) > - return; > - > - update_cfs_rq_blocked_load(cfs_rq, 1); > - > - if (se) { > - update_entity_load_avg(se, 1); > - /* > - * We pivot on our runnable average having decayed to zero for > - * list removal. This generally implies that all our children > - * have also been removed (modulo rounding error or bandwidth > - * control); however, such cases are rare and we can fix these > - * at enqueue. > - * > - * TODO: fix up out-of-order children on enqueue. > - */ > - if (!se->avg.runnable_avg_sum && !cfs_rq->nr_running) > - list_del_leaf_cfs_rq(cfs_rq); > - } > -} > - > static void update_blocked_averages(int cpu) > { > struct rq *rq =3D cpu_rq(cpu); > @@ -6015,17 +5794,17 @@ static void update_blocked_averages(int cpu) > =20 > raw_spin_lock_irqsave(&rq->lock, flags); > update_rq_clock(rq); > + > /* > * Iterates the task_group tree in a bottom up fashion, see > * list_add_leaf_cfs_rq() for details. > */ > for_each_leaf_cfs_rq(rq, cfs_rq) { > - /* > - * Note: We may want to consider periodically releasing > - * rq->lock about these updates so that creating many task > - * groups does not result in continually extending hold time. > - */ > - __update_blocked_averages_cpu(cfs_rq->tg, rq->cpu); > + /* throttled entities do not contribute to load */ > + if (throttled_hierarchy(cfs_rq)) > + continue; > + > + update_cfs_rq_load_avg(cfs_rq_clock_task(cfs_rq), cfs_rq); We iterates task_group tree(actually the corresponding cfs_rq tree on that cpu), because we want to do a bottom-up update. And we want a bottom-up update, because we want to update the weigthed_cpuload(). __update_blocked_averages_cpu(tg, cpu) does three things: Let's say: cfs_rq =3D tg->cfs_rq[cpu] se =3D tg->cfs_rq[cpu] pcfs_rq =3D cfs_rq_of(se) ,which is the parent of cfs_rq. 1. update cfs_rq->blocked_load_avg, and its contrib to its task group. 2. update se->avg and calculate the deltas of se->avg.*_avg_contrib. 3. update pcfs_rq->*_load_avg with the deltas in step 2 In this way, __update_blocked_averages_cpu(tg, cpu) can contributes tg's load changes to its parent. So that update_blocked_averages() can aggregate all load changes to weighted_cpuload(). However, update_cfs_rq_load_avg() only updates cfs_rq->avg, the change won't be contributed or aggregated to cfs_rq's parent in the for_each_leaf_cfs_rq loop, therefore that's actually not a bottom-up update. To fix this, I think we can add a update_cfs_shares(cfs_rq) after update_cfs_rq_load_avg(). Like: for_each_leaf_cfs_rq(rq, cfs_rq) { - /* - * Note: We may want to consider periodically releasing - * rq->lock about these updates so that creating many task - * groups does not result in continually extending hold time. - */ - __update_blocked_averages_cpu(cfs_rq->tg, rq->cpu); + /* throttled entities do not contribute to load */ + if (throttled_hierarchy(cfs_rq)) + continue; + + update_cfs_rq_load_avg(cfs_rq_clock_task(cfs_rq), cfs_rq); + update_cfs_share(cfs_rq); } However, I think update_cfs_share isn't cheap, because it may do a bottom-up update once called. So how about just update the root cfs_rq? Like: - /* - * Iterates the task_group tree in a bottom up fashion, see - * list_add_leaf_cfs_rq() for details. - */ - for_each_leaf_cfs_rq(rq, cfs_rq) { - /* - * Note: We may want to consider periodically releasing - * rq->lock about these updates so that creating many task - * groups does not result in continually extending hold time. - */ - __update_blocked_averages_cpu(cfs_rq->tg, rq->cpu); - } + update_cfs_rq_load_avg(rq_clock_task(rq), rq->cfs_rq); Thanks and Best Regards, Boqun --d6Gm4EdcadzBjdND Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAABCAAGBQJVg7ACAAoJEEl56MO1B/q4RQUH/11l3YUGd3+nUvVUAuPGlov1 CJMmXhWxp/cMNvExdy1TDsMpVY2LIHItQRKgTn7O4+EHx3HwWO9UzuIip9IGiwxn x+fdZ+am9Vog3CHNqe6+Rc3eaGnqMSAtBA7lN/SMzVIcsv8EHYs8LG2AnMPV46iR K7DkPjxs4mJOH3HA11+95ymyNKXLJL7dw9kM3CxOWXcCEpL4zI+CROxcR0MBnMWl U4XG0UxOiJv/19DvnSvBrv6gDrB07Lnx/LYkDVlQiZoQ6ay2VEUOEDGetMkYizGV xvAlZV1vwp/Kc+Ng9Emu+LLWMPmQpcGdh/91S5tnp+uxu1MsNVbrRM1VTN/cCAs= =12v4 -----END PGP SIGNATURE----- --d6Gm4EdcadzBjdND-- -- 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/