Re: Regression: USB is nfg after suspend/resume(RAM) cycle on Intelchipset

[Mark Lord]

Alan Stern wrote:
> On Tue, 29 May 2007, Mark Lord wrote:
> > Mark Lord wrote:
> > > 7ed92f1a149dddc3cb537ccd7441e98adac12c3e USB: make the autosuspend 
> > > workqueue thread freezable
> > > ef7f6c7084b333c7524dcd297e0578d43733a2a2 USB: more autosuspend timer stuff
> > >
> > > I yanked them both, as they appeared to be releated based on the titles.
> > > Reverting this pair of commits fixes the USB suspend/resume regression.
> > Okay, just to make it trivial,
> > I've narrowed it down to only this commit from Alan Stern:
> >
> > 7ed92f1a149dddc3cb537ccd7441e98adac12c3e USB: make the autosuspend workqueue thread freezable
>
> I'm a little surprised; I would have expected the other patch to be the 
> one responsible.  Are you sure you got the right one?

Absolutely, 100% sure.

> Your symptoms suggest that the ksuspend_usbd and khubd kernel threads
> are in deadlock.
..
> If this is indeed the case, the patch below ought to fix your problem.  
> Essentially it replaces calls to flush_workqueue() with
> cancel_sync_work().  I sent it to Andrew last week, but his problem
> occurs only sporadically so it's hard to test.

I didn't re-break the kernel to do the alt-sysrq thing,
but I did reapply the b0rken commit plus your new patch (below).

So far, so good with that combination, thanks.
..

> Index: 2.6.22-rc3/drivers/usb/core/hub.c
> ===================================================================
> --- 2.6.22-rc3.orig/drivers/usb/core/hub.c
> +++ 2.6.22-rc3/drivers/usb/core/hub.c
> @@ -1158,6 +1158,30 @@ static void release_address(struct usb_d
>  	}
>  }
>  
> +#ifdef	CONFIG_USB_SUSPEND
> +
> +static void usb_stop_pm(struct usb_device *udev)
> +{
> +	/* Synchronize with the ksuspend thread to prevent any more
> +	 * autosuspend requests from being submitted, and decrement
> +	 * the parent's count of unsuspended children.
> +	 */
> +	usb_pm_lock(udev);
> +	if (udev->parent && !udev->discon_suspended)
> +		usb_autosuspend_device(udev->parent);
> +	usb_pm_unlock(udev);
> +
> +	/* Stop any autosuspend requests already submitted */
> +	cancel_rearming_delayed_work(&udev->autosuspend);
> +}
> +
> +#else
> +
> +static inline void usb_stop_pm(struct usb_device *udev)
> +{ }
> +
> +#endif
> +
>  /**
>   * usb_disconnect - disconnect a device (usbcore-internal)
>   * @pdev: pointer to device being disconnected
> @@ -1224,13 +1248,7 @@ void usb_disconnect(struct usb_device **
>  	*pdev = NULL;
>  	spin_unlock_irq(&device_state_lock);
>  
> -	/* Decrement the parent's count of unsuspended children */
> -	if (udev->parent) {
> -		usb_pm_lock(udev);
> -		if (!udev->discon_suspended)
> -			usb_autosuspend_device(udev->parent);
> -		usb_pm_unlock(udev);
> -	}
> +	usb_stop_pm(udev);
>  
>  	put_device(&udev->dev);
>  }
> Index: 2.6.22-rc3/drivers/usb/core/usb.c
> ===================================================================
> --- 2.6.22-rc3.orig/drivers/usb/core/usb.c
> +++ 2.6.22-rc3/drivers/usb/core/usb.c
> @@ -184,10 +184,6 @@ static void usb_release_dev(struct devic
>  
>  	udev = to_usb_device(dev);
>  
> -#ifdef	CONFIG_USB_SUSPEND
> -	cancel_delayed_work(&udev->autosuspend);
> -	flush_workqueue(ksuspend_usb_wq);
> -#endif
>  	usb_destroy_configuration(udev);
>  	usb_put_hcd(bus_to_hcd(udev->bus));
>  	kfree(udev->product);
> Index: 2.6.22-rc3/drivers/usb/core/hcd.c
> ===================================================================
> --- 2.6.22-rc3.orig/drivers/usb/core/hcd.c
> +++ 2.6.22-rc3/drivers/usb/core/hcd.c
> @@ -1681,7 +1681,7 @@ void usb_remove_hcd(struct usb_hcd *hcd)
>  	spin_unlock_irq (&hcd_root_hub_lock);
>  
>  #ifdef CONFIG_PM
> -	flush_workqueue(ksuspend_usb_wq);
> +	cancel_work_sync(&hcd->wakeup_work);
>  #endif
>  
>  	mutex_lock(&usb_bus_list_lock);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/