Re: [PATCH] usbnet: ignore get interface retval of -EINPROGRESS
flat view
From: Jim Wylder <hidden>
Date: 2011-09-05 16:29:09
Subsystem:
networking drivers, the rest, usb "usbnet" driver framework, usb networking drivers · Maintainers:
Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds, Oliver Neukum
When calling pm_runtime_get, usb_autopm_get_interface_async treats a return value of -EINPROGRESS as a success and increments the usage count. Since the interface is resuming, it is safe for usbnet_start_xmit to submit the urb. If instead, usbnet_start_xmit treats this as an error the packet will be dropped. Additionally, a corresponding tx_complete will not run to offset the earlier increment of the usage count from the call to usb_autopm_get_interface_async. Signed-off-by: James Wylder <redacted> --- drivers/net/usb/usbnet.c | 16 +++++++++++++++- 1 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index ce395fe..7dcef05 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c@@ -81,6 +81,19 @@ /*-------------------------------------------------------------------------*/ +/* When power management is configured, usb_autopm_get_interface_async() + * will return -EINPROGRESS to signify that the interface was already + * resuming at the time that the function was called. In our case + * (all cases?) this not an error condition. + * */ +#ifdef CONFIG_PM +#define USBNET_PM_INTF_ALREADY_RESUMING(x) (x == -EINPROGRESS) +#else +#define USBNET_PM_INTF_ALREADY_RESUMING(x) 0 +#endif + +/*-------------------------------------------------------------------------*/ + // randomly generated ethernet address static u8 node_id [ETH_ALEN];
@@ -1042,6 +1055,7 @@ EXPORT_SYMBOL_GPL(usbnet_tx_timeout); /*-------------------------------------------------------------------------*/ + netdev_tx_t usbnet_start_xmit (struct sk_buff *skb, struct net_device *net) {
@@ -1105,7 +1119,7 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb, spin_lock_irqsave(&dev->txq.lock, flags); retval = usb_autopm_get_interface_async(dev->intf); - if (retval < 0) { + if (retval < 0 && !USBNET_PM_INTF_ALREADY_RESUMING(retval)) { spin_unlock_irqrestore(&dev->txq.lock, flags); goto drop; }
--
1.7.6
On Sun, Sep 4, 2011 at 5:16 AM, Oliver Neukum <oliver@neukum.org> wrote:
> Am Samstag, 3. September 2011, 19:32:44 schrieb Jim Wylder:
>
> Hi,
>
>> Thanks for the quick feedback. True usbnet_start_xmit() could be
>> running at anytime, but the usb_autopm_get_interface_async() will only
>> return -EINPROGRESS when rpm_resume detects that
>> dev->power.runtime_status == RPM_RESUMING. My understanding is that
>> for an asynchronous request, the promise that the device is resuming
>> would be equivalent to cases where usb_autopm_get_interface_async()
>> returns success.
>
> If CONFIG_PM is set.
>
>> In all other cases, when we are not attempting to resume an already
>> resuming interface, this change should have no impact.
>>
>> Are you recommending that I add an additional check for DEV_ASLEEP,
>
> The check is already there but depending on CONFIG_PM.
>
>> possibly to decide whether to drop the or continue on? Or am I
>> missing your point? I had not done anything similar because my
>> understanding was that knowing that the device is in fact resuming
>> would be sufficient.
>
> At that point it is sufficient. Upon further thought it looks like your check
> is correct.
> We just need to make very sure that for the decision to queue or
> to submit EVENT_DEV_ASLEEP is relevant.
> Would you consider defining a macro with a nice name for the
> ==0 || == -EINPROGRESS check, so that any reader knows what
> this is about? This is because I doubt only usbnet is affected.
>
> Regards
> Oliver
>