[PATCH] usb: storage: fix runtime pm issue in usb_stor_probe2
Since commit 71723f95463d "PM / runtime: print error when activating a
child to unactive parent" I see the following error message:
scsi host2: usb-storage 1-3:1.0
scsi host2: runtime PM trying to activate child device host2 but parent
(1-3:1.0) is not active
Digging into it it seems to be related to the problem described in the
commit message for cd998ded5c12 "i2c: designware: Prevent runtime
suspend during adapter registration" as scsi_add_host also calls
device_add and after the call to device_add the parent device is
suspended.
Fix this by using the approach from the mentioned commit and getting
the runtime pm reference before calling scsi_add_host.
Signed-off-by: Heiner Kallweit <hkallwe...@gmail.com>
---
drivers/usb/storage/usb.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index ef2d8cd..8c5f011 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -1070,17 +1070,17 @@ int usb_stor_probe2(struct us_data *us)
result = usb_stor_acquire_resources(us);
if (result)
goto BadDevice;
+ usb_autopm_get_interface_no_resume(us->pusb_intf);
snprintf(us->scsi_name, sizeof(us->scsi_name), "usb-storage %s",
dev_name(>pusb_intf->dev));
result = scsi_add_host(us_to_host(us), dev);
if (result) {
dev_warn(dev,
"Unable to add the scsi host\n");
- goto BadDevice;
+ goto HostAddErr;
}
/* Submit the delayed_work for SCSI-device scanning */
- usb_autopm_get_interface_no_resume(us->pusb_intf);
set_bit(US_FLIDX_SCAN_PENDING, >dflags);
if (delay_use > 0)
@@ -1090,6 +1090,8 @@ int usb_stor_probe2(struct us_data *us)
return 0;
/* We come here if there are any problems */
+HostAddErr:
+ usb_autopm_put_interface_no_suspend(us->pusb_intf);
BadDevice:
usb_stor_dbg(us, "storage_probe() failed\n");
release_everything(us);
--
2.9.2
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] scsi: core: configure runtime pm before calling device_add in scsi_add_host_with_dma
Runtime PM should be configured already once we call device_add. See also the description in this mail thread https://lists.linuxfoundation.org/pipermail/linux-pm/2009-November/023198.html or the order of calls e.g. in usb_new_device. The changed order also helps to avoid scenarios where runtime pm for >shost_gendev is activated whilst the parent is suspended, resulting in error message "runtime PM trying to activate child device hostx but parent yyy is not active". In addition properly reverse the runtime pm calls in the error path. Signed-off-by: Heiner Kallweit <hkallwe...@gmail.com> --- drivers/scsi/hosts.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c index ba9af4a..9ab94ad 100644 --- a/drivers/scsi/hosts.c +++ b/drivers/scsi/hosts.c @@ -246,10 +246,6 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev, shost->dma_dev = dma_dev; - error = device_add(>shost_gendev); - if (error) - goto out_destroy_freelist; - /* * Increase usage count temporarily here so that calling * scsi_autopm_put_host() will trigger runtime idle if there is @@ -260,6 +256,10 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev, pm_runtime_enable(>shost_gendev); device_enable_async_suspend(>shost_gendev); + error = device_add(>shost_gendev); + if (error) + goto out_destroy_freelist; + scsi_host_set_state(shost, SHOST_RUNNING); get_device(shost->shost_gendev.parent); @@ -309,6 +309,10 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev, out_del_gendev: device_del(>shost_gendev); out_destroy_freelist: + device_disable_async_suspend(>shost_gendev); + pm_runtime_disable(>shost_gendev); + pm_runtime_set_suspended(>shost_gendev); + pm_runtime_put_noidle(>shost_gendev); scsi_destroy_command_freelist(shost); out_destroy_tags: if (shost_use_blk_mq(shost)) -- 2.9.2 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: storage: fix runtime pm issue in usb_stor_probe2
Am 03.08.2016 um 23:25 schrieb Alan Stern:
> On Wed, 3 Aug 2016, Heiner Kallweit wrote:
>
>> Since commit 71723f95463d "PM / runtime: print error when activating a
>> child to unactive parent" I see the following error message:
>>
>> scsi host2: usb-storage 1-3:1.0
>> scsi host2: runtime PM trying to activate child device host2 but parent
>> (1-3:1.0) is not active
>>
>> Digging into it it seems to be related to the problem described in the
>> commit message for cd998ded5c12 "i2c: designware: Prevent runtime
>> suspend during adapter registration" as scsi_add_host also calls
>> device_add and after the call to device_add the parent device is
>> suspended.
>>
>> Fix this by using the approach from the mentioned commit and getting
>> the runtime pm reference before calling scsi_add_host.
>
> Acked-by: Alan Stern <st...@rowland.harvard.edu>
>
> There's nothing wrong with this patch; it's a worthwhile thing to do
> because it can prevent an unnecessary runtime-suspend/resume cycle.
>
> Still, it looks like the real problem here may lie in
> drivers/scsi/hosts.c. Commit bc4f24014de5 ("[SCSI] implement runtime
> Power Management") added a call to pm_runtime_set_active() in
> scsi_add_host_with_dma() _after_ device_add() instead of _before_.
>
> If that were changed, this problem would not have occurred.
>
In parallel to this patch I sent another one to address exactly the
ordering issue in scsi_add_host_with_dma you mention.
The other patch went to Martin and the SCSI mailing list only, sorry.
I'll forward it to you for review.
Heiner
> Alan Stern
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: core: configure runtime pm before calling device_add in scsi_add_host_with_dma
Am 03.08.2016 um 21:49 schrieb Heiner Kallweit: > Runtime PM should be configured already once we call device_add. See also > the description in this mail thread > https://lists.linuxfoundation.org/pipermail/linux-pm/2009-November/023198.html > or the order of calls e.g. in usb_new_device. > > The changed order also helps to avoid scenarios where runtime pm for > >shost_gendev is activated whilst the parent is suspended, > resulting in error message "runtime PM trying to activate child device > hostx but parent yyy is not active". > > In addition properly reverse the runtime pm calls in the error path. > > Signed-off-by: Heiner Kallweit <hkallwe...@gmail.com> > --- > drivers/scsi/hosts.c | 12 > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c > index ba9af4a..9ab94ad 100644 > --- a/drivers/scsi/hosts.c > +++ b/drivers/scsi/hosts.c > @@ -246,10 +246,6 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, > struct device *dev, > > shost->dma_dev = dma_dev; > > - error = device_add(>shost_gendev); > - if (error) > - goto out_destroy_freelist; > - > /* >* Increase usage count temporarily here so that calling >* scsi_autopm_put_host() will trigger runtime idle if there is > @@ -260,6 +256,10 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, > struct device *dev, > pm_runtime_enable(>shost_gendev); > device_enable_async_suspend(>shost_gendev); > > + error = device_add(>shost_gendev); > + if (error) > + goto out_destroy_freelist; > + > scsi_host_set_state(shost, SHOST_RUNNING); > get_device(shost->shost_gendev.parent); > > @@ -309,6 +309,10 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, > struct device *dev, > out_del_gendev: > device_del(>shost_gendev); > out_destroy_freelist: > + device_disable_async_suspend(>shost_gendev); > + pm_runtime_disable(>shost_gendev); > + pm_runtime_set_suspended(>shost_gendev); > + pm_runtime_put_noidle(>shost_gendev); > scsi_destroy_command_freelist(shost); > out_destroy_tags: > if (shost_use_blk_mq(shost)) > Acked-by: Alan Stern <st...@rowland.harvard.edu> -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Bug 153171] New: scsi host6: runtime PM trying to activate child device host6 but parent (2-2:1.0) is not active
Am 15.08.2016 um 21:55 schrieb bugzilla-dae...@bugzilla.kernel.org: > https://bugzilla.kernel.org/show_bug.cgi?id=153171 > > Bug ID: 153171 >Summary: scsi host6: runtime PM trying to activate child device > host6 but parent (2-2:1.0) is not active >Product: IO/Storage >Version: 2.5 > Kernel Version: 4.8.0-040800rc2-lowlatency > Hardware: All > OS: Linux > Tree: Mainline > Status: NEW > Severity: normal > Priority: P1 > Component: SCSI > Assignee: linux-scsi@vger.kernel.org > Reporter: carav...@gmail.com > Regression: No > > Created attachment 228931 > --> https://bugzilla.kernel.org/attachment.cgi?id=228931=edit > dmesg_4.8.0-040800rc2-lowlatency > > Mount USB Pendrive > > dmesg: > [ 4630.693915] scsi host6: runtime PM trying to activate child device host6 > but > parent (2-2:1.0) is not active > This issue is fixed by patch scsi: "core: configure runtime pm before calling device_add in scsi_add_host_with_dma" I sent on Aug 3rd. Regards, Heiner -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
