Re: "usb_wwan: error case of resume" (16871dcac) is buggy

[Alan Stern]

On Mon, 21 Mar 2011, Oliver Neukum wrote:

> Am Montag, 21. März 2011, 01:36:45 schrieb Jiri Kosina:
> > Hi,
> > 
> > the commit in subject make the kernel with CONFIG_PM_RUNTIME unset fail 
> > during compilation, as struct dev_pm_info doesn't have whole bunch of 
> > members in such case.
> > 
> > The commit in question adds this code:
> > 
> > 	/* we have to throw away the rest */
> > 	do {
> > 		unbusy_queued_urb(urb, portdata);
> > 		//extremely dirty
> > 		atomic_dec(&port->serial->interface->dev.power.usage_count);
> > 	} while ((urb = usb_get_from_anchor(&portdata->delayed)));
> > 
> > The 'extermely dirty' comment makes me a bit nervous whether the patch 
> > below is correct or some more thinking would be necessary.
> 
> I've since posted a clean patch, although your fix looks correct to me.

In fact the original code and Jiri's fix are both wrong.  The layering 
violation here is not only inelegant, it is downright buggy.

The USB core contains its own usage_count for interfaces, in parallel 
with the usage_count in interface->dev.power.  Bypassing the USB layer 
in this way causes the two counts to get out of sync, which will lead 
to errors.

Oliver's new fix is correct.

Alan Stern

--
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/