Re: [RFC] Asynchronous suspend/resume - test results

Rafael J. Wysocki
2009-12-21T20:35:29Z

On Monday 21 December 2009, Alan Stern wrote:
> On Mon, 21 Dec 2009, Rafael J. Wysocki wrote:
>
> > Hi,
> >
> > After the Dmitry's suggestion to use PSMOUSE_CMD_RESET_DIS during suspend
> > (and analogously for atkbd), I found that it reduced the suspend time
> > significantly and changed the picture quite a bit. For this reason I re-ran
> > the async suspend and resume tests on the nx6325 and Wind U100.
> >
> > This time I marked the following devices as "async":
> > - all USB devices (including interfaces and endpoints)
> > - ACPI battery
> > - sd and its parent
>
> Apparently not. On nx6325 the sd device is 0:0:0:0, which did suspend
> in a different thread, but its parent is target0:0:0 which suspended
> and resumed in the main thread. And its grandparent, host0, also
> suspended and resumed in the main thread.

Hmm. I must have confused things, then.

...
> There are still some odd things going on. Look at
> nx6325-async-suspend-upfront-dmesg.log. There's a peculiar gap between
>
> [ 233.176029] call 0000:00:14.2+ returned 0 after 217543 usecs
>
> and
>
> [ 234.059948] call 0:0:0:0+ returned 0 after 1078047 usecs
>
> A lot of stuff could have happened during that gap, but nothing did.
> That's because the target0:0:0 device was synchronous, and it was
> waiting for its child (the long-running async 0:0:0:0) to finish. Not
> that it made much difference in the end, because the entire suspend
> finished about 16 ms later. But if there was more than one disk or if
> the disk had been a USB drive, it would have mattered.
>
> You should also make SCSI targets and hosts async. Hosts are added in
> drivers/scsi/hosts.c:scsi_add_host_with_dma() (in 2.6.32 this was
> named scsi_add_host()). Targets are added in
> drivers/scsi/scsi_sysfs.c:scsi_target_add(). And for thoroughness,
> SCSI devices are added in scsi_sysfs_add_sdev() in the same file.

Thanks a lot for the pointers.

Do you think it's a good idea to mark all SCSI devices as async? Well, for
testing it probably is, but I doubt in general.

> More importantly, why did 0:0:0:0 wait almost 20 ms to begin
> suspending? Undoubtedly because it was blocked, waiting for some
> unlogged synchronous device. That's why you need to print out log
> entries for devices without methods. In this case it was probably a
> class device.

OK

> Although USB host controllers aren't the longest-running devices to
> resume, they do tend to be on the longest paths. Speeding them up
> would help. One change you could try is in the patch below.
> Currently when a controller has to be reset, the root hub beneath it is
> also reset and then re-suspended. However there's no reason to suspend
> it if the PM core is only going to resume it a little bit later. The
> patch gets rid of the unnecessary suspend. Note: I haven't tested it.

OK, I'll try it.

Rafael

> Index: usb-2.6/drivers/usb/host/ohci-hub.c
> ===================================================================
> --- usb-2.6.orig/drivers/usb/host/ohci-hub.c
> +++ usb-2.6/drivers/usb/host/ohci-hub.c
> @@ -333,14 +333,12 @@ static void ohci_finish_controller_resum
> }
> }
>
> - /* If needed, reinitialize and suspend the root hub */
> + /* If needed, reinitialize the root hub */
> if (need_reinit) {
> spin_lock_irq(&ohci->lock);
> hcd->state = HC_STATE_RESUMING;
> - ohci_rh_resume(ohci);
> - hcd->state = HC_STATE_QUIESCING;
> - ohci_rh_suspend(ohci, 0);
> - hcd->state = HC_STATE_SUSPENDED;
> + if (ohci_rh_resume(ohci) == 0)
> + hcd->state = HC_STATE_RUNNING;
> spin_unlock_irq(&ohci->lock);
> }
>
> Index: usb-2.6/drivers/usb/host/uhci-hcd.c
> ===================================================================
> --- usb-2.6.orig/drivers/usb/host/uhci-hcd.c
> +++ usb-2.6/drivers/usb/host/uhci-hcd.c
> @@ -835,7 +835,6 @@ static int uhci_pci_resume(struct usb_hc
>
> /* The controller had to be reset */
> usb_root_hub_lost_power(hcd->self.root_hub);
> - suspend_rh(uhci, UHCI_RH_SUSPENDED);
> }
>
> spin_unlock_irq(&uhci->lock);
>