From patchwork Tue Sep 9 04:16:39 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 37020 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-pa0-f71.google.com (mail-pa0-f71.google.com [209.85.220.71]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 1ADE32054D for ; Tue, 9 Sep 2014 04:17:00 +0000 (UTC) Received: by mail-pa0-f71.google.com with SMTP id rd3sf20239129pab.2 for ; Mon, 08 Sep 2014 21:16:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:from:to:cc:subject:date:message-id :mime-version:sender:precedence:list-id:x-original-sender :x-original-authentication-results:mailing-list:list-post:list-help :list-archive:list-unsubscribe:content-type :content-transfer-encoding; bh=s9TJRxIPu6zRhXo/NWN1DQFK3mLJ4FuiUZnghn9SaWo=; b=DixJxQnfLbxvNhG4x6vcvIleEKl2UumX1AGDOTSfPqRLsIZnvRiD3zuyNtSdwI0oPe P4gyGZmpz9Elc7OF459od064Tn9fr23ZyMS4Z5tyfX5DzgsWHZcntj/StrJ9uQm3Af1l AxqWFG36ooZK7OoECcgwCQ3aYh07qI6v22Q10a/XL8rZKTpMU1WNxDzf67ROD5OP8bEi SFaZvvg8KMR9hL+yeByMI2nQZcsrXrZd700JlMiDtbMaeiqALdwKT1ju9D8dul7d2rK1 itvDtyW73EdpikjWDssbZRd46jzEHDCFJONt863JMMRRterS2h8Wctlzk1zdxCS2eS/x o+Jg== X-Gm-Message-State: ALoCoQnTjCUMq88yJoGf8IdgKac+RBLNsAWEP62UuxaPiCwVjbdIgIBVWKvM+cDCtmfrPUBPsGba X-Received: by 10.66.124.136 with SMTP id mi8mr20300694pab.15.1410236219379; Mon, 08 Sep 2014 21:16:59 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.34.206 with SMTP id l72ls1933036qgl.46.gmail; Mon, 08 Sep 2014 21:16:59 -0700 (PDT) X-Received: by 10.52.245.66 with SMTP id xm2mr11393411vdc.36.1410236219264; Mon, 08 Sep 2014 21:16:59 -0700 (PDT) Received: from mail-vc0-f177.google.com (mail-vc0-f177.google.com [209.85.220.177]) by mx.google.com with ESMTPS id x5si3255447vdv.2.2014.09.08.21.16.59 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 08 Sep 2014 21:16:59 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.177 as permitted sender) client-ip=209.85.220.177; Received: by mail-vc0-f177.google.com with SMTP id hq11so16161424vcb.22 for ; Mon, 08 Sep 2014 21:16:59 -0700 (PDT) X-Received: by 10.52.33.231 with SMTP id u7mr3257456vdi.54.1410236219138; Mon, 08 Sep 2014 21:16:59 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.221.45.67 with SMTP id uj3csp226811vcb; Mon, 8 Sep 2014 21:16:58 -0700 (PDT) X-Received: by 10.68.183.68 with SMTP id ek4mr26445698pbc.54.1410236218174; Mon, 08 Sep 2014 21:16:58 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id sn9si20943362pac.108.2014.09.08.21.16.57 for ; Mon, 08 Sep 2014 21:16:58 -0700 (PDT) Received-SPF: none (google.com: linux-pm-owner@vger.kernel.org does not designate permitted sender hosts) client-ip=209.132.180.67; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751077AbaIIEQ4 (ORCPT + 15 others); Tue, 9 Sep 2014 00:16:56 -0400 Received: from mail-pa0-f45.google.com ([209.85.220.45]:46747 "EHLO mail-pa0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750879AbaIIEQz (ORCPT ); Tue, 9 Sep 2014 00:16:55 -0400 Received: by mail-pa0-f45.google.com with SMTP id rd3so3892310pab.32 for ; Mon, 08 Sep 2014 21:16:55 -0700 (PDT) X-Received: by 10.70.38.202 with SMTP id i10mr1148600pdk.169.1410236215081; Mon, 08 Sep 2014 21:16:55 -0700 (PDT) Received: from localhost ([122.167.129.166]) by mx.google.com with ESMTPSA id cb2sm10192218pbb.34.2014.09.08.21.16.53 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Mon, 08 Sep 2014 21:16:54 -0700 (PDT) From: Viresh Kumar To: Rafael Wysocki Cc: linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org, robert.schoene@tu-dresden.de, prarit@redhat.com, skannan@codeaurora.org, Viresh Kumar Subject: [PATCH 1/2] cpufreq: serialize calls to __cpufreq_governor() Date: Tue, 9 Sep 2014 09:46:39 +0530 Message-Id: X-Mailer: git-send-email 2.0.3.693.g996b0fd MIME-Version: 1.0 Sender: linux-pm-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: linux-pm@vger.kernel.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: viresh.kumar@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.177 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , This commit was earlier commited in kernel as: 19c7630 cpufreq: serialize calls to __cpufreq_governor() and was later reverted by Srivatsa: 56d07db cpufreq: Remove temporary fix for race between CPU hotplug and sysfs-writes When this commit was first introduced it was written for races during hotplug and because we got some other solution to take care of the races with hotplug we reverted it. But (as I also said in the revert patch: https://lkml.org/lkml/2013/9/10/61) there are more cases where this is required. Recently Robert shown an instance where changing governors with multiple threads leads to following warnings: ------------[ cut here ]------------ WARNING: CPU: 1 PID: 2458 at drivers/cpufreq/cpufreq_governor.c:261 cpufreq_governor_dbs+0x6d2/0x740() CPU: 1 PID: 2458 Comm: tee Tainted: G OE 3.16.0-rc6+ #1 Hardware name: FUJITSU ESPRIMO P700/D3061-A1, BIOS V4.6.4.0 R1.12.0 for D3061-A1x 07/04/2011 0000000000000009 ffff8800ae403b78 ffffffff8173b0bf 0000000000000000 ffff8800ae403bb0 ffffffff8106c82d 0000000000000000 ffff88022fa27000 0000000000000005 0000000000000002 ffffffff81cd5d00 ffff8800ae403bc0 Call Trace: [] dump_stack+0x45/0x56 [] warn_slowpath_common+0x7d/0xa0 [] warn_slowpath_null+0x1a/0x20 [] cpufreq_governor_dbs+0x6d2/0x740 [] ? notifier_call_chain+0x4c/0x70 [] od_cpufreq_governor_dbs+0x17/0x20 [] __cpufreq_governor+0xb0/0x2a0 [] cpufreq_set_policy+0x14c/0x2f0 [] store_scaling_governor+0x96/0xf0 [] ? cpufreq_update_policy+0x1d0/0x1d0 [] store+0x79/0xc0 [] sysfs_kf_write+0x3d/0x50 [] kernfs_fop_write+0xe0/0x160 [] vfs_write+0xb7/0x1f0 [] SyS_write+0x46/0xb0 [] tracesys+0xe1/0xe6 ---[ end trace a2dad7e42b22c796 ]--- BUG: unable to handle kernel NULL pointer dereference at (null) IP: [] cpufreq_governor_dbs+0x55/0x740 PGD 36a05067 PUD b47df067 PMD 0 Oops: 0000 [#1] SMP Robert also provided a small script to reproduce it: crash_governor.sh: for I in `seq 1000` do echo ondemand | sudo tee /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor echo userspace | sudo tee /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor done runme.sh: for I in `seq 8` do ./crash_governor.sh & done Just run runme.sh to crash your system :) Introduce an additional variable which would guarantee serialization of __cpufreq_governor() routine. Reported-and-tested-by: Robert Schöne Signed-off-by: Viresh Kumar --- Hi Rafael, These fixes the issues reported by Robert. There is slight change after Robert tested my initial patch, 'bool' is replaced by 'int' for 'governor_state'. Regardingn stable trees, I am not too sure. The first patch of this series was earlier applied on 3.12 and then was reverted quickly in the same release. So, the best we can do is 3.12+. drivers/cpufreq/cpufreq.c | 7 ++++++- include/linux/cpufreq.h | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index d9fdedd..a7ceae3 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -2012,13 +2012,15 @@ static int __cpufreq_governor(struct cpufreq_policy *policy, policy->cpu, event); mutex_lock(&cpufreq_governor_lock); - if ((policy->governor_enabled && event == CPUFREQ_GOV_START) + if (policy->governor_busy + || (policy->governor_enabled && event == CPUFREQ_GOV_START) || (!policy->governor_enabled && (event == CPUFREQ_GOV_LIMITS || event == CPUFREQ_GOV_STOP))) { mutex_unlock(&cpufreq_governor_lock); return -EBUSY; } + policy->governor_busy = true; if (event == CPUFREQ_GOV_STOP) policy->governor_enabled = false; else if (event == CPUFREQ_GOV_START) @@ -2047,6 +2049,9 @@ static int __cpufreq_governor(struct cpufreq_policy *policy, ((event == CPUFREQ_GOV_POLICY_EXIT) && !ret)) module_put(policy->governor->owner); + mutex_lock(&cpufreq_governor_lock); + policy->governor_busy = false; + mutex_unlock(&cpufreq_governor_lock); return ret; } diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index 7d1955a..c7aa96b 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -82,6 +82,7 @@ struct cpufreq_policy { struct cpufreq_governor *governor; /* see below */ void *governor_data; bool governor_enabled; /* governor start/stop flag */ + bool governor_busy; struct work_struct update; /* if update_policy() needs to be * called, but you're in IRQ context */