From patchwork Fri May 18 08:48:49 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ulf Hansson X-Patchwork-Id: 136255 Delivered-To: patches@linaro.org Received: by 2002:a2e:9706:0:0:0:0:0 with SMTP id r6-v6csp962400lji; Fri, 18 May 2018 01:48:59 -0700 (PDT) X-Received: by 2002:a2e:9095:: with SMTP id l21-v6mr5393306ljg.58.1526633339030; Fri, 18 May 2018 01:48:59 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1526633339; cv=none; d=google.com; s=arc-20160816; b=CrakaEu3R2LD7d92PkJTL6Le7Zm6xk2Fta/h09bpIXgnf6jLSfbPZyWQpI9PsUkIrk 3Hz/Hgwit9Sz4ixlhsPgtFRZuLqybNd3KDj1QeQKp3z0lNcUts75vIzqFKLpVscGfRP2 IMAYIwea2R+q8q8A0CPOWlMelJdvvJpeWeo5FIRaH4JbI7SqiXgqgtZRbNGGzmqVgthr /HrWYYv1ez57Pmu7Nu3sIy7ZYRkh6EkM110QY6kHvd8ys266IMqY+tGdJy5yM4pac3lS zDBer6zbq7bDBoTR4DQG8AAPryRRge7dNEIGqgvFyqhxn2QZF4X0y5nB7hQThKVaPOn4 qegg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=message-id:date:subject:cc:to:from:dkim-signature :arc-authentication-results; bh=ypQSpbl47A6zpwSR8Xs7mcMX9FyOrN1qvT1l4XAyVtU=; b=tVZcTTxt5cLsU67N/dB47s6uzwoSAJBr0Pdvc6xiwjuyiSamPou4evqmPdPA2B1K9w heLMJOidPM5zsWFWVAp67xJ2CzdVvJdYIzAm9C9w2kj2iUKyUiDY8TbO18YrK8qtUE06 Ypqu1I4y3JCL9oXAkRKLTFWyLg8cz5yV00qGGz9Opc8Fqsk8FMLrWanVUkleYlaT2A3w DJV/aQzoHxpmh0NYbCHIR4HxSp+8m8s0C7DuGMQTNg/1xjiJ9Sw/gXpuK8USy96i9EKX asEQ6wGwb4GPYVb7M3c8jJ2hxiqQ2X6DfVX+A+6re+aMHn14MVOgUAVAiKHPwa3dsB72 VV0w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=bap9S3/z; spf=pass (google.com: domain of ulf.hansson@linaro.org designates 209.85.220.65 as permitted sender) smtp.mailfrom=ulf.hansson@linaro.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from mail-sor-f65.google.com (mail-sor-f65.google.com. [209.85.220.65]) by mx.google.com with SMTPS id q26-v6sor1921839lfc.34.2018.05.18.01.48.58 for (Google Transport Security); Fri, 18 May 2018 01:48:58 -0700 (PDT) Received-SPF: pass (google.com: domain of ulf.hansson@linaro.org designates 209.85.220.65 as permitted sender) client-ip=209.85.220.65; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=bap9S3/z; spf=pass (google.com: domain of ulf.hansson@linaro.org designates 209.85.220.65 as permitted sender) smtp.mailfrom=ulf.hansson@linaro.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=ypQSpbl47A6zpwSR8Xs7mcMX9FyOrN1qvT1l4XAyVtU=; b=bap9S3/z9eUzqWC3JtBIVFGoER2xlHjQdrhSu8WzMbH9+ddiAYmx5HdISiDQCqtiPF 5Mr8SmYb7V9dp8YiFoTpYww+SvJPHQwWtZMiYAC6sXB5j1PKVjFbjquBZxZTeO4GOdMX 8VCeaQsOXhPJ6zsLRds8h7ly6rJqHMR7W4OIo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=ypQSpbl47A6zpwSR8Xs7mcMX9FyOrN1qvT1l4XAyVtU=; b=BH+hR2E/u2bLGMtCZOxViyiFjMCWfIrL8LRB+o+ljKTsqN/V2lI/sasCFbieWzwWNC wwSHtlDOl7v0u6fAq9XtS/s6f/svd4lJVnWj/W/nxvW9YtbbS7QmPmLSF9RWG7DijSRK KeYt73x6Z7SNXFKrsErhwoe4uL1X53WoERhVE5BIoJVSqBzf0jtX4iyUt7JH5F71B/WK dvWzqB91wCXxKIVQLBLMyxbILTBaC2vX4hN7GeirbsC+4EuqhoULb6CbnXIyGw0/MjeQ 8s+Iu6WJPggZSgqS+WirHve/qK9Y2gqU0Nv9typ2fcC4nscr7UzxHj59tJ3jluxxfXJp A4bw== X-Gm-Message-State: ALKqPweOzH2tMNmC6RBUtHUGTWbGEst4+1Jd7yIZbZkbww2kACY6FWDC PRnecCeKThSz4L8AnAO63pIJB52o X-Google-Smtp-Source: AB8JxZqcg9jWbo1shP/53S6oiTTG7Ki3/e9XZ14ZCEyDytdg8wkvka7CDjJYlyL80WW2CakZxL3hlA== X-Received: by 2002:a19:a70f:: with SMTP id q15-v6mr9803400lfe.39.1526633338738; Fri, 18 May 2018 01:48:58 -0700 (PDT) Return-Path: Received: from localhost.localdomain (h-158-174-22-210.NA.cust.bahnhof.se. [158.174.22.210]) by smtp.gmail.com with ESMTPSA id z24-v6sm1731264lfg.56.2018.05.18.01.48.57 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 18 May 2018 01:48:57 -0700 (PDT) From: Ulf Hansson To: "Rafael J . Wysocki" , Greg Kroah-Hartman , linux-pm@vger.kernel.org Cc: Ulf Hansson , Viresh Kumar , Vincent Guittot , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: [PATCH] PM / runtime: Fixup reference counting of device link suppliers at probe Date: Fri, 18 May 2018 10:48:49 +0200 Message-Id: <1526633329-31729-1-git-send-email-ulf.hansson@linaro.org> X-Mailer: git-send-email 2.7.4 In the driver core, before it invokes really_probe() it runtime resumes the suppliers for the device via calling pm_runtime_get_suppliers(), which also increases the runtime PM usage count for each of the available supplier. This makes sense, as to be able to allow the consumer device to be probed by its driver. However, if the driver decides to add a new supplier link during ->probe(), hence updating the list of suppliers, the following call to pm_runtime_put_suppliers(), invoked after really_probe() in the driver core, we get into trouble. More precisely, pm_runtime_put() gets called also for the new supplier(s), which is wrong as the driver core, didn't trigger pm_runtime_get_sync() to be called for it in the first place. In other words, the new supplier may be runtime suspended even in cases when it shouldn't. Fix this behaviour, by runtime resume suppliers according to the same conditions as managed by the runtime PM core, when runtime resume callbacks are being invoked. Additionally, don't try to runtime suspend any of the suppliers after really_probe(), but instead rely on that to happen via the consumer device, when it becomes runtime suspended. Fixes: 21d5c57b3726 ("PM / runtime: Use device links") Signed-off-by: Ulf Hansson --- An optional solution (requires to add code, rather than remove), in case we think it's not suffient to rely on the consumer device to be runtime suspended after probe. This could be done via adding a new bool member to the struct device_link, and set it from pm_runtime_get_suppliers(). Check it at pm_runtime_put_suppliers(), and only call pm_runtime_put() on those suppliers that has it set. --- drivers/base/dd.c | 3 +-- drivers/base/power/runtime.c | 27 +++------------------------ include/linux/pm_runtime.h | 6 ++---- 3 files changed, 6 insertions(+), 30 deletions(-) -- 2.7.4 diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 10454fe..a41c91b 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -580,7 +580,7 @@ int driver_probe_device(struct device_driver *drv, struct device *dev) pr_debug("bus: '%s': %s: matched device %s with driver %s\n", drv->bus->name, __func__, dev_name(dev), drv->name); - pm_runtime_get_suppliers(dev); + pm_runtime_resume_suppliers(dev); if (dev->parent) pm_runtime_get_sync(dev->parent); @@ -591,7 +591,6 @@ int driver_probe_device(struct device_driver *drv, struct device *dev) if (dev->parent) pm_runtime_put(dev->parent); - pm_runtime_put_suppliers(dev); return ret; } diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 8bef3cb..6f4f50e 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1563,37 +1563,16 @@ void pm_runtime_clean_up_links(struct device *dev) } /** - * pm_runtime_get_suppliers - Resume and reference-count supplier devices. + * pm_runtime_resume_suppliers - Resume supplier devices. * @dev: Consumer device. */ -void pm_runtime_get_suppliers(struct device *dev) +void pm_runtime_resume_suppliers(struct device *dev) { - struct device_link *link; int idx; idx = device_links_read_lock(); - list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) - if (link->flags & DL_FLAG_PM_RUNTIME) - pm_runtime_get_sync(link->supplier); - - device_links_read_unlock(idx); -} - -/** - * pm_runtime_put_suppliers - Drop references to supplier devices. - * @dev: Consumer device. - */ -void pm_runtime_put_suppliers(struct device *dev) -{ - struct device_link *link; - int idx; - - idx = device_links_read_lock(); - - list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) - if (link->flags & DL_FLAG_PM_RUNTIME) - pm_runtime_put(link->supplier); + rpm_get_suppliers(dev); device_links_read_unlock(idx); } diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h index f0fc470..db5dbbf 100644 --- a/include/linux/pm_runtime.h +++ b/include/linux/pm_runtime.h @@ -56,8 +56,7 @@ extern void pm_runtime_update_max_time_suspended(struct device *dev, s64 delta_ns); extern void pm_runtime_set_memalloc_noio(struct device *dev, bool enable); extern void pm_runtime_clean_up_links(struct device *dev); -extern void pm_runtime_get_suppliers(struct device *dev); -extern void pm_runtime_put_suppliers(struct device *dev); +extern void pm_runtime_resume_suppliers(struct device *dev); extern void pm_runtime_new_link(struct device *dev); extern void pm_runtime_drop_link(struct device *dev); @@ -173,8 +172,7 @@ static inline unsigned long pm_runtime_autosuspend_expiration( static inline void pm_runtime_set_memalloc_noio(struct device *dev, bool enable){} static inline void pm_runtime_clean_up_links(struct device *dev) {} -static inline void pm_runtime_get_suppliers(struct device *dev) {} -static inline void pm_runtime_put_suppliers(struct device *dev) {} +static inline void pm_runtime_resume_suppliers(struct device *dev) {} static inline void pm_runtime_new_link(struct device *dev) {} static inline void pm_runtime_drop_link(struct device *dev) {}