From patchwork Wed Aug 12 08:23:02 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 52326 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-la0-f71.google.com (mail-la0-f71.google.com [209.85.215.71]) by patches.linaro.org (Postfix) with ESMTPS id 3C326218E7 for ; Wed, 12 Aug 2015 08:23:22 +0000 (UTC) Received: by labd1 with SMTP id d1sf3317143lab.0 for ; Wed, 12 Aug 2015 01:23:21 -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=q20EXhtSbE1HODpj1HFa+O7A6hK9EUOeVwL2T7XMlOA=; b=gllnepcMkooPa5MosYlOiJf/aYtYsw1OsKGJ/97DQ5ARDU99LMkhVTEvL9evKuW8gg NLfR1yG+Ls9zksRjEHdzavU/ST/NgWSHmDH5x75+XNN1EqU4WgvnmgRv9V8PbU8Um/jk L4V/rK1LlbYxG9YvXYlSgrwnnHcX0L69Y4gGRCSDUEo7N6uLOQkfV4FxBq1BqJMNf2rY Ux3RenvSOQWyhpjSGVRrRhFY6ATTbsXw8E8XvM/hNU/z354Mok6Bqb3HDy0JiqBR/uss BvQuxgOu/MpWQR4x8Q3pZlJo0Yy+r0NsccVmmKNwaas0peiIcfUbcpmS1CY/OjxBJ/98 hXBA== X-Gm-Message-State: ALoCoQmev4AOZZjSszkPiGrV1fzpAD8haQLukauFsusmVgc8FA/On31o4qOltCp4Ep+Ivx/MGq5b X-Received: by 10.180.89.104 with SMTP id bn8mr3233607wib.4.1439367801162; Wed, 12 Aug 2015 01:23:21 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.23.135 with SMTP id m7ls12116laf.103.gmail; Wed, 12 Aug 2015 01:23:20 -0700 (PDT) X-Received: by 10.152.23.167 with SMTP id n7mr31113062laf.108.1439367800777; Wed, 12 Aug 2015 01:23:20 -0700 (PDT) Received: from mail-la0-f54.google.com (mail-la0-f54.google.com. [209.85.215.54]) by mx.google.com with ESMTPS id h9si3590939lam.85.2015.08.12.01.23.20 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 12 Aug 2015 01:23:20 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.54 as permitted sender) client-ip=209.85.215.54; Received: by lahi9 with SMTP id i9so5038440lah.2 for ; Wed, 12 Aug 2015 01:23:20 -0700 (PDT) X-Received: by 10.112.166.2 with SMTP id zc2mr30640305lbb.29.1439367800233; Wed, 12 Aug 2015 01:23:20 -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.7.198 with SMTP id l6csp167698lba; Wed, 12 Aug 2015 01:23:19 -0700 (PDT) X-Received: by 10.67.8.40 with SMTP id dh8mr64195334pad.129.1439367798387; Wed, 12 Aug 2015 01:23:18 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id k12si8268023pbq.59.2015.08.12.01.23.17; Wed, 12 Aug 2015 01:23:18 -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; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934111AbbHLIXO (ORCPT + 28 others); Wed, 12 Aug 2015 04:23:14 -0400 Received: from mail-pa0-f48.google.com ([209.85.220.48]:34328 "EHLO mail-pa0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934026AbbHLIXH (ORCPT ); Wed, 12 Aug 2015 04:23:07 -0400 Received: by pawu10 with SMTP id u10so9783086paw.1 for ; Wed, 12 Aug 2015 01:23:07 -0700 (PDT) X-Received: by 10.66.63.9 with SMTP id c9mr64945820pas.40.1439367786963; Wed, 12 Aug 2015 01:23:06 -0700 (PDT) Received: from localhost ([122.171.186.190]) by smtp.gmail.com with ESMTPSA id f10sm4680968pdk.20.2015.08.12.01.23.05 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Wed, 12 Aug 2015 01:23:06 -0700 (PDT) Date: Wed, 12 Aug 2015 13:53:02 +0530 From: Viresh Kumar To: Dan Carpenter Cc: Rafael Wysocki , nm@ti.com, sboyd@codeaurora.org, linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org, khilman@linaro.org, Greg Kroah-Hartman , Len Brown , open list , Pavel Machek Subject: Re: [PATCH V2 1/6] PM / OPP: Free resources and properly return error on failure Message-ID: <20150812082302.GD16445@linux> References: <334a9052264630b9157fa9bfc3d4efe945054c34.1439288881.git.viresh.kumar@linaro.org> <20150811144345.GN5180@mwanda> <20150811145938.GA32049@linux> <20150811171132.GA25166@mwanda> <20150812064309.GL32049@linux> <20150812081113.GC32040@mwanda> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20150812081113.GC32040@mwanda> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: linux-kernel@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.54 as permitted sender) smtp.mailfrom=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 12-08-15, 11:11, Dan Carpenter wrote: > If it doesn't WARN() then it's not buggy, but it's still ugly. We > should not call of_free_opp_table() because we *tried* to add an OPP, we > should only call it if we *succeeded*. This is done in order to write lesser code. Otherwise we need something like this: To some people, this will look even more *ugly*. And so I just called free_table() on error. > The way the code is written and from your emails I was afraid that if > you tried to call _opp_add_static_v2() and it fails then it leaves > artifacts lying around that need to be cleaned up by the caller. No. The problem is that we are trying to add OPPs in a while loop and on failure we need to free all we added earlier. diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index bcbd92c3b717..650e92e2f2f0 100644 --- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -1317,14 +1317,15 @@ static int _of_init_opp_table_v2(struct device *dev, /* We have opp-list node now, iterate over it and add OPPs */ for_each_available_child_of_node(opp_np, np) { - count++; - ret = _opp_add_static_v2(dev, np); if (ret) { dev_err(dev, "%s: Failed to add OPP, %d\n", __func__, ret); + if (!count) + goto put_opp_np; goto free_table; } + count++; } /* There should be one of more OPP defined */