On Wednesday 13 June 2012 16:26:06 Alan Stern wrote:
> On Wed, 13 Jun 2012, [windows-1252] Hans Petter Selasky wrote:
> > Hi,
> >
> > ?
> >
> > It appears that several USB drivers are not dropping the additional ref
> > done by usb_anchor_urb() when URBs are anchored temporarily as a result
> > of the device being suspended or temporarily offline.
> >
> > ?
> >
> > I've done a quick grep and tried to fix the issues I could find.
> > Unfortunately I cannot test all the hardware which this change affects.
> > Linux USB developers of:
> >
>
> > Please test the following and also attached patch:
> The documentation you added to urb.c looks good.
>
> Here and in lots of other places, you misspelled "additional".
Fixed.
Please find inlined and attached new patch. When everything is ready I will
post the patch in the required format. Until then please help test the drivers
affected by this patch. Thank you.
--HPS
diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c
index 7ae65fc..400ea6d 100644
--- a/drivers/net/can/usb/ems_usb.c
+++ b/drivers/net/can/usb/ems_usb.c
@@ -626,6 +626,7 @@ static int ems_usb_start(struct ems_usb *dev)
usb_unanchor_urb(urb);
usb_free_coherent(dev->udev, RX_BUFFER_SIZE, buf,
urb->transfer_dma);
+ usb_free_urb(urb);
break;
}
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 9f58330..22d2bc4 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1554,6 +1554,10 @@ int usbnet_resume (struct usb_interface *intf)
skb = (struct sk_buff *)res->context;
retval = usb_submit_urb(res, GFP_ATOMIC);
+
+ /* release additional ref done by anchor */
+ usb_put_urb(res);
+
if (retval < 0) {
dev_kfree_skb_any(skb);
usb_free_urb(res);
diff --git a/drivers/net/wireless/ath/carl9170/usb.c
b/drivers/net/wireless/ath/carl9170/usb.c
index 888152c..7ee9f33 100644
--- a/drivers/net/wireless/ath/carl9170/usb.c
+++ b/drivers/net/wireless/ath/carl9170/usb.c
@@ -153,6 +153,10 @@ static void carl9170_usb_submit_data_urb(struct ar9170
*ar)
usb_anchor_urb(urb, &ar->tx_err);
}
+ /* release additional ref done by previous anchor */
+ usb_free_urb(urb);
+
+ /* release additional ref done by current anchor */
usb_free_urb(urb);
if (likely(err == 0))
@@ -229,6 +233,10 @@ static int carl9170_usb_submit_cmd_urb(struct ar9170 *ar)
usb_unanchor_urb(urb);
atomic_dec(&ar->tx_cmd_urbs);
}
+ /* release additional ref done by previous anchor */
+ usb_put_urb(urb);
+
+ /* release additional ref done by current anchor */
usb_free_urb(urb);
return err;
@@ -323,6 +331,10 @@ static int carl9170_usb_submit_rx_urb(struct ar9170 *ar,
gfp_t gfp)
atomic_dec(&ar->rx_pool_urbs);
atomic_inc(&ar->rx_anch_urbs);
}
+ /* release additional ref done by previous anchor */
+ usb_put_urb(urb);
+
+ /* release additional ref done by current anchor */
usb_free_urb(urb);
}
}
@@ -345,10 +357,14 @@ static void carl9170_usb_rx_work(struct ar9170 *ar)
carl9170_rx(ar, urb->transfer_buffer,
urb->actual_length);
}
-
usb_anchor_urb(urb, &ar->rx_pool);
+
atomic_inc(&ar->rx_pool_urbs);
+ /* release additional ref done by previous anchor */
+ usb_put_urb(urb);
+
+ /* release additional ref done by current anchor */
usb_free_urb(urb);
carl9170_usb_submit_rx_urb(ar, GFP_ATOMIC);
@@ -364,6 +380,11 @@ void carl9170_usb_handle_tx_err(struct ar9170 *ar)
carl9170_tx_drop(ar, skb);
carl9170_tx_callback(ar, skb);
+
+ /* release additional ref done by previous anchor */
+ usb_put_urb(urb);
+
+ /* release additional ref done by current anchor */
usb_free_urb(urb);
}
}
@@ -553,6 +574,11 @@ static int carl9170_usb_flush(struct ar9170 *ar)
struct sk_buff *skb = (void *)urb->context;
carl9170_tx_drop(ar, skb);
carl9170_tx_callback(ar, skb);
+
+ /* release additional ref done by previous anchor */
+ usb_put_urb(urb);
+
+ /* release additional ref done by current anchor */
usb_free_urb(urb);
}
diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
index 9d912bf..fc97738 100644
--- a/drivers/usb/core/urb.c
+++ b/drivers/usb/core/urb.c
@@ -118,7 +118,18 @@ EXPORT_SYMBOL_GPL(usb_get_urb);
* @anchor: pointer to the anchor
*
* This can be called to have access to URBs which are to be executed
- * without bothering to track them
+ * without bothering to track them.
+ *
+ * NOTE: If the URB in question is not successfully submitted for
+ * transmission after anchoring, the URB must either be:
+ *
+ * a) Unanchored by call to usb_unanchor_urb().
+ * b) Dequeued through usb_get_from_anchor() and then usb_put_urb()
+ * must be called on the URB in question. Else the URB will have
+ * an additional ref.
+ *
+ * NOTE: URB's are automatically un-anchored at USB transfer
+ * completion or killing.
*/
void usb_anchor_urb(struct urb *urb, struct usb_anchor *anchor)
{
@@ -827,8 +838,11 @@ EXPORT_SYMBOL_GPL(usb_wait_anchor_empty_timeout);
* usb_get_from_anchor - get an anchor's oldest urb
* @anchor: the anchor whose urb you want
*
- * this will take the oldest urb from an anchor,
- * unanchor and return it
+ * This will take the oldest URB from an anchor,
+ * unanchor and return it.
+ *
+ * NOTE: This function will not remove the additional ref done by
+ * usb_anchor_urb().
*/
struct urb *usb_get_from_anchor(struct usb_anchor *anchor)
{
diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index 1aae902..a666b95 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -1275,7 +1275,6 @@ struct option_port_private {
u8 *out_buffer[N_OUT_URB];
unsigned long out_busy; /* Bit vector of URBs in use */
int opened;
- struct usb_anchor delayed;
/* Settings for the port */
int rts_state; /* Handshaking pins (outputs) */
diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
index ba54a0a..403713f 100644
--- a/drivers/usb/serial/sierra.c
+++ b/drivers/usb/serial/sierra.c
@@ -1016,6 +1016,9 @@ static int sierra_resume(struct usb_serial *serial)
portdata = usb_get_serial_port_data(port);
while ((urb = usb_get_from_anchor(&portdata->delayed))) {
+ /* release additional ref done by anchor */
+ usb_put_urb(urb);
+
usb_anchor_urb(urb, &portdata->active);
intfdata->in_flight++;
err = usb_submit_urb(urb, GFP_ATOMIC);
diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c
index f35971d..54c449e 100644
--- a/drivers/usb/serial/usb_wwan.c
+++ b/drivers/usb/serial/usb_wwan.c
@@ -669,9 +669,13 @@ static void play_delayed(struct usb_serial_port *port)
err = usb_submit_urb(urb, GFP_ATOMIC);
if (!err) {
data->in_flight++;
+ /* release additional ref done by anchor */
+ usb_put_urb(urb);
} else {
/* we have to throw away the rest */
do {
+ /* release additional ref done by anchor */
+ usb_put_urb(urb);
unbusy_queued_urb(urb, portdata);
usb_autopm_put_interface_no_suspend(port->serial->interface);
} while ((urb = usb_get_from_anchor(&portdata->delayed)));
diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c
index 7ae65fc..400ea6d 100644
--- a/drivers/net/can/usb/ems_usb.c
+++ b/drivers/net/can/usb/ems_usb.c
@@ -626,6 +626,7 @@ static int ems_usb_start(struct ems_usb *dev)
usb_unanchor_urb(urb);
usb_free_coherent(dev->udev, RX_BUFFER_SIZE, buf,
urb->transfer_dma);
+ usb_free_urb(urb);
break;
}
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 9f58330..22d2bc4 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1554,6 +1554,10 @@ int usbnet_resume (struct usb_interface *intf)
skb = (struct sk_buff *)res->context;
retval = usb_submit_urb(res, GFP_ATOMIC);
+
+ /* release additional ref done by anchor */
+ usb_put_urb(res);
+
if (retval < 0) {
dev_kfree_skb_any(skb);
usb_free_urb(res);
diff --git a/drivers/net/wireless/ath/carl9170/usb.c b/drivers/net/wireless/ath/carl9170/usb.c
index 888152c..7ee9f33 100644
--- a/drivers/net/wireless/ath/carl9170/usb.c
+++ b/drivers/net/wireless/ath/carl9170/usb.c
@@ -153,6 +153,10 @@ static void carl9170_usb_submit_data_urb(struct ar9170 *ar)
usb_anchor_urb(urb, &ar->tx_err);
}
+ /* release additional ref done by previous anchor */
+ usb_free_urb(urb);
+
+ /* release additional ref done by current anchor */
usb_free_urb(urb);
if (likely(err == 0))
@@ -229,6 +233,10 @@ static int carl9170_usb_submit_cmd_urb(struct ar9170 *ar)
usb_unanchor_urb(urb);
atomic_dec(&ar->tx_cmd_urbs);
}
+ /* release additional ref done by previous anchor */
+ usb_put_urb(urb);
+
+ /* release additional ref done by current anchor */
usb_free_urb(urb);
return err;
@@ -323,6 +331,10 @@ static int carl9170_usb_submit_rx_urb(struct ar9170 *ar, gfp_t gfp)
atomic_dec(&ar->rx_pool_urbs);
atomic_inc(&ar->rx_anch_urbs);
}
+ /* release additional ref done by previous anchor */
+ usb_put_urb(urb);
+
+ /* release additional ref done by current anchor */
usb_free_urb(urb);
}
}
@@ -345,10 +357,14 @@ static void carl9170_usb_rx_work(struct ar9170 *ar)
carl9170_rx(ar, urb->transfer_buffer,
urb->actual_length);
}
-
usb_anchor_urb(urb, &ar->rx_pool);
+
atomic_inc(&ar->rx_pool_urbs);
+ /* release additional ref done by previous anchor */
+ usb_put_urb(urb);
+
+ /* release additional ref done by current anchor */
usb_free_urb(urb);
carl9170_usb_submit_rx_urb(ar, GFP_ATOMIC);
@@ -364,6 +380,11 @@ void carl9170_usb_handle_tx_err(struct ar9170 *ar)
carl9170_tx_drop(ar, skb);
carl9170_tx_callback(ar, skb);
+
+ /* release additional ref done by previous anchor */
+ usb_put_urb(urb);
+
+ /* release additional ref done by current anchor */
usb_free_urb(urb);
}
}
@@ -553,6 +574,11 @@ static int carl9170_usb_flush(struct ar9170 *ar)
struct sk_buff *skb = (void *)urb->context;
carl9170_tx_drop(ar, skb);
carl9170_tx_callback(ar, skb);
+
+ /* release additional ref done by previous anchor */
+ usb_put_urb(urb);
+
+ /* release additional ref done by current anchor */
usb_free_urb(urb);
}
diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
index 9d912bf..fc97738 100644
--- a/drivers/usb/core/urb.c
+++ b/drivers/usb/core/urb.c
@@ -118,7 +118,18 @@ EXPORT_SYMBOL_GPL(usb_get_urb);
* @anchor: pointer to the anchor
*
* This can be called to have access to URBs which are to be executed
- * without bothering to track them
+ * without bothering to track them.
+ *
+ * NOTE: If the URB in question is not successfully submitted for
+ * transmission after anchoring, the URB must either be:
+ *
+ * a) Unanchored by call to usb_unanchor_urb().
+ * b) Dequeued through usb_get_from_anchor() and then usb_put_urb()
+ * must be called on the URB in question. Else the URB will have
+ * an additional ref.
+ *
+ * NOTE: URB's are automatically un-anchored at USB transfer
+ * completion or killing.
*/
void usb_anchor_urb(struct urb *urb, struct usb_anchor *anchor)
{
@@ -827,8 +838,11 @@ EXPORT_SYMBOL_GPL(usb_wait_anchor_empty_timeout);
* usb_get_from_anchor - get an anchor's oldest urb
* @anchor: the anchor whose urb you want
*
- * this will take the oldest urb from an anchor,
- * unanchor and return it
+ * This will take the oldest URB from an anchor,
+ * unanchor and return it.
+ *
+ * NOTE: This function will not remove the additional ref done by
+ * usb_anchor_urb().
*/
struct urb *usb_get_from_anchor(struct usb_anchor *anchor)
{
diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index 1aae902..a666b95 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -1275,7 +1275,6 @@ struct option_port_private {
u8 *out_buffer[N_OUT_URB];
unsigned long out_busy; /* Bit vector of URBs in use */
int opened;
- struct usb_anchor delayed;
/* Settings for the port */
int rts_state; /* Handshaking pins (outputs) */
diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
index ba54a0a..403713f 100644
--- a/drivers/usb/serial/sierra.c
+++ b/drivers/usb/serial/sierra.c
@@ -1016,6 +1016,9 @@ static int sierra_resume(struct usb_serial *serial)
portdata = usb_get_serial_port_data(port);
while ((urb = usb_get_from_anchor(&portdata->delayed))) {
+ /* release additional ref done by anchor */
+ usb_put_urb(urb);
+
usb_anchor_urb(urb, &portdata->active);
intfdata->in_flight++;
err = usb_submit_urb(urb, GFP_ATOMIC);
diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c
index f35971d..54c449e 100644
--- a/drivers/usb/serial/usb_wwan.c
+++ b/drivers/usb/serial/usb_wwan.c
@@ -669,9 +669,13 @@ static void play_delayed(struct usb_serial_port *port)
err = usb_submit_urb(urb, GFP_ATOMIC);
if (!err) {
data->in_flight++;
+ /* release additional ref done by anchor */
+ usb_put_urb(urb);
} else {
/* we have to throw away the rest */
do {
+ /* release additional ref done by anchor */
+ usb_put_urb(urb);
unbusy_queued_urb(urb, portdata);
usb_autopm_put_interface_no_suspend(port->serial->interface);
} while ((urb = usb_get_from_anchor(&portdata->delayed)));
