From patchwork Thu Oct 29 02:38:38 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 55740 Delivered-To: patch@linaro.org Received: by 10.112.61.134 with SMTP id p6csp285283lbr; Wed, 28 Oct 2015 19:39:12 -0700 (PDT) X-Received: by 10.68.130.74 with SMTP id oc10mr38472144pbb.159.1446086352625; Wed, 28 Oct 2015 19:39:12 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id io6si74776448pbc.166.2015.10.28.19.39.12; Wed, 28 Oct 2015 19:39:12 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-pm-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-pm-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-pm-owner@vger.kernel.org; dkim=neutral (body hash did not verify) header.i=@linaro_org.20150623.gappssmtp.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756178AbbJ2CjL (ORCPT + 11 others); Wed, 28 Oct 2015 22:39:11 -0400 Received: from mail-pa0-f50.google.com ([209.85.220.50]:36074 "EHLO mail-pa0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751893AbbJ2CjK (ORCPT ); Wed, 28 Oct 2015 22:39:10 -0400 Received: by pacfv9 with SMTP id fv9so26052872pac.3 for ; Wed, 28 Oct 2015 19:39:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro_org.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id; bh=zdThFoL6/jusk8bDRC6H+GThWbH7zRQDK/utCVePw5E=; b=IiIM984+iJsg0e0aB1yfPGm6oDatZvr6Di5yIpR5wLvRtSuQuoc+Kl0SQxJB779Nuw fb5l7bBH53IMeqXiRcoi2oTOgh5VrGIQySStEZC5tRKfeDjSyC/cnFj3c5SK1gIKXdul SJMTzGsIiUdI4Zgy+y35Exl9HVA9hPme9RIJ648NPbh5TyFjQv+8SKEEA4mufKY3hHyT SlR4b4GFzrrCVMXMDVveaAqE3s5cvxkwKN6JWydNaBr2GtwBnuOwkE3l+uwg/HcVbQpZ 6i4YxXoyTt1yXlMD/BWTGi0bh3HUTT4suna01jT9s8Eh8Y1f/1YDh9bTxd40HHOK5ymP on1A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=zdThFoL6/jusk8bDRC6H+GThWbH7zRQDK/utCVePw5E=; b=j4W47GKejEYinFCelU6BvkqY7W+zasLxxBzBfM3x2v1QBtqafybFfrctgZHg4dRArk 7tXgqb+2d/BZo7HOfSV6m1Oaf+aXVnLBxoMROEHWCVNgaFAaGOL0uQd8gfR5JjLWYFH/ k14izP//MCkbBgRUVUTZI4G944hTIfQ0b/pf5gKWIGXHT1Tt2QNKbPuFnRe6Fs1N/uPb O1cwBKNHpR2P7Yp0HBLFvwXlta6tngFoui3YVi68yZ77sxdy7It7xm+B86WaiHsNiDr2 wnzlbcEydQHKmily8kzwhAn56TZbzVOeGPKsWpJ27kiAkKvtxuq5N1iFeSRlheLwNJdy SW3w== X-Gm-Message-State: ALoCoQnqP6UpFvCt0bCXEFvSXTeW/q7sM2WXIcfOcPh2WVpYYXDZBOvqxnjFcspg0NN4CF+IAulz X-Received: by 10.66.188.49 with SMTP id fx17mr37869998pac.95.1446086349353; Wed, 28 Oct 2015 19:39:09 -0700 (PDT) Received: from localhost ([122.172.111.169]) by smtp.gmail.com with ESMTPSA id ou1sm42168046pbc.7.2015.10.28.19.39.07 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Wed, 28 Oct 2015 19:39:08 -0700 (PDT) From: Viresh Kumar To: Rafael Wysocki Cc: linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org, Viresh Kumar , linux-kernel@vger.kernel.org (open list) Subject: [PATCH V4] cpufreq: governor: Quit work-handlers early if governor is stopped Date: Thu, 29 Oct 2015 08:08:38 +0530 Message-Id: <133510f34e89a9be81766bea3c7ddc1605198ee6.1446086230.git.viresh.kumar@linaro.org> X-Mailer: git-send-email 2.6.2.198.g614a2ac Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org gov_queue_work() acquires cpufreq_governor_lock to allow cpufreq_governor_stop() to drain delayed work items possibly scheduled on CPUs that share the policy with a CPU being taken offline. However, the same goal may be achieved in a more straightforward way if the policy pointer in the struct cpu_dbs_info matching the policy CPU is reset upfront by cpufreq_governor_stop() under the timer_mutex belonging to it and checked against NULL, under the same lock, at the beginning of dbs_timer(). In that case every instance of dbs_timer() run for a struct cpu_dbs_info sharing the policy pointer in question after cpufreq_governor_stop() has started will notice that that pointer is NULL and bail out immediately without queuing up any new work items. In turn, gov_cancel_work() called by cpufreq_governor_stop() before destroying timer_mutex will wait for all of the delayed work items currently running on the CPUs sharing the policy to drop the mutex, so it may be destroyed safely. Make cpufreq_governor_stop() and dbs_timer() work as described and modify gov_queue_work() so it does not acquire cpufreq_governor_lock any more. Signed-off-by: Viresh Kumar --- @Rafael: Please apply this patch alone for now, let me work out with the timer/work thing first before applying other patches. V3->V4: - Updated changelog as suggested by Rafael. drivers/cpufreq/cpufreq_governor.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) -- 2.6.2.198.g614a2ac -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c index 11258c4c1b17..b260576ddb12 100644 --- a/drivers/cpufreq/cpufreq_governor.c +++ b/drivers/cpufreq/cpufreq_governor.c @@ -171,10 +171,6 @@ void gov_queue_work(struct dbs_data *dbs_data, struct cpufreq_policy *policy, { int i; - mutex_lock(&cpufreq_governor_lock); - if (!policy->governor_enabled) - goto out_unlock; - if (!all_cpus) { /* * Use raw_smp_processor_id() to avoid preemptible warnings. @@ -188,9 +184,6 @@ void gov_queue_work(struct dbs_data *dbs_data, struct cpufreq_policy *policy, for_each_cpu(i, policy->cpus) __gov_queue_work(i, dbs_data, delay); } - -out_unlock: - mutex_unlock(&cpufreq_governor_lock); } EXPORT_SYMBOL_GPL(gov_queue_work); @@ -229,13 +222,24 @@ static void dbs_timer(struct work_struct *work) struct cpu_dbs_info *cdbs = container_of(work, struct cpu_dbs_info, dwork.work); struct cpu_common_dbs_info *shared = cdbs->shared; - struct cpufreq_policy *policy = shared->policy; - struct dbs_data *dbs_data = policy->governor_data; + struct cpufreq_policy *policy; + struct dbs_data *dbs_data; unsigned int sampling_rate, delay; bool modify_all = true; mutex_lock(&shared->timer_mutex); + policy = shared->policy; + + /* + * Governor might already be disabled and there is no point continuing + * with the work-handler. + */ + if (!policy) + goto unlock; + + dbs_data = policy->governor_data; + if (dbs_data->cdata->governor == GOV_CONSERVATIVE) { struct cs_dbs_tuners *cs_tuners = dbs_data->tuners; @@ -252,6 +256,7 @@ static void dbs_timer(struct work_struct *work) delay = dbs_data->cdata->gov_dbs_timer(cdbs, dbs_data, modify_all); gov_queue_work(dbs_data, policy, delay, modify_all); +unlock: mutex_unlock(&shared->timer_mutex); } @@ -478,9 +483,17 @@ static int cpufreq_governor_stop(struct cpufreq_policy *policy, if (!shared || !shared->policy) return -EBUSY; + /* + * Work-handler must see this updated, as it should not proceed any + * further after governor is disabled. And so timer_mutex is taken while + * updating this value. + */ + mutex_lock(&shared->timer_mutex); + shared->policy = NULL; + mutex_unlock(&shared->timer_mutex); + gov_cancel_work(dbs_data, policy); - shared->policy = NULL; mutex_destroy(&shared->timer_mutex); return 0; }