From patchwork Mon Sep 7 12:23:32 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomeu Vizoso X-Patchwork-Id: 7134831 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 1FB34BEEC1 for ; Mon, 7 Sep 2015 12:45:22 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 15218206CB for ; Mon, 7 Sep 2015 12:45:21 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 86A77206D4 for ; Mon, 7 Sep 2015 12:45:19 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1ZYvlY-0008J3-0T; Mon, 07 Sep 2015 12:43:20 +0000 Received: from casper.infradead.org ([2001:770:15f::2]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1ZYvlI-0007xU-TW for linux-arm-kernel@bombadil.infradead.org; Mon, 07 Sep 2015 12:43:05 +0000 Received: from mail-wi0-x229.google.com ([2a00:1450:400c:c05::229]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1ZYvTV-000538-RE for linux-arm-kernel@lists.infradead.org; Mon, 07 Sep 2015 12:24:43 +0000 Received: by wicfx3 with SMTP id fx3so86928483wic.1 for ; Mon, 07 Sep 2015 05:24:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references; bh=BDxzaUVlo8vNRSI6R1W7dDAblwOJ9DVkAsAUKihGYS0=; b=jpRiWl/PK/NGJ+l65AC29agzwFEoDcWjbIOpNc6yUepd8KL1p8w9+cuQqNDwfGCLih VtEk94f8kzT3lnOW6fGHemRj+jJAXt24wDE+u7DOqS3XRkbdWqMecbI/bLTAiau7LceY 4w9cUbrn0CoDxaAswUL9RPrp/m2e1JJ/iz0yjCvTqKbN92+IxTfFAgeeWo8fMzehDpHW SYVbWakLDBfM6XitFZgWYVcaKDUxu/TtoECuTDB/umwssP1UTSBHPrnPAweh2Cb6xy/1 w56SXEy+A90F1mXY7AEhKtZt48QkTfq+iXf7AmoUBjBrGMX1VmRks62CoWRpPRNoyOyM pl0g== X-Received: by 10.180.182.107 with SMTP id ed11mr33964088wic.52.1441628658425; Mon, 07 Sep 2015 05:24:18 -0700 (PDT) Received: from cizrna.lan ([109.72.12.78]) by smtp.gmail.com with ESMTPSA id cm6sm15438wib.22.2015.09.07.05.24.16 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 07 Sep 2015 05:24:17 -0700 (PDT) From: Tomeu Vizoso To: linux-kernel@vger.kernel.org Subject: [PATCH v4 07/22] regulator: core: Reduce critical area in _regulator_get Date: Mon, 7 Sep 2015 14:23:32 +0200 Message-Id: <1441628627-5143-8-git-send-email-tomeu.vizoso@collabora.com> X-Mailer: git-send-email 2.4.3 In-Reply-To: <1441628627-5143-1-git-send-email-tomeu.vizoso@collabora.com> References: <1441628627-5143-1-git-send-email-tomeu.vizoso@collabora.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20150907_132442_034005_F09E1974 X-CRM114-Status: GOOD ( 26.02 ) X-Spam-Score: -2.4 (--) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Tomeu Vizoso , devicetree@vger.kernel.org, linux-acpi@vger.kernel.org, Arnd Bergmann , Stephen Warren , Linus Walleij , Dmitry Torokhov , "Rafael J. Wysocki" , Liam Girdwood , Rob Herring , Javier Martinez Canillas , Mark Brown , Thierry Reding , linux-arm-kernel@lists.infradead.org MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP By moving the locking of regulator_list_mutex into regulator_dev_lookup, where it is iterated over. The reference count of the regulator's device is increased in case it's unregistered while in use. In _regulator_get() the regulator_list_mutex mutex was held for most of the function, but that is only strictly needed to protect the list lookups. This change would be useful if for example regulator devices could be registered on demand when a driver requests them. regulator_register() could end up being called from within _regulator_get while the lock on regulator_list_mutex is being held, causing a deadlock. This sequence illustrates the situation described above: tegra_hdmi_probe _regulator_get regulator_dev_lookup of_device_probe reg_fixed_voltage_probe regulator_register Signed-off-by: Tomeu Vizoso --- Changes in v4: - Take a reference to the regulator's device to prevent dangling pointers Changes in v3: - Avoid unlocking the regulator device's mutex if we don't have a device Changes in v2: - Acquire regulator device lock before returning from regulator_dev_lookup() drivers/regulator/core.c | 56 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 7150ff6ef46b..f4aa6cae76d5 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -1340,10 +1340,15 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev, if (dev && dev->of_node) { node = of_get_regulator(dev, supply); if (node) { + mutex_lock(®ulator_list_mutex); list_for_each_entry(r, ®ulator_list, list) if (r->dev.parent && - node == r->dev.of_node) + node == r->dev.of_node && + get_device(&r->dev)) { + mutex_unlock(®ulator_list_mutex); return r; + } + mutex_unlock(®ulator_list_mutex); *ret = -EPROBE_DEFER; return NULL; } else { @@ -1361,9 +1366,13 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev, if (dev) devname = dev_name(dev); + mutex_lock(®ulator_list_mutex); list_for_each_entry(r, ®ulator_list, list) - if (strcmp(rdev_get_name(r), supply) == 0) + if (strcmp(rdev_get_name(r), supply) == 0 && + get_device(&r->dev)) { + mutex_unlock(®ulator_list_mutex); return r; + } list_for_each_entry(map, ®ulator_map_list, list) { /* If the mapping has a device set up it must match */ @@ -1371,9 +1380,13 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev, (!devname || strcmp(map->dev_name, devname))) continue; - if (strcmp(map->supply, supply) == 0) + if (strcmp(map->supply, supply) == 0 && + get_device(&map->regulator->dev)) { + mutex_unlock(®ulator_list_mutex); return map->regulator; + } } + mutex_unlock(®ulator_list_mutex); return NULL; @@ -1405,6 +1418,7 @@ static int regulator_resolve_supply(struct regulator_dev *rdev) if (!r) { if (have_full_constraints()) { r = dummy_regulator_rdev; + get_device(&r->dev); } else { dev_err(dev, "Failed to resolve %s-supply for %s\n", rdev->supply_name, rdev->desc->name); @@ -1414,12 +1428,16 @@ static int regulator_resolve_supply(struct regulator_dev *rdev) /* Recursively resolve the supply of the supply */ ret = regulator_resolve_supply(r); - if (ret < 0) + if (ret < 0) { + put_device(&r->dev); return ret; + } ret = set_supply(rdev, r); - if (ret < 0) + if (ret < 0) { + put_device(&r->dev); return ret; + } /* Cascade always-on state to supply */ if (_regulator_is_enabled(rdev) && rdev->supply) { @@ -1455,8 +1473,6 @@ static struct regulator *_regulator_get(struct device *dev, const char *id, else ret = -EPROBE_DEFER; - mutex_lock(®ulator_list_mutex); - rdev = regulator_dev_lookup(dev, id, &ret); if (rdev) goto found; @@ -1468,7 +1484,7 @@ static struct regulator *_regulator_get(struct device *dev, const char *id, * succeed, so, quit with appropriate error value */ if (ret && ret != -ENODEV) - goto out; + return regulator; if (!devname) devname = "deviceless"; @@ -1482,40 +1498,46 @@ static struct regulator *_regulator_get(struct device *dev, const char *id, devname, id); rdev = dummy_regulator_rdev; + get_device(&rdev->dev); goto found; /* Don't log an error when called from regulator_get_optional() */ } else if (!have_full_constraints() || exclusive) { dev_warn(dev, "dummy supplies not allowed\n"); } - mutex_unlock(®ulator_list_mutex); return regulator; found: if (rdev->exclusive) { regulator = ERR_PTR(-EPERM); - goto out; + put_device(&rdev->dev); + return regulator; } if (exclusive && rdev->open_count) { regulator = ERR_PTR(-EBUSY); - goto out; + put_device(&rdev->dev); + return regulator; } ret = regulator_resolve_supply(rdev); if (ret < 0) { regulator = ERR_PTR(ret); - goto out; + put_device(&rdev->dev); + return regulator; } - if (!try_module_get(rdev->owner)) - goto out; + if (!try_module_get(rdev->owner)) { + put_device(&rdev->dev); + return regulator; + } regulator = create_regulator(rdev, dev, id); if (regulator == NULL) { regulator = ERR_PTR(-ENOMEM); + put_device(&rdev->dev); module_put(rdev->owner); - goto out; + return regulator; } rdev->open_count++; @@ -1529,9 +1551,6 @@ found: rdev->use_count = 0; } -out: - mutex_unlock(®ulator_list_mutex); - return regulator; } @@ -1629,6 +1648,7 @@ static void _regulator_put(struct regulator *regulator) rdev->open_count--; rdev->exclusive = 0; + put_device(&rdev->dev); mutex_unlock(&rdev->mutex); kfree(regulator->supply_name);