* [PATCH] PCI/pwrctrl: lock device when calling device_is_bound()
@ 2026-05-18 10:07 Bartosz Golaszewski
2026-05-18 10:29 ` Manivannan Sadhasivam
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Bartosz Golaszewski @ 2026-05-18 10:07 UTC (permalink / raw)
To: Bartosz Golaszewski, Manivannan Sadhasivam, Bjorn Helgaas,
Krishna Chaitanya Chundru
Cc: linux-pci, linux-kernel, Bartosz Golaszewski
The kerneldoc for device_is_bound() states that it must be called with
the device lock taken. Synchronize the two calls in pwrctrl core.
Fixes: b35cf3b6aa1e ("PCI/pwrctrl: Add APIs to power on/off pwrctrl devices")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/pci/pwrctrl/core.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
index 97cff5b8ca88..cd08d590483b 100644
--- a/drivers/pci/pwrctrl/core.c
+++ b/drivers/pci/pwrctrl/core.c
@@ -161,10 +161,12 @@ static void pci_pwrctrl_power_off_device(struct device_node *np)
if (!pdev)
return;
- if (device_is_bound(&pdev->dev)) {
- ret = __pci_pwrctrl_power_off_device(&pdev->dev);
- if (ret)
- dev_err(&pdev->dev, "Failed to power off device: %d", ret);
+ scoped_guard(device, &pdev->dev) {
+ if (device_is_bound(&pdev->dev)) {
+ ret = __pci_pwrctrl_power_off_device(&pdev->dev);
+ if (ret)
+ dev_err(&pdev->dev, "Failed to power off device: %d", ret);
+ }
}
platform_device_put(pdev);
@@ -205,7 +207,7 @@ static int __pci_pwrctrl_power_on_device(struct device *dev)
static int pci_pwrctrl_power_on_device(struct device_node *np)
{
struct platform_device *pdev;
- int ret;
+ int ret = 0;
for_each_available_child_of_node_scoped(np, child) {
ret = pci_pwrctrl_power_on_device(child);
@@ -217,12 +219,14 @@ static int pci_pwrctrl_power_on_device(struct device_node *np)
if (!pdev)
return 0;
- if (device_is_bound(&pdev->dev)) {
- ret = __pci_pwrctrl_power_on_device(&pdev->dev);
- } else {
- /* FIXME: Use blocking wait instead of probe deferral */
- dev_dbg(&pdev->dev, "driver is not bound\n");
- ret = -EPROBE_DEFER;
+ scoped_guard(device, &pdev->dev) {
+ if (device_is_bound(&pdev->dev)) {
+ ret = __pci_pwrctrl_power_on_device(&pdev->dev);
+ } else {
+ /* FIXME: Use blocking wait instead of probe deferral */
+ dev_dbg(&pdev->dev, "driver is not bound\n");
+ ret = -EPROBE_DEFER;
+ }
}
platform_device_put(pdev);
--
2.47.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] PCI/pwrctrl: lock device when calling device_is_bound()
2026-05-18 10:07 [PATCH] PCI/pwrctrl: lock device when calling device_is_bound() Bartosz Golaszewski
@ 2026-05-18 10:29 ` Manivannan Sadhasivam
2026-05-18 10:45 ` sashiko-bot
2026-05-18 22:32 ` Bjorn Helgaas
2 siblings, 0 replies; 4+ messages in thread
From: Manivannan Sadhasivam @ 2026-05-18 10:29 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Bartosz Golaszewski, Bjorn Helgaas, Krishna Chaitanya Chundru,
linux-pci, linux-kernel
On Mon, May 18, 2026 at 12:07:00PM +0200, Bartosz Golaszewski wrote:
> The kerneldoc for device_is_bound() states that it must be called with
> the device lock taken. Synchronize the two calls in pwrctrl core.
>
> Fixes: b35cf3b6aa1e ("PCI/pwrctrl: Add APIs to power on/off pwrctrl devices")
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
- Mani
> ---
> drivers/pci/pwrctrl/core.c | 26 +++++++++++++++-----------
> 1 file changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
> index 97cff5b8ca88..cd08d590483b 100644
> --- a/drivers/pci/pwrctrl/core.c
> +++ b/drivers/pci/pwrctrl/core.c
> @@ -161,10 +161,12 @@ static void pci_pwrctrl_power_off_device(struct device_node *np)
> if (!pdev)
> return;
>
> - if (device_is_bound(&pdev->dev)) {
> - ret = __pci_pwrctrl_power_off_device(&pdev->dev);
> - if (ret)
> - dev_err(&pdev->dev, "Failed to power off device: %d", ret);
> + scoped_guard(device, &pdev->dev) {
> + if (device_is_bound(&pdev->dev)) {
> + ret = __pci_pwrctrl_power_off_device(&pdev->dev);
> + if (ret)
> + dev_err(&pdev->dev, "Failed to power off device: %d", ret);
> + }
> }
>
> platform_device_put(pdev);
> @@ -205,7 +207,7 @@ static int __pci_pwrctrl_power_on_device(struct device *dev)
> static int pci_pwrctrl_power_on_device(struct device_node *np)
> {
> struct platform_device *pdev;
> - int ret;
> + int ret = 0;
>
> for_each_available_child_of_node_scoped(np, child) {
> ret = pci_pwrctrl_power_on_device(child);
> @@ -217,12 +219,14 @@ static int pci_pwrctrl_power_on_device(struct device_node *np)
> if (!pdev)
> return 0;
>
> - if (device_is_bound(&pdev->dev)) {
> - ret = __pci_pwrctrl_power_on_device(&pdev->dev);
> - } else {
> - /* FIXME: Use blocking wait instead of probe deferral */
> - dev_dbg(&pdev->dev, "driver is not bound\n");
> - ret = -EPROBE_DEFER;
> + scoped_guard(device, &pdev->dev) {
> + if (device_is_bound(&pdev->dev)) {
> + ret = __pci_pwrctrl_power_on_device(&pdev->dev);
> + } else {
> + /* FIXME: Use blocking wait instead of probe deferral */
> + dev_dbg(&pdev->dev, "driver is not bound\n");
> + ret = -EPROBE_DEFER;
> + }
> }
>
> platform_device_put(pdev);
> --
> 2.47.3
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] PCI/pwrctrl: lock device when calling device_is_bound()
2026-05-18 10:07 [PATCH] PCI/pwrctrl: lock device when calling device_is_bound() Bartosz Golaszewski
2026-05-18 10:29 ` Manivannan Sadhasivam
@ 2026-05-18 10:45 ` sashiko-bot
2026-05-18 22:32 ` Bjorn Helgaas
2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-05-18 10:45 UTC (permalink / raw)
To: Bartosz Golaszewski; +Cc: linux-pci
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] If a device fails to power on (or is probe deferred), its successfully powered-on children and previous siblings within its subtree are not powered off, causing a resource leak.
--
commit 9be02f191b459f8fb2b5eb9ee177a3aa7076d6f1
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
PCI/pwrctrl: lock device when calling device_is_bound()
This commit adds device locking around device_is_bound() calls in the
PCI power control core to satisfy its kerneldoc requirements.
> diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
> index 97cff5b8ca885..cd08d590483b8 100644
> --- a/drivers/pci/pwrctrl/core.c
> +++ b/drivers/pci/pwrctrl/core.c
[ ... ]
> @@ -205,7 +207,7 @@ static int __pci_pwrctrl_power_on_device(struct device *dev)
> static int pci_pwrctrl_power_on_device(struct device_node *np)
> {
> struct platform_device *pdev;
> - int ret;
> + int ret = 0;
>
> for_each_available_child_of_node_scoped(np, child) {
> ret = pci_pwrctrl_power_on_device(child);
> @@ -217,12 +219,14 @@ static int pci_pwrctrl_power_on_device(struct device_node *np)
> if (!pdev)
> return 0;
>
> - if (device_is_bound(&pdev->dev)) {
> - ret = __pci_pwrctrl_power_on_device(&pdev->dev);
> - } else {
> - /* FIXME: Use blocking wait instead of probe deferral */
> - dev_dbg(&pdev->dev, "driver is not bound\n");
> - ret = -EPROBE_DEFER;
> + scoped_guard(device, &pdev->dev) {
> + if (device_is_bound(&pdev->dev)) {
> + ret = __pci_pwrctrl_power_on_device(&pdev->dev);
> + } else {
> + /* FIXME: Use blocking wait instead of probe deferral */
> + dev_dbg(&pdev->dev, "driver is not bound\n");
> + ret = -EPROBE_DEFER;
> + }
> }
>
> platform_device_put(pdev);
This problem wasn't introduced by this patch, but does returning here leak
resources on probe deferral?
If a device fails to power on or is probe deferred, the function returns
the error without reverting the state of the children that were already
successfully powered on in the for_each_available_child_of_node_scoped()
loop above.
The top-level caller pci_pwrctrl_power_on_devices() catches the error and
executes the err_power_off cleanup path, but that path only cleans up fully
successful top-level sibling trees up to the failing node. It appears to
skip the partially-powered subtree of the failing node itself.
Since probe deferrals are a normal and frequently repeated process during
system boot, could these leaked refcounts accumulate and eventually cause
resource exhaustion?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260518100700.47581-1-bartosz.golaszewski@oss.qualcomm.com?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] PCI/pwrctrl: lock device when calling device_is_bound()
2026-05-18 10:07 [PATCH] PCI/pwrctrl: lock device when calling device_is_bound() Bartosz Golaszewski
2026-05-18 10:29 ` Manivannan Sadhasivam
2026-05-18 10:45 ` sashiko-bot
@ 2026-05-18 22:32 ` Bjorn Helgaas
2 siblings, 0 replies; 4+ messages in thread
From: Bjorn Helgaas @ 2026-05-18 22:32 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Bartosz Golaszewski, Manivannan Sadhasivam, Bjorn Helgaas,
Krishna Chaitanya Chundru, linux-pci, linux-kernel
On Mon, May 18, 2026 at 12:07:00PM +0200, Bartosz Golaszewski wrote:
> The kerneldoc for device_is_bound() states that it must be called with
> the device lock taken. Synchronize the two calls in pwrctrl core.
>
> Fixes: b35cf3b6aa1e ("PCI/pwrctrl: Add APIs to power on/off pwrctrl devices")
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Applied to pci/pwrctrl for v7.2, thanks!
> ---
> drivers/pci/pwrctrl/core.c | 26 +++++++++++++++-----------
> 1 file changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
> index 97cff5b8ca88..cd08d590483b 100644
> --- a/drivers/pci/pwrctrl/core.c
> +++ b/drivers/pci/pwrctrl/core.c
> @@ -161,10 +161,12 @@ static void pci_pwrctrl_power_off_device(struct device_node *np)
> if (!pdev)
> return;
>
> - if (device_is_bound(&pdev->dev)) {
> - ret = __pci_pwrctrl_power_off_device(&pdev->dev);
> - if (ret)
> - dev_err(&pdev->dev, "Failed to power off device: %d", ret);
> + scoped_guard(device, &pdev->dev) {
> + if (device_is_bound(&pdev->dev)) {
> + ret = __pci_pwrctrl_power_off_device(&pdev->dev);
> + if (ret)
> + dev_err(&pdev->dev, "Failed to power off device: %d", ret);
> + }
> }
>
> platform_device_put(pdev);
> @@ -205,7 +207,7 @@ static int __pci_pwrctrl_power_on_device(struct device *dev)
> static int pci_pwrctrl_power_on_device(struct device_node *np)
> {
> struct platform_device *pdev;
> - int ret;
> + int ret = 0;
>
> for_each_available_child_of_node_scoped(np, child) {
> ret = pci_pwrctrl_power_on_device(child);
> @@ -217,12 +219,14 @@ static int pci_pwrctrl_power_on_device(struct device_node *np)
> if (!pdev)
> return 0;
>
> - if (device_is_bound(&pdev->dev)) {
> - ret = __pci_pwrctrl_power_on_device(&pdev->dev);
> - } else {
> - /* FIXME: Use blocking wait instead of probe deferral */
> - dev_dbg(&pdev->dev, "driver is not bound\n");
> - ret = -EPROBE_DEFER;
> + scoped_guard(device, &pdev->dev) {
> + if (device_is_bound(&pdev->dev)) {
> + ret = __pci_pwrctrl_power_on_device(&pdev->dev);
> + } else {
> + /* FIXME: Use blocking wait instead of probe deferral */
> + dev_dbg(&pdev->dev, "driver is not bound\n");
> + ret = -EPROBE_DEFER;
> + }
> }
>
> platform_device_put(pdev);
> --
> 2.47.3
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-05-18 22:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-18 10:07 [PATCH] PCI/pwrctrl: lock device when calling device_is_bound() Bartosz Golaszewski
2026-05-18 10:29 ` Manivannan Sadhasivam
2026-05-18 10:45 ` sashiko-bot
2026-05-18 22:32 ` Bjorn Helgaas
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.