Received: by 2002:a4a:311b:0:0:0:0:0 with SMTP id k27-v6csp2471344ooa; Thu, 16 Aug 2018 12:18:08 -0700 (PDT) X-Google-Smtp-Source: AA+uWPzzmt27hzuBGjGNVH9zo6BvvKR1Lu6i26EzTdaRbal09+lWdPmj3mQb+3KIdNqe8YsfgHUx X-Received: by 2002:a63:ef4f:: with SMTP id c15-v6mr503680pgk.368.1534447088748; Thu, 16 Aug 2018 12:18:08 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1534447088; cv=none; d=google.com; s=arc-20160816; b=fjNZGxEbH/8EdkIvYaZp4mG+vXKUp02UbqreZ0XEo5/ZKPaRLyAiKg+9crDAeYlW/n kvgwyaQ3lOFPV/x+T6+Rp3J+IzqtyIg1C6o2e751eIyYBH9u2UZVZVuK2HqZ4cjZwTcW twRipjILHRz6yz2NQMEeDT4SnATDtHqQ2ccknUHFAWngljFd7E40/rFP2TZ8+/w0L2rF ub2Zp3XvIVAlLc/zmznOw+N8PMxavEqxYIv9UaiuV+ZteQNucIbiWTc5Q6lNPuxYJLZn 3WRLHbO7npwk4yf8AEjqsJJZlsP2qSF+W8BvyQ4FPoJHjq9cX2+ThfSeab9Le8XwyPoJ 5YWw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date:arc-authentication-results; bh=urBVgM6537x85/EMmLhMRDdTL9vZXMSB/DqtRlPpoE8=; b=zlELQVPM3G09Ip6jQZ7lx+4wdfEvPoVt2wUgwl4GY5c+/7PUQSS9vcqVbxyCx7l/R3 xBB/xgg2fry0UutKkitvTKH702gbTC1CSirZYh7JU+9h5LPySMUkwUq0/xFRWtfInbIX CbWSnlh497cKrGQNvysCclyMYXLi9fsimuA/zs6+0Wsp5z3yo5+j6+4YYq9TZYkRXfCa LiF1Ag1qWQq3Kh71HJft0ZAM9k7HDu2U1Sa0pIi/dbD9hlThGStuY7nM9WUhGy1lHLK7 JqusLobDKrhGIXx6j1A/mvcSSGWUp0FpZvaBvIJjukdm5SwlFDiSZWy3QTEZyLLWigSe 8h7g== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id l190-v6si26062pgd.626.2018.08.16.12.17.53; Thu, 16 Aug 2018 12:18:08 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2391645AbeHPQVd (ORCPT + 99 others); Thu, 16 Aug 2018 12:21:33 -0400 Received: from foss.arm.com ([217.140.101.70]:36676 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727244AbeHPQVd (ORCPT ); Thu, 16 Aug 2018 12:21:33 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id BF67D80D; Thu, 16 Aug 2018 06:22:57 -0700 (PDT) Received: from e110439-lin (e110439-lin.Emea.Arm.com [10.4.12.126]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 2036F3F5BC; Thu, 16 Aug 2018 06:22:54 -0700 (PDT) Date: Thu, 16 Aug 2018 14:22:49 +0100 From: Patrick Bellasi To: Dietmar Eggemann Cc: linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, Ingo Molnar , Peter Zijlstra , Tejun Heo , "Rafael J . Wysocki" , Viresh Kumar , Vincent Guittot , Paul Turner , Morten Rasmussen , Juri Lelli , Todd Kjos , Joel Fernandes , Steve Muckle , Suren Baghdasaryan Subject: Re: [PATCH v3 04/14] sched/core: uclamp: update CPU's refcount on clamp changes Message-ID: <20180816132249.GA2960@e110439-lin> References: <20180806163946.28380-1-patrick.bellasi@arm.com> <20180806163946.28380-5-patrick.bellasi@arm.com> <4023267d-be06-c599-136e-1f70bf376d5d@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4023267d-be06-c599-136e-1f70bf376d5d@arm.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 15-Aug 17:02, Dietmar Eggemann wrote: > On 08/06/2018 06:39 PM, Patrick Bellasi wrote: > > [...] > > >+/** > >+ * uclamp_task_active: check if a task is currently clamping a CPU > >+ * @p: the task to check > >+ * > >+ * A task actively affects the utilization clamp of a CPU if: > >+ * - it's currently enqueued or running on that CPU > >+ * - it's refcounted in at least one clamp group of that CPU > >+ * > >+ * Return: true if p is currently clamping the utilization of its CPU. > >+ */ > >+static inline bool uclamp_task_active(struct task_struct *p) > >+{ > >+ struct rq *rq = task_rq(p); > >+ int clamp_id; > >+ > >+ lockdep_assert_held(&p->pi_lock); > >+ lockdep_assert_held(&rq->lock); > >+ > >+ if (!task_on_rq_queued(p) && !p->on_cpu) > >+ return false; > >+ > >+ for (clamp_id = 0; clamp_id < UCLAMP_CNT; ++clamp_id) { > >+ if (uclamp_task_affects(p, clamp_id)) > >+ return true; > >+ } > >+ > >+ return false; > >+} > > Looks like that uclamp_task_active() is only used once (in > uclamp_task_update_active()). Can you not code the if condition and the for > loop directly in uclamp_task_update_active()? This would save code > (lockdep_assert_held() etc.) and comment lines. Yes, that's possible... and we will have: ---8<--- diff --git a/kernel/sched/core.c b/kernel/sched/core.c index ae528a7b9bef..0c8bec892018 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -1331,7 +1331,9 @@ uclamp_task_update_active(struct task_struct *p, int clamp_id, int group_id) * index, then that task is not yet RUNNABLE and it's going to be * enqueued with the proper clamp group value. */ - if (!uclamp_task_active(p)) + if (!task_on_rq_queued(p) && !p->on_cpu) + goto done; + if (!uclamp_task_affects(p, clamp_id)) goto done; /* Release p's currently referenced clamp group */ diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h index 11d139faed1f..65fdf4abc6ff 100644 --- a/kernel/sched/sched.h +++ b/kernel/sched/sched.h @@ -2231,35 +2231,6 @@ static inline bool uclamp_task_affects(struct task_struct *p, int clamp_id) { return (p->uclamp[clamp_id].effective.group_id != UCLAMP_NOT_VALID); } - -/** - * uclamp_task_active: check if a task is currently clamping a CPU - * @p: the task to check - * - * A task actively affects the utilization clamp of a CPU if: - * - it's currently enqueued or running on that CPU - * - it's refcounted in at least one clamp group of that CPU - * - * Return: true if p is currently clamping the utilization of its CPU. - */ -static inline bool uclamp_task_active(struct task_struct *p) -{ - struct rq *rq = task_rq(p); - int clamp_id; - - lockdep_assert_held(&p->pi_lock); - lockdep_assert_held(&rq->lock); - - if (!task_on_rq_queued(p) && !p->on_cpu) - return false; - - for (clamp_id = 0; clamp_id < UCLAMP_CNT; ++clamp_id) { - if (uclamp_task_affects(p, clamp_id)) - return true; - } - - return false; -} #endif /* CONFIG_UCLAMP_TASK */ #ifdef CONFIG_CPU_FREQ ---8<--- I'll add this to the next posting! Cheers Patrick -- #include Patrick Bellasi