From patchwork Thu Jun 4 10:17:06 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 49521 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-wg0-f69.google.com (mail-wg0-f69.google.com [74.125.82.69]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id CA8582154F for ; Thu, 4 Jun 2015 10:17:17 +0000 (UTC) Received: by wgv5 with SMTP id 5sf9100023wgv.0 for ; Thu, 04 Jun 2015 03:17:17 -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:date:from:to:cc:subject:message-id :references:mime-version:content-type:content-disposition :in-reply-to:user-agent:sender:precedence:list-id:x-original-sender :x-original-authentication-results:mailing-list:list-post:list-help :list-archive:list-unsubscribe; bh=o9OQ1uwg92IWnG22q3ROv9Li8TX4uXGdqEDMw5v2a9g=; b=ZMpOFjoFoKGf+zUI64x2+pEiZv/SxoeDU3nMyBD/nHmF/FC8hTyB594qAbu3U0ZcFh oA6w+Zp5wNgxOodrF44N5ICnzfXRYz3x356qQA3IrVrkRgime99EQOIVLhMLqP/zsKx1 nvrNePOn/F7ARBtwe78D0apWvHRbncqRQ1UbOoil88OTQwlc1OAFYXK2ur1e4QyMD1JN 10nEiLF6CTdqoQuBOPwkes3bGeb+fOblXeXJmfli4HodyDYOufigp1h7uezf18WNFbs1 zl3pW9IAVOgflMUwDVtJJZtZm6O+FEfIEjC0vUR4rmW4E1kvcuP5egweqthw7devz04X UuqA== X-Gm-Message-State: ALoCoQnoPtwHJ/cQmG+Ksw49TEcO7KLdN8i91XaOG33dL95vvfzDc7oU5WFYYpHznDAT3SfLsRVq X-Received: by 10.180.84.70 with SMTP id w6mr3145573wiy.4.1433413036975; Thu, 04 Jun 2015 03:17:16 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.19.231 with SMTP id i7ls169617lae.85.gmail; Thu, 04 Jun 2015 03:17:16 -0700 (PDT) X-Received: by 10.152.29.161 with SMTP id l1mr36277578lah.76.1433413036811; Thu, 04 Jun 2015 03:17:16 -0700 (PDT) Received: from mail-la0-f46.google.com (mail-la0-f46.google.com. [209.85.215.46]) by mx.google.com with ESMTPS id k11si1424244lbz.74.2015.06.04.03.17.16 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 04 Jun 2015 03:17:16 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.46 as permitted sender) client-ip=209.85.215.46; Received: by laei3 with SMTP id i3so28222811lae.3 for ; Thu, 04 Jun 2015 03:17:16 -0700 (PDT) X-Received: by 10.112.182.4 with SMTP id ea4mr31761552lbc.35.1433413036698; Thu, 04 Jun 2015 03:17:16 -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.112.108.230 with SMTP id hn6csp482905lbb; Thu, 4 Jun 2015 03:17:15 -0700 (PDT) X-Received: by 10.67.4.138 with SMTP id ce10mr68211168pad.102.1433413034803; Thu, 04 Jun 2015 03:17:14 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id z2si5224776pdj.39.2015.06.04.03.17.13; Thu, 04 Jun 2015 03:17:14 -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; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750928AbbFDKRN (ORCPT + 11 others); Thu, 4 Jun 2015 06:17:13 -0400 Received: from mail-pa0-f48.google.com ([209.85.220.48]:36267 "EHLO mail-pa0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751337AbbFDKRM (ORCPT ); Thu, 4 Jun 2015 06:17:12 -0400 Received: by pabqy3 with SMTP id qy3so26674444pab.3 for ; Thu, 04 Jun 2015 03:17:11 -0700 (PDT) X-Received: by 10.66.154.233 with SMTP id vr9mr67663450pab.124.1433413031544; Thu, 04 Jun 2015 03:17:11 -0700 (PDT) Received: from localhost ([122.167.219.251]) by mx.google.com with ESMTPSA id f1sm3370080pdp.24.2015.06.04.03.17.09 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Thu, 04 Jun 2015 03:17:10 -0700 (PDT) Date: Thu, 4 Jun 2015 15:47:06 +0530 From: Viresh Kumar To: Preeti U Murthy Cc: Rafael Wysocki , linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org, ego@linux.vnet.ibm.com, paulus@samba.org, shilpa.bhat@linux.vnet.ibm.com, prarit@redhat.com, robert.schoene@tu-dresden.de, skannan@codeaurora.org Subject: Re: [PATCH 2/3] cpufreq: governor: split cpufreq_governor_dbs() Message-ID: <20150604101706.GB743@linux> References: <6880bd7b6e6e7968f008d6328ab15353d99ccd57.1433326032.git.viresh.kumar@linaro.org> <5570229F.3080808@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <5570229F.3080808@linux.vnet.ibm.com> User-Agent: Mutt/1.5.21 (2010-09-15) 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.215.46 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: , On 04-06-15, 15:34, Preeti U Murthy wrote: > > + if (dbs_data) { > > + WARN_ON(have_governor_per_policy()); > > Shouldn't this be outside this loop ? We warn here and allocate dbs_dta Loop ? Its just an 'if' block :) > freshly in the current code for the case where governor is per policy. So what we are doing in the current code is equally disgusting. We already have a pointer and we overwrite it. > > > + dbs_data->usage_count++; > > Besides, in the case where a governor exists per policy, we will end up > incrementing the usage_count to more than 1 under this condition, which > does not make sense. So, the only sane option here is to return an error immediately I think. diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c index ed849a8777dd..57a39f8a92b7 100644 --- a/drivers/cpufreq/cpufreq_governor.c +++ b/drivers/cpufreq/cpufreq_governor.c @@ -247,7 +247,8 @@ static int cpufreq_governor_init(struct cpufreq_policy *policy, int ret; if (dbs_data) { - WARN_ON(have_governor_per_policy()); + if (WARN_ON(have_governor_per_policy())) + return -EINVAL; dbs_data->usage_count++; policy->governor_data = dbs_data; return 0; @@ -276,24 +277,28 @@ static int cpufreq_governor_init(struct cpufreq_policy *policy, latency * LATENCY_MULTIPLIER)); if (!have_governor_per_policy()) { - WARN_ON(cpufreq_get_global_kobject()); + if (WARN_ON(cpufreq_get_global_kobject())) { + ret = -EINVAL; + goto cdata_exit; + } cdata->gdbs_data = dbs_data; } ret = sysfs_create_group(get_governor_parent_kobj(policy), get_sysfs_attr(dbs_data)); if (ret) - goto cdata_exit; + goto put_kobj; policy->governor_data = dbs_data; return 0; -cdata_exit: +put_kobj: if (!have_governor_per_policy()) { cdata->gdbs_data = NULL; cpufreq_put_global_kobject(); } +cdata_exit: cdata->exit(dbs_data, !policy->governor->initialized); free_dbs_data: kfree(dbs_data);