From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753164AbbBFK6r (ORCPT ); Fri, 6 Feb 2015 05:58:47 -0500 Received: from casper.infradead.org ([85.118.1.10]:33195 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752226AbbBFK6q (ORCPT ); Fri, 6 Feb 2015 05:58:46 -0500 Date: Fri, 6 Feb 2015 11:58:40 +0100 From: Peter Zijlstra To: Zefan Li Cc: Ingo Molnar , Mike Galbraith , LKML , Stefan Bader Subject: Re: [PATCH] sched, autogroup: Fix failure when writing to cpu.rt_runtime_us Message-ID: <20150206105840.GJ23123@twins.programming.kicks-ass.net> References: <54D32AD4.1060003@huawei.com> <20150205142527.GI5029@twins.programming.kicks-ass.net> <54D4194B.2040808@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <54D4194B.2040808@huawei.com> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Feb 06, 2015 at 09:30:51AM +0800, Zefan Li wrote: > After running the test program, we have: > > root > | > autogroup <-- the RT test program is now here > > root.rt_runtime = 950000 > autogroup.rt_runtime = 0 > > Now if you try to change the rt_runtime of the root cgroup, it returns > -EBUSY. > > That comes from: > > static int tg_rt_schedulable(struct task_group *tg, void *data) > ( > ... > if (rt_bandwidth_enabled() && !runtime && tg_has_rt_tasks(tg)) > return -EBUSY; > ... > } > > static inline int tg_has_rt_tasks(struct task_group *tg) > { > struct task_struct *g, *p; > > for_each_process_thread(g, p) { > if (rt_task(p) && task_group(p) == tg) > return 1; > } > > return 0; > } > > here tg == autogroup, p == the test program, and task_group(p) > returns autogroup, which is wrong. No its not. > See this commit: > > commit f44937718ce3b8360f72f6c68c9481712517a867 > Author: Mike Galbraith > Date: Thu Jan 13 04:54:50 2011 +0100 > > sched, autogroup: Fix CONFIG_RT_GROUP_SCHED sched_setscheduler() failure > ... > + /* > + * Autogroup RT tasks are redirected to the root task group > ... > + * the policy change to proceed. Thereafter, task_group() > + * returns &root_task_group, ... > + */ That comment is misleading; if you look at the actual code what we do is redirect RT programs to _run_ in the root_task_group, but their task_group() should still be autogroup. Otherwise people could escape their cgroup by switching to and from a RT class. So what I think you want is something like the below; preferably with a comment on ;-) --- kernel/sched/core.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 1f37fe7f77a4..f4fd048ce7cf 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -7644,6 +7644,9 @@ static inline int tg_has_rt_tasks(struct task_group *tg) { struct task_struct *g, *p; + if (task_group_is_autogroup(tg)) + return 0; + for_each_process_thread(g, p) { if (rt_task(p) && task_group(p) == tg) return 1;