From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754831AbcHZREu (ORCPT ); Fri, 26 Aug 2016 13:04:50 -0400 Received: from mail-pa0-f43.google.com ([209.85.220.43]:36533 "EHLO mail-pa0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751764AbcHZREs (ORCPT ); Fri, 26 Aug 2016 13:04:48 -0400 From: bsegall@google.com To: Jeehong Kim Cc: mingo@redhat.com, peterz@infradead.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org, ezjjilong@gmail.com Subject: Re: [PATCH] sched/fair: Fix that tasks are not constrained by cfs_b->quota on hotplug core, when hotplug core is offline and then online. References: <1472204439-3542-1-git-send-email-jhez.kim@samsung.com> Date: Fri, 26 Aug 2016 10:04:45 -0700 In-Reply-To: <1472204439-3542-1-git-send-email-jhez.kim@samsung.com> (Jeehong Kim's message of "Fri, 26 Aug 2016 18:40:39 +0900") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Jeehong Kim writes: > In case that CONFIG_HOTPLUG_CPU and CONFIG_CFS_BANDWIDTH is turned on and tasks in bandwidth controlled task group run on hotplug core, the tasks are not controlled by cfs_b->quota when hotplug core is offline and then online. The remaining tasks in task group consume all of cfs_b->quota on other cores. > > The cause of this problem is described as below; > > 1. When hotplug core is offline while tasks in task group run on hotplug core, unregister_fair_sched_group() deletes leaf_cfs_rq_list of tg->cfs_rq[cpu] from &rq_of(cfs_rq)->leaf_cfs_rq_list. > > 2. Then, when hotplug core is online, update_runtime_enabled() registers cfs_b->quota on cfs_rq->runtime_enabled of all leaf cfs_rq on runqueue. However, because this is before enqueue_entity() adds &cfs_rq->leaf_cfs_rq_list on &rq_of(cfs_rq)->leaf_cfs_rq_list, cfs->quota is not register on cfs_rq->runtime_enabled. > > To resolve this problem, this patch registers cfs_b->quota on cfs_rq->runtime_enabled after list_add_leaf_cfs_rq() for every enqueue_entity(). > > Signed-off-by: Jeehong Kim > --- > kernel/sched/fair.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > index 6488815..1f4b104 100644 > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -4246,9 +4246,16 @@ static void do_sched_cfs_slack_timer(struct cfs_bandwidth *cfs_b) > */ > static void check_enqueue_throttle(struct cfs_rq *cfs_rq) > { > + struct cfs_bandwidth *cfs_b = &cfs_rq->tg->cfs_bandwidth; > + > if (!cfs_bandwidth_used()) > return; > > + /* register cfs_b->quota */ > + raw_spin_lock(&cfs_b->lock); > + cfs_rq->runtime_enabled = cfs_b->quota != RUNTIME_INF; > + raw_spin_unlock(&cfs_b->lock); > + > /* an active group must be handled by the update_curr()->put() path */ > if (!cfs_rq->runtime_enabled || cfs_rq->curr) > return; > -- > 1.9.1 It would be much better to avoid taking the cfs_b lock on every enqueue. update_runtime_enabled could instead walk the whole tg tree, which while it would also hit tgs that have never run on this rq, would be sufficient (and probably not much more expensive).