Re: [PATCH]suspend/resume for cdc_acm

Am Mittwoch 10 Oktober 2007 schrieb Alan Stern:
> On Wed, 10 Oct 2007, Oliver Neukum wrote:

> > +	flush_scheduled_work(); /* wait for acm_softint */
> 
> flush_scheduled_work() should be avoided wherever possible.  I realize 
> you just moved the existing code.  Nevertheless, that routine is prone 
> to deadlocks.  You'd be better off cancelling any outstanding workqueue 
> items or calling the work routines directly.

OK.

> > +	*/
> > +	mutex_lock(&open_mutex);
> > +
> > +	if (acm->used)
> > +		stop_data_traffic(acm);
> > +
> > +	mutex_unlock(&open_mutex);
> 
> You have to be very careful about locking order in the suspend and
> resume methods.  I'm not familiar enough with the rest of the driver to
> say whether this is okay or not, but the possibility exists for locking
> loops involving the device's semaphore, the device's PM mutex, and the
> private open_mutex.  You may be okay for now, but then adding
> autosuspend support is quite likely to cause a loop.

Yes, autoresume with this methods hangs. I have a patch that changes
the locking, but I want to be conservative in case somebody bisects this.

	Regards
		Oliver



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel