[PATCH net] net: usbnet: fix softirq storm on suspend

[PATCH net] net: usbnet: fix softirq storm on suspend

[Bjørn Mork]

Suspending an open usbnet device results in constant
rescheduling of usbnet_bh.

commit 65841fd5 "usbnet: handle remote wakeup asap"
refactored the usbnet_bh code to allow sharing the
urb allocate and submit code with usbnet_resume. In
this process, a test for, and immediate return on,
ENOLINK from rx_submit was unintentionally dropped.

The rx queue will not grow if rx_submit fails,
making usbnet_bh reschedule itself.  This results
in a softirq storm if the error is persistent.
rx_submit translates the usb_submit_urb error
EHOSTUNREACH into ENOLINK, so this is an expected
and persistent error for a suspended device. The
old code tested for this condition and avoided
rescheduling.  Putting this test back.

Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # v3.5
Cc: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Cc: Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org>
Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
Sorry for not noticing this before, but commit 65841fd5
makes usbnet autosuspend completely unusable.  The device
is suspended fine, but burning one CPU core at full load
uses a tiny bit more power making the power saving 
negative...

I hope this can go into 3.6 and 3.5-stable ASAP. It is
a hard to notice regression, but all the same a serious
one.


Thanks,
Bjørn


 drivers/net/usb/usbnet.c |   16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index fd4b26d..fc9f578 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1201,19 +1201,26 @@ deferred:
 }
 EXPORT_SYMBOL_GPL(usbnet_start_xmit);
 
-static void rx_alloc_submit(struct usbnet *dev, gfp_t flags)
+static int rx_alloc_submit(struct usbnet *dev, gfp_t flags)
 {
 	struct urb	*urb;
 	int		i;
+	int		ret = 0;
 
 	/* don't refill the queue all at once */
 	for (i = 0; i < 10 && dev->rxq.qlen < RX_QLEN(dev); i++) {
 		urb = usb_alloc_urb(0, flags);
 		if (urb != NULL) {
-			if (rx_submit(dev, urb, flags) == -ENOLINK)
-				return;
+			ret = rx_submit(dev, urb, flags);
+			if (ret)
+				goto err;
+		} else {
+			ret = -ENOMEM;
+			goto err;
 		}
 	}
+err:
+	return ret;
 }
 
 /*-------------------------------------------------------------------------*/
@@ -1257,7 +1264,8 @@ static void usbnet_bh (unsigned long param)
 		int	temp = dev->rxq.qlen;
 
 		if (temp < RX_QLEN(dev)) {
-			rx_alloc_submit(dev, GFP_ATOMIC);
+			if (rx_alloc_submit(dev, GFP_ATOMIC) == -ENOLINK)
+				return;
 			if (temp != dev->rxq.qlen)
 				netif_dbg(dev, link, dev->net,
 					  "rxqlen %d --> %d\n",
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net] net: usbnet: fix softirq storm on suspend

[David Miller]

From: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
Date: Mon,  3 Sep 2012 10:26:18 +0200

> Suspending an open usbnet device results in constant
> rescheduling of usbnet_bh.
> 
> commit 65841fd5 "usbnet: handle remote wakeup asap"
> refactored the usbnet_bh code to allow sharing the
> urb allocate and submit code with usbnet_resume. In
> this process, a test for, and immediate return on,
> ENOLINK from rx_submit was unintentionally dropped.
> 
> The rx queue will not grow if rx_submit fails,
> making usbnet_bh reschedule itself.  This results
> in a softirq storm if the error is persistent.
> rx_submit translates the usb_submit_urb error
> EHOSTUNREACH into ENOLINK, so this is an expected
> and persistent error for a suspended device. The
> old code tested for this condition and avoided
> rescheduling.  Putting this test back.
> 
> Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # v3.5
> Cc: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> Cc: Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org>
> Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net] net: usbnet: fix softirq storm on suspend

[Ming Lei]

On Mon, Sep 3, 2012 at 4:26 PM, Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org> wrote:
> Suspending an open usbnet device results in constant
> rescheduling of usbnet_bh.
>
> commit 65841fd5 "usbnet: handle remote wakeup asap"
> refactored the usbnet_bh code to allow sharing the
> urb allocate and submit code with usbnet_resume. In
> this process, a test for, and immediate return on,
> ENOLINK from rx_submit was unintentionally dropped.
>
> The rx queue will not grow if rx_submit fails,
> making usbnet_bh reschedule itself.  This results
> in a softirq storm if the error is persistent.
> rx_submit translates the usb_submit_urb error
> EHOSTUNREACH into ENOLINK, so this is an expected
> and persistent error for a suspended device. The
> old code tested for this condition and avoided
> rescheduling.  Putting this test back.
>
> Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # v3.5
> Cc: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> Cc: Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org>
> Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
> ---
> Sorry for not noticing this before, but commit 65841fd5
> makes usbnet autosuspend completely unusable.  The device
> is suspended fine, but burning one CPU core at full load
> uses a tiny bit more power making the power saving
> negative...

I am wondering how you can reproduce the issue.

usbnet_terminate_urbs is called inside usbnet_suspend to
consume all URBs and SKBs, and rx_alloc_submit can't be
called during the period because of !netif_device_present().
Once usbnet_terminate_urbs and netif_device_attach() are
completed, who will schedule tasklet to trigger rx_alloc_submit?

>
> I hope this can go into 3.6 and 3.5-stable ASAP. It is
> a hard to notice regression, but all the same a serious
> one.

Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net] net: usbnet: fix softirq storm on suspend

[Bjørn Mork]

Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> writes:
> On Mon, Sep 3, 2012 at 4:26 PM, Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org> wrote:
>
>> Sorry for not noticing this before, but commit 65841fd5
>> makes usbnet autosuspend completely unusable.  The device
>> is suspended fine, but burning one CPU core at full load
>> uses a tiny bit more power making the power saving
>> negative...
>
> I am wondering how you can reproduce the issue.

That's easy:

- Take any usbnet device supporting remote wakeup (and of course with a
  minidriver supporting it as well),
- enable autosuspend,
- ip link set dev ethX/usbX/wwanX up

And watch ksoftirqd/X use 100% of one of your CPU cores.

I'd very much like to hear the details if you are unable to reproduce
the bug using this simple test.

> usbnet_terminate_urbs is called inside usbnet_suspend to
> consume all URBs and SKBs, and rx_alloc_submit can't be
> called during the period because of !netif_device_present().

Huh???? We do support suspending a USB device while the network device
is up and running. That's the whole idea here, isn't it? I.e.
netif_device_present() is *expected* to be true while the USB device is
suspended.

> Once usbnet_terminate_urbs and netif_device_attach() are
> completed, who will schedule tasklet to trigger rx_alloc_submit?

That's a good question.  It sure would be nice if that never happened
without waking the device first.  But there are just too many call sites
for me to follow:

bjorn@nemi:/usr/local/src/git/linux$ grep tasklet_schedule drivers/net/usb/usbnet.c
                tasklet_schedule(&dev->bh);
 * but tasklet_schedule() doesn't.  hope the failure is rare.
                        tasklet_schedule (&dev->bh);
        tasklet_schedule(&dev->bh);
                tasklet_schedule(&dev->bh);
        tasklet_schedule (&dev->bh);
                        tasklet_schedule (&dev->bh);
                                tasklet_schedule (&dev->bh);
        tasklet_schedule (&dev->bh);
                                tasklet_schedule (&dev->bh);
                        tasklet_schedule (&dev->bh);


And I do believe the code before your change demonstrated that the
original authors had the same view.  There was an explicit exception for
just this case, and I do assume that was put there for a good
reason. usbnet_bh() will be called while the device is suspended, and we
must avoid making it reschedule itself in this case.

Anyway, the ENOLINK test was there.  You removed it with no explanation
whatsoever. It is *your* call to verify and explain to us why this test
is unnecessary, not mine.


Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net] net: usbnet: fix softirq storm on suspend

[Bjørn Mork]

Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org> writes:

> And I do believe the code before your change demonstrated that the
> original authors had the same view.  There was an explicit exception for
> just this case, and I do assume that was put there for a good
> reason. usbnet_bh() will be called while the device is suspended, and we
> must avoid making it reschedule itself in this case.
>
> Anyway, the ENOLINK test was there.  You removed it with no explanation
> whatsoever. It is *your* call to verify and explain to us why this test
> is unnecessary, not mine.

For your convienience, all the reasons why this code ended up like it
was are documented in the netdev patchwork:
http://patchwork.ozlabs.org/patch/59488/

There were different proposed solutions circulating, before the test for
ENOLINK was chosen.  No-one challenged the fact that some test for a
suspended device was needed here.


Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net] net: usbnet: fix softirq storm on suspend

[Ming Lei]

On Tue, Sep 4, 2012 at 3:49 PM, Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org> wrote:
>
> That's easy:
>
> - Take any usbnet device supporting remote wakeup (and of course with a
>   minidriver supporting it as well),
> - enable autosuspend,
> - ip link set dev ethX/usbX/wwanX up
>
> And watch ksoftirqd/X use 100% of one of your CPU cores.
>
> I'd very much like to hear the details if you are unable to reproduce
> the bug using this simple test.

I have no such kind of devices, and can't reproduce the problem with
a cdc-ether gadget built by one omap3 beagleboard.

>> Once usbnet_terminate_urbs and netif_device_attach() are
>> completed, who will schedule tasklet to trigger rx_alloc_submit?
>
> That's a good question.  It sure would be nice if that never happened
> without waking the device first.  But there are just too many call sites
> for me to follow:

To be honest, your patch is correct, and I am just curious about
the reason why tasklet is scheduled after the device is suspended.

>
> bjorn@nemi:/usr/local/src/git/linux$ grep tasklet_schedule drivers/net/usb/usbnet.c
>                 tasklet_schedule(&dev->bh);
>  * but tasklet_schedule() doesn't.  hope the failure is rare.
>                         tasklet_schedule (&dev->bh);
>         tasklet_schedule(&dev->bh);
>                 tasklet_schedule(&dev->bh);
>         tasklet_schedule (&dev->bh);
>                         tasklet_schedule (&dev->bh);
>                                 tasklet_schedule (&dev->bh);
>         tasklet_schedule (&dev->bh);
>                                 tasklet_schedule (&dev->bh);
>                         tasklet_schedule (&dev->bh);

Could you add some debug info inside the caller to see which one
cause the tasklet to be scheduled?


Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net] net: usbnet: fix softirq storm on suspend

[Bjørn Mork]

Ming Lei <ming.lei@canonical.com> writes:
> On Tue, Sep 4, 2012 at 3:49 PM, Bjørn Mork <bjorn@mork.no> wrote:
>>
>> That's easy:
>>
>> - Take any usbnet device supporting remote wakeup (and of course with a
>>   minidriver supporting it as well),
>> - enable autosuspend,
>> - ip link set dev ethX/usbX/wwanX up
>>
>> And watch ksoftirqd/X use 100% of one of your CPU cores.
>>
>> I'd very much like to hear the details if you are unable to reproduce
>> the bug using this simple test.
>
> I have no such kind of devices, and can't reproduce the problem with
> a cdc-ether gadget built by one omap3 beagleboard.

I believe any Ericsson or Gobi modem would do, and most likely other USB
networking devices too.

I haven't explored the gadget.  Doesn't it support remote wakeup?  Well,
it doesn't really have to just for testing this.  You just have to fake
the remote wakeup support, either in the gadget or in the device
driver.  It doesn't matter whether it works or not.  The point is making
the driver suspend the USB device while the network device is running.

>>> Once usbnet_terminate_urbs and netif_device_attach() are
>>> completed, who will schedule tasklet to trigger rx_alloc_submit?
>>
>> That's a good question.  It sure would be nice if that never happened
>> without waking the device first.  But there are just too many call sites
>> for me to follow:
>
> To be honest, your patch is correct, and I am just curious about
> the reason why tasklet is scheduled after the device is suspended.
>
>>
>> bjorn@nemi:/usr/local/src/git/linux$ grep tasklet_schedule drivers/net/usb/usbnet.c
>>                 tasklet_schedule(&dev->bh);
>>  * but tasklet_schedule() doesn't.  hope the failure is rare.
>>                         tasklet_schedule (&dev->bh);
>>         tasklet_schedule(&dev->bh);
>>                 tasklet_schedule(&dev->bh);
>>         tasklet_schedule (&dev->bh);
>>                         tasklet_schedule (&dev->bh);
>>                                 tasklet_schedule (&dev->bh);
>>         tasklet_schedule (&dev->bh);
>>                                 tasklet_schedule (&dev->bh);
>>                         tasklet_schedule (&dev->bh);
>
> Could you add some debug info inside the caller to see which one
> cause the tasklet to be scheduled?


Goode idea.  I replaced the tasklet_schedule() calls with

#define DBG_TASKLET(bh) { \
 pr_err("%s scheduling tasklet %p\n", __func__, bh); \
 tasklet_schedule(bh); \
}

and added a useless WARN_ON around the test:

		if (temp < RX_QLEN(dev)) {
			if (WARN_ON_ONCE(rx_alloc_submit(dev, GFP_ATOMIC) == -ENOLINK))
				return;


and got:

Sep  4 12:22:48 nemi kernel: [50270.964818] qmi_wwan 2-4:1.8: wdm1_resume
Sep  4 12:22:48 nemi kernel: [50270.964832] qmi_wwan 2-4:1.19: wdm2_resume
Sep  4 12:22:48 nemi kernel: [50270.964845] usbnet_open scheduling tasklet ffff88014a8529d0
Sep  4 12:22:48 nemi kernel: [50270.964857] qmi_wwan 2-4:1.8: qmi_wwan_manage_power() pmcount=0, on=1
Sep  4 12:22:48 nemi kernel: [50270.964921] qmi_wwan 2-4:1.8: wwan1: rxqlen 0 --> 10
Sep  4 12:22:48 nemi kernel: [50270.964926] usbnet_bh scheduling tasklet ffff88014a8529d0
Sep  4 12:22:48 nemi kernel: [50270.964962] qmi_wwan 2-4:1.8: wwan1: rxqlen 10 --> 20
Sep  4 12:22:48 nemi kernel: [50270.964966] usbnet_bh scheduling tasklet ffff88014a8529d0
Sep  4 12:22:48 nemi kernel: [50270.965016] qmi_wwan 2-4:1.8: wwan1: rxqlen 20 --> 30
Sep  4 12:22:48 nemi kernel: [50270.965020] usbnet_bh scheduling tasklet ffff88014a8529d0
Sep  4 12:22:48 nemi kernel: [50270.965057] qmi_wwan 2-4:1.8: wwan1: rxqlen 30 --> 40
Sep  4 12:22:48 nemi kernel: [50270.965061] usbnet_bh scheduling tasklet ffff88014a8529d0
Sep  4 12:22:48 nemi kernel: [50270.965097] qmi_wwan 2-4:1.8: wwan1: rxqlen 40 --> 50
Sep  4 12:22:48 nemi kernel: [50270.965101] usbnet_bh scheduling tasklet ffff88014a8529d0
Sep  4 12:22:48 nemi kernel: [50270.965136] qmi_wwan 2-4:1.8: wwan1: rxqlen 50 --> 60
Sep  4 12:22:48 nemi kernel: [50270.972270] defer_bh scheduling tasklet ffff88014a8529d0
Sep  4 12:22:48 nemi kernel: [50271.040290] defer_bh scheduling tasklet ffff88014a8529d0
Sep  4 12:22:48 nemi kernel: [50271.240286] defer_bh scheduling tasklet ffff88014a8529d0
Sep  4 12:22:49 nemi kernel: [50272.040536] defer_bh scheduling tasklet ffff88014a8529d0
Sep  4 12:22:51 nemi kernel: [50274.040121] qmi_wwan 2-4:1.19: wdm2_suspend
Sep  4 12:22:51 nemi kernel: [50274.040234] qmi_wwan 2-4:1.8: wdm1_suspend
Sep  4 12:22:51 nemi kernel: [50274.040263] defer_bh scheduling tasklet ffff88014a8529d0
Sep  4 12:22:51 nemi kernel: [50274.040599] ------------[ cut here ]------------
Sep  4 12:22:51 nemi kernel: [50274.040612] WARNING: at /usr/local/src/git/linux/drivers/net/usb/usbnet.c:1271 tasklet_action+0x76/0xc4()
Sep  4 12:22:51 nemi kernel: [50274.040617] Hardware name: 2776LEG
Sep  4 12:22:51 nemi kernel: [50274.040620] Modules linked in: qmi_wwan(O) usbnet(O) nfnetlink_log nfnetlink option nls_utf8 isofs usb_storage uas usbmon cdc_wdm(O) mii rfcomm bnep xt_multiport iptable_filter ip_tables cpufreq_userspace cpufreq_stats cpufreq_conservative cpufreq_powersave binfmt_misc xt_hl ip6table_filter ip6_tables x_tables fuse nfsd nfs_acl nfs lockd fscache sunrpc 8021q garp stp llc tun ext2 loop snd_hda_codec_conexant snd_hda_intel snd_hda_codec snd_hwdep snd_pcm_oss snd_mixer_oss snd_pcm arc4 iwldvm mac80211 snd_page_alloc thinkpad_acpi nvram snd_seq_midi snd_seq_midi_event snd_rawmidi snd_seq coretemp kvm_intel mei btusb qcserial kvm usb_wwan usbserial bluetooth crc16 snd_timer snd_seq_device i915 microcode iwlwifi psmouse serio_raw snd evdev lpc_ich drm_kms_helper i2c_i801 mfd_core cfg80211 drm i2c_algo_bit i2c_core acpi_cpufreq wmi soundcore rfkill battery ac mperf processor button video ext3 mbcache jbd sha256_generic ablk_helper cryptd aes_x86_64 aes_generic cbc dm_crypt dm_mod nbd s
Sep  4 12:22:51 nemi kernel: g sd_mod crc_t10dif sr_mod cdrom thermal thermal_sys ahci libahci libata uhci_hcd ehci_hcd scsi_mod e1000e usbcore usb_common [last unloaded: usbnet]
Sep  4 12:22:51 nemi kernel: [50274.040857] Pid: 7758, comm: kworker/0:0 Tainted: G        W  O 3.6.0-rc4+ #32
Sep  4 12:22:51 nemi kernel: [50274.040861] Call Trace:
Sep  4 12:22:51 nemi kernel: [50274.040865]  <IRQ>  [<ffffffff8103e0ed>] ? warn_slowpath_common+0x78/0x8c
Sep  4 12:22:51 nemi kernel: [50274.040883]  [<ffffffff810457b2>] ? tasklet_action+0x76/0xc4
Sep  4 12:22:51 nemi kernel: [50274.040890]  [<ffffffff810451e5>] ? __do_softirq+0xd9/0x1d1
Sep  4 12:22:51 nemi kernel: [50274.040898]  [<ffffffff8109f761>] ? handle_irq_event_percpu+0x172/0x191
Sep  4 12:22:51 nemi kernel: [50274.040908]  [<ffffffff81371bfc>] ? call_softirq+0x1c/0x30
Sep  4 12:22:51 nemi kernel: [50274.040918]  [<ffffffff8100fa4b>] ? do_softirq+0x3f/0x79
Sep  4 12:22:51 nemi kernel: [50274.040924]  [<ffffffff81044f9e>] ? irq_exit+0x44/0xb1
Sep  4 12:22:51 nemi kernel: [50274.040931]  [<ffffffff8100f326>] ? do_IRQ+0x94/0xaa
Sep  4 12:22:51 nemi kernel: [50274.040941]  [<ffffffff8136b5ea>] ? common_interrupt+0x6a/0x6a
Sep  4 12:22:51 nemi kernel: [50274.040944]  <EOI>  [<ffffffff8126fa99>] ? dev_get_drvdata+0x15/0x19
Sep  4 12:22:51 nemi kernel: [50274.040969]  [<ffffffffa04ee4ca>] ? usb_wwan_suspend+0x70/0xd7 [usb_wwan]
Sep  4 12:22:51 nemi kernel: [50274.040983]  [<ffffffffa02aa5d0>] ? usb_serial_suspend+0x35/0x74 [usbserial]
Sep  4 12:22:51 nemi kernel: [50274.041010]  [<ffffffffa00d313b>] ? usb_suspend_both+0x8e/0x19c [usbcore]
Sep  4 12:22:51 nemi kernel: [50274.041035]  [<ffffffffa00d3271>] ? usb_runtime_suspend+0x28/0x4c [usbcore]
Sep  4 12:22:51 nemi kernel: [50274.041043]  [<ffffffff81277943>] ? __rpm_callback+0x33/0x57
Sep  4 12:22:51 nemi kernel: [50274.041050]  [<ffffffff81277ee1>] ? rpm_suspend+0x30e/0x53c
Sep  4 12:22:51 nemi kernel: [50274.041059]  [<ffffffff81278e1a>] ? pm_runtime_set_autosuspend_delay+0x4b/0x4b
Sep  4 12:22:51 nemi kernel: [50274.041066]  [<ffffffff81278e8e>] ? pm_runtime_work+0x74/0x94
Sep  4 12:22:51 nemi kernel: [50274.041075]  [<ffffffff81055d3d>] ? process_one_work+0x1ff/0x311
Sep  4 12:22:51 nemi kernel: [50274.041083]  [<ffffffff8105604a>] ? worker_thread+0x1fb/0x2fb
Sep  4 12:22:51 nemi kernel: [50274.041091]  [<ffffffff81055e4f>] ? process_one_work+0x311/0x311
Sep  4 12:22:51 nemi kernel: [50274.041099]  [<ffffffff81055e4f>] ? process_one_work+0x311/0x311
Sep  4 12:22:51 nemi kernel: [50274.041106]  [<ffffffff81059861>] ? kthread+0x81/0x89
Sep  4 12:22:51 nemi kernel: [50274.041115]  [<ffffffff81371b04>] ? kernel_thread_helper+0x4/0x10
Sep  4 12:22:51 nemi kernel: [50274.041123]  [<ffffffff810597e0>] ? kthread_freezable_should_stop+0x53/0x53
Sep  4 12:22:51 nemi kernel: [50274.041130]  [<ffffffff81371b00>] ? gs_change+0x13/0x13
Sep  4 12:22:51 nemi kernel: [50274.041135] ---[ end trace 8293423100f8e272 ]---
Sep  4 12:22:53 nemi kernel: [50276.048112] qmi_wwan 2-4:1.8: wwan1: Delaying transmission for resumption
Sep  4 12:22:53 nemi kernel: [50276.097673] qmi_wwan 2-4:1.8: wdm1_resume
Sep  4 12:22:53 nemi kernel: [50276.097743] usbnet_resume scheduling tasklet ffff88014a8529d0
Sep  4 12:22:53 nemi kernel: [50276.097754] qmi_wwan 2-4:1.19: wdm2_resume
Sep  4 12:22:53 nemi kernel: [50276.097789] defer_bh scheduling tasklet ffff88014a8529d0
Sep  4 12:22:53 nemi kernel: [50276.097847] qmi_wwan 2-4:1.8: wwan1: rxqlen 10 --> 20
Sep  4 12:22:53 nemi kernel: [50276.097851] usbnet_bh scheduling tasklet ffff88014a8529d0
Sep  4 12:22:53 nemi kernel: [50276.097890] qmi_wwan 2-4:1.8: wwan1: rxqlen 20 --> 30
Sep  4 12:22:53 nemi kernel: [50276.097895] usbnet_bh scheduling tasklet ffff88014a8529d0
Sep  4 12:22:53 nemi kernel: [50276.097930] qmi_wwan 2-4:1.8: wwan1: rxqlen 30 --> 40
Sep  4 12:22:53 nemi kernel: [50276.097935] usbnet_bh scheduling tasklet ffff88014a8529d0
Sep  4 12:22:53 nemi kernel: [50276.097970] qmi_wwan 2-4:1.8: wwan1: rxqlen 40 --> 50
Sep  4 12:22:53 nemi kernel: [50276.097974] usbnet_bh scheduling tasklet ffff88014a8529d0
Sep  4 12:22:53 nemi kernel: [50276.098010] qmi_wwan 2-4:1.8: wwan1: rxqlen 50 --> 60
Sep  4 12:22:56 nemi kernel: [50278.824340] qmi_wwan 2-4:1.19: wdm2_suspend
Sep  4 12:22:56 nemi kernel: [50278.824441] defer_bh scheduling tasklet ffff88014a8529d0
Sep  4 12:22:56 nemi kernel: [50278.824708] qmi_wwan 2-4:1.8: wdm1_suspend
Sep  4 12:22:56 nemi kernel: [50278.824833] defer_bh scheduling tasklet ffff88014a8529d0
Sep  4 12:22:56 nemi kernel: [50278.824916] ------------[ cut here ]------------
Sep  4 12:22:56 nemi kernel: [50278.824929] WARNING: at /usr/local/src/git/linux/drivers/net/usb/usbnet.c:1271 tasklet_action+0x76/0xc4()
Sep  4 12:22:56 nemi kernel: [50278.824933] Hardware name: 2776LEG
Sep  4 12:22:56 nemi kernel: [50278.824936] Modules linked in: qmi_wwan(O) usbnet(O) nfnetlink_log nfnetlink option nls_utf8 isofs usb_storage uas usbmon cdc_wdm(O) mii rfcomm bnep xt_multiport iptable_filter ip_tables cpufreq_userspace cpufreq_stats cpufreq_conservative cpufreq_powersave binfmt_misc xt_hl ip6table_filter ip6_tables x_tables fuse nfsd nfs_acl nfs lockd fscache sunrpc 8021q garp stp llc tun ext2 loop snd_hda_codec_conexant snd_hda_intel snd_hda_codec snd_hwdep snd_pcm_oss snd_mixer_oss snd_pcm arc4 iwldvm mac80211 snd_page_alloc thinkpad_acpi nvram snd_seq_midi snd_seq_midi_event snd_rawmidi snd_seq coretemp kvm_intel mei btusb qcserial kvm usb_wwan usbserial bluetooth crc16 snd_timer snd_seq_device i915 microcode iwlwifi psmouse serio_raw snd evdev lpc_ich drm_kms_helper i2c_i801 mfd_core cfg80211 drm i2c_algo_bit i2c_core acpi_cpufreq wmi soundcore rfkill battery ac mperf processor button video ext3 mbcache jbd sha256_generic ablk_helper cryptd aes_x86_64 aes_generic cbc dm_crypt dm_mod nbd s
Sep  4 12:22:56 nemi kernel: g sd_mod crc_t10dif sr_mod cdrom thermal thermal_sys ahci libahci libata uhci_hcd ehci_hcd scsi_mod e1000e usbcore usb_common [last unloaded: usbnet]
Sep  4 12:22:56 nemi kernel: [50278.825189] Pid: 0, comm: swapper/0 Tainted: G        W  O 3.6.0-rc4+ #32
Sep  4 12:22:56 nemi kernel: [50278.825193] Call Trace:
Sep  4 12:22:56 nemi kernel: [50278.825197]  <IRQ>  [<ffffffff8103e0ed>] ? warn_slowpath_common+0x78/0x8c
Sep  4 12:22:56 nemi kernel: [50278.825215]  [<ffffffff810457b2>] ? tasklet_action+0x76/0xc4
Sep  4 12:22:56 nemi kernel: [50278.825222]  [<ffffffff810451e5>] ? __do_softirq+0xd9/0x1d1
Sep  4 12:22:56 nemi kernel: [50278.825231]  [<ffffffff8109f761>] ? handle_irq_event_percpu+0x172/0x191
Sep  4 12:22:56 nemi kernel: [50278.825241]  [<ffffffff81371bfc>] ? call_softirq+0x1c/0x30
Sep  4 12:22:56 nemi kernel: [50278.825250]  [<ffffffff8100fa4b>] ? do_softirq+0x3f/0x79
Sep  4 12:22:56 nemi kernel: [50278.825257]  [<ffffffff81044f9e>] ? irq_exit+0x44/0xb1
Sep  4 12:22:56 nemi kernel: [50278.825264]  [<ffffffff8100f326>] ? do_IRQ+0x94/0xaa
Sep  4 12:22:56 nemi kernel: [50278.825274]  [<ffffffff8136b5ea>] ? common_interrupt+0x6a/0x6a
Sep  4 12:22:56 nemi kernel: [50278.825277]  <EOI>  [<ffffffffa0179150>] ? acpi_idle_enter_bm+0x276/0x2ad [processor]
Sep  4 12:22:56 nemi kernel: [50278.825314]  [<ffffffffa017914c>] ? acpi_idle_enter_bm+0x272/0x2ad [processor]
Sep  4 12:22:56 nemi kernel: [50278.825325]  [<ffffffff8129046f>] ? cpuidle_enter_state+0xa/0x30
Sep  4 12:22:56 nemi kernel: [50278.825332]  [<ffffffff81290bba>] ? cpuidle_idle_call+0xd4/0x14e
Sep  4 12:22:56 nemi kernel: [50278.825340]  [<ffffffff81014c19>] ? cpu_idle+0xae/0xf7
Sep  4 12:22:56 nemi kernel: [50278.825349]  [<ffffffff816a9ccb>] ? start_kernel+0x3b8/0x3c3
Sep  4 12:22:56 nemi kernel: [50278.825357]  [<ffffffff816a974c>] ? kernel_init+0x1c9/0x1c9
Sep  4 12:22:56 nemi kernel: [50278.825365]  [<ffffffff816a93e1>] ? x86_64_start_kernel+0x102/0x10f
Sep  4 12:22:56 nemi kernel: [50278.825371] ---[ end trace 8293423100f8e273 ]---
Sep  4 12:22:56 nemi kernel: [50279.502537] qmi_wwan 2-4:1.8: qmi_wwan_manage_power() pmcount=1, on=0
Sep  4 12:22:57 nemi kernel: [50279.549214] qmi_wwan 2-4:1.8: wdm1_resume
Sep  4 12:22:57 nemi kernel: [50279.549229] qmi_wwan 2-4:1.19: wdm2_resume
Sep  4 12:22:59 nemi kernel: [50281.824317] qmi_wwan 2-4:1.19: wdm2_suspend
Sep  4 12:22:59 nemi kernel: [50281.824335] qmi_wwan 2-4:1.8: wdm1_suspend



So the code trigging this seems to be 


static enum skb_state defer_bh(struct usbnet *dev, struct sk_buff *skb,
		struct sk_buff_head *list, enum skb_state state)
{
	unsigned long		flags;
	enum skb_state 		old_state;
	struct skb_data *entry = (struct skb_data *) skb->cb;

	spin_lock_irqsave(&list->lock, flags);
	old_state = entry->state;
	entry->state = state;
	__skb_unlink(skb, list);
	spin_unlock(&list->lock);
	spin_lock(&dev->done.lock);
	__skb_queue_tail(&dev->done, skb);
	if (dev->done.qlen == 1)
		tasklet_schedule(&dev->bh);
	spin_unlock_irqrestore(&dev->done.lock, flags);
	return old_state;
}


Hmm, I should probably dump stack here as well.. Anyway, it's a start.
I certainly have no clue if this coode could or should be changed.  I
just don't understand all the finer details.  So I don't think I am
going to try refining this any further.  The old strategy worked before
3.5 and it works after my fix.


Bjørn

Re: [PATCH net] net: usbnet: fix softirq storm on suspend

[Ming Lei]

[-- Attachment #1: Type: text/plain, Size: 1673 bytes --]

On Tue, Sep 4, 2012 at 6:36 PM, Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org> wrote:
>
> I believe any Ericsson or Gobi modem would do, and most likely other USB
> networking devices too.
>
> I haven't explored the gadget.  Doesn't it support remote wakeup?  Well,

No.

> it doesn't really have to just for testing this.  You just have to fake
> the remote wakeup support, either in the gadget or in the device
> driver.  It doesn't matter whether it works or not.  The point is making
> the driver suspend the USB device while the network device is running.

The attachment patch is what I have been using to make it support remote
wakeup, and looks it works wrt. runtime PM.


>
>
>
> So the code trigging this seems to be
>
>
> static enum skb_state defer_bh(struct usbnet *dev, struct sk_buff *skb,
>                 struct sk_buff_head *list, enum skb_state state)
> {
>         unsigned long           flags;
>         enum skb_state          old_state;
>         struct skb_data *entry = (struct skb_data *) skb->cb;
>
>         spin_lock_irqsave(&list->lock, flags);
>         old_state = entry->state;
>         entry->state = state;
>         __skb_unlink(skb, list);
>         spin_unlock(&list->lock);
>         spin_lock(&dev->done.lock);
>         __skb_queue_tail(&dev->done, skb);
>         if (dev->done.qlen == 1)
>                 tasklet_schedule(&dev->bh);
>         spin_unlock_irqrestore(&dev->done.lock, flags);
>         return old_state;
> }
>
>
> Hmm, I should probably dump stack here as well.. Anyway, it's a start.

Maybe a debug message in defer_bh is enough, :-)


Thanks,
--
Ming Lei

[-- Attachment #2: usb-gadget-remote-wakeup.patch --]
[-- Type: application/octet-stream, Size: 1087 bytes --]

diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index a28f6ff..694f639 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -236,6 +236,7 @@ static int __init rndis_do_config(struct usb_configuration *c)
 		c->descriptors = otg_desc;
 		c->bmAttributes |= USB_CONFIG_ATT_WAKEUP;
 	}
+	c->bmAttributes |= USB_CONFIG_ATT_WAKEUP;
 
 	return rndis_bind_config(c, hostaddr);
 }
@@ -268,6 +269,7 @@ static int __init eth_do_config(struct usb_configuration *c)
 		c->descriptors = otg_desc;
 		c->bmAttributes |= USB_CONFIG_ATT_WAKEUP;
 	}
+	c->bmAttributes |= USB_CONFIG_ATT_WAKEUP;
 
 	if (use_eem)
 		return eem_bind_config(c);
diff --git a/drivers/usb/gadget/u_ether.c b/drivers/usb/gadget/u_ether.c
index 47cf48b..197fcc0 100644
--- a/drivers/usb/gadget/u_ether.c
+++ b/drivers/usb/gadget/u_ether.c
@@ -497,6 +497,7 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
 	struct usb_ep		*in;
 	u16			cdc_filter;
 
+	usb_gadget_wakeup(dev->gadget);
 	spin_lock_irqsave(&dev->lock, flags);
 	if (dev->port_usb) {
 		in = dev->port_usb->in_ep;