Re: usbnet: driver_info-stop required to stop USB interrupts?

2014-03-24 Thread Grant Grundler
On Fri, Mar 21, 2014 at 1:33 AM, Oliver Neukum oneu...@suse.de wrote: ... Very well. Thorough testing is good. I'll wait for the result. Could you notify me of the final outcome? Ship it. :) The testing so far has completed over 15K iterations of unload/reload of the asix driver on the

Re: usbnet: driver_info-stop required to stop USB interrupts?

2014-03-21 Thread Oliver Neukum
On Thu, 2014-03-20 at 16:53 -0700, Julius Werner wrote: Can you please explain why we need to check if the waitqueue is active? and add a comment that answers the above question. Oh the braces!!! Well, that's just mean... Yes, I was unsure about this, but so it is. I expect

Re: usbnet: driver_info-stop required to stop USB interrupts?

2014-03-21 Thread Oliver Neukum
On Thu, 2014-03-20 at 14:19 -0700, Grant Grundler wrote: On Thu, Mar 20, 2014 at 9:35 AM, Grant Grundler grund...@google.com wrote: On Thu, Mar 20, 2014 at 12:15 AM, Oliver Neukum oneu...@suse.de wrote: ... I have an idea. Could you test this patch? ... - if (dev-wait) { .. +

Re: usbnet: driver_info-stop required to stop USB interrupts?

2014-03-21 Thread Grant Grundler
On Fri, Mar 21, 2014 at 1:33 AM, Oliver Neukum oneu...@suse.de wrote: ... Very well. Thorough testing is good. I'll wait for the result. Could you notify me of the final outcome? Ceratinly. Bad news is I had to restart my workstation that was driving the test due to completely different issue

Re: usbnet: driver_info-stop required to stop USB interrupts?

2014-03-20 Thread Oliver Neukum
On Wed, 2014-03-19 at 17:58 -0700, Grant Grundler wrote: Oliver, So even with this additional change to usbnet_probe, the device is reporting a link but can't transmit packets. I've tried with three different USB dongles (AX88178, AX88772B, SMSC75xx). In the last case, I ran ifconfig eth0

Re: usbnet: driver_info-stop required to stop USB interrupts?

2014-03-20 Thread Grant Grundler
On Thu, Mar 20, 2014 at 12:15 AM, Oliver Neukum oneu...@suse.de wrote: ... I have an idea. Could you test this patch? ... - if (dev-wait) { .. + if (waitqueue_active(dev-wait)) { Yes - building new image now (and transfer to USB and boot from USB). Should know in an hour or so

Re: usbnet: driver_info-stop required to stop USB interrupts?

2014-03-20 Thread Grant Grundler
On Thu, Mar 20, 2014 at 9:35 AM, Grant Grundler grund...@google.com wrote: On Thu, Mar 20, 2014 at 12:15 AM, Oliver Neukum oneu...@suse.de wrote: ... I have an idea. Could you test this patch? ... - if (dev-wait) { .. + if (waitqueue_active(dev-wait)) { Yes - building new

Re: usbnet: driver_info-stop required to stop USB interrupts?

2014-03-19 Thread Grant Grundler
On Tue, Mar 18, 2014 at 6:40 PM, Grant Grundler grund...@google.com wrote: On Tue, Mar 18, 2014 at 6:09 PM, Julius Werner jwer...@chromium.org wrote: I think a better place for this would be in usbnet_probe() (together with all the other dev-xxx initialization). Definitely better. @@

Re: usbnet: driver_info-stop required to stop USB interrupts?

2014-03-18 Thread Grant Grundler
On Mon, Mar 17, 2014 at 2:55 PM, Oliver Neukum oneu...@suse.de wrote: I am hping for the reporter of the original bug to test it. Oliver, on a haswell system running ChromeOS-3.8 kernel, this patch as-is resulted in a Bad Spinlock Magic error and subsequent pagefault. I believe the sequence was:

Re: usbnet: driver_info-stop required to stop USB interrupts?

2014-03-18 Thread Grant Grundler
On Tue, Mar 18, 2014 at 1:51 PM, Grant Grundler grund...@google.com wrote: On Mon, Mar 17, 2014 at 2:55 PM, Oliver Neukum oneu...@suse.de wrote: I am hping for the reporter of the original bug to test it. Oliver, on a haswell system running ChromeOS-3.8 kernel, this patch as-is resulted in a

Re: usbnet: driver_info-stop required to stop USB interrupts?

2014-03-18 Thread Julius Werner
I tried adding the following change on top of your patch but believe the plumbing still isn't quite correct since the USB device (eth0) is reporting a link but no TX or RX of traffic: @@ -805,6 +807,9 @@ int usbnet_open (struct net_device *net) goto done; } +

Re: usbnet: driver_info-stop required to stop USB interrupts?

2014-03-18 Thread Grant Grundler
On Tue, Mar 18, 2014 at 6:09 PM, Julius Werner jwer...@chromium.org wrote: I think a better place for this would be in usbnet_probe() (together with all the other dev-xxx initialization). Definitely better. @@ -1536,6 +1536,7 @@ usbnet_probe (struct usb_interface *udev, const struct usb

Re: usbnet: driver_info-stop required to stop USB interrupts?

2014-03-17 Thread Julius Werner
Hi Oliver, Any update on the state of this patch? Did it get picked up for merge somewhere? Do you agree with my suggestion of sticking closer to the original logic instead of adding that autopm_get(), or do you maybe want to add some more reviewers to confirm your code? I don't really care that

Re: usbnet: driver_info-stop required to stop USB interrupts?

2014-03-17 Thread Oliver Neukum
On Mon, 2014-03-17 at 14:53 -0700, Julius Werner wrote: Hi Oliver, Any update on the state of this patch? Did it get picked up for merge somewhere? Do you agree with my suggestion of sticking closer to the original logic instead of adding that autopm_get(), or do you maybe want to add some

Re: usbnet: driver_info-stop required to stop USB interrupts?

2014-03-17 Thread Grant Grundler
On Mon, Mar 17, 2014 at 2:55 PM, Oliver Neukum oneu...@suse.de wrote: On Mon, 2014-03-17 at 14:53 -0700, Julius Werner wrote: Hi Oliver, Any update on the state of this patch? Did it get picked up for merge somewhere? Do you agree with my suggestion of sticking closer to the original logic

Re: usbnet: driver_info-stop required to stop USB interrupts?

2014-03-11 Thread Oliver Neukum
On Mon, 2014-03-10 at 19:53 -0700, Julius Werner wrote: I think usbnet_stop() raced with the dev-bh tasklet, which by itself might not be a problem (usbnet_stop() later kills the tasklet itself, so it should expect that it can be running before that). The issue is that it calls

Re: usbnet: driver_info-stop required to stop USB interrupts?

2014-03-11 Thread Oliver Neukum
On Mon, 2014-03-10 at 19:53 -0700, Julius Werner wrote: I think the best solution would be to just make dev-wait a directly embedded structure inside struct usbnet instead of a pointer to something stack-allocated. usbnet_bh() could just call wake_up() unconditionally (if empty it will be a

Re: usbnet: driver_info-stop required to stop USB interrupts?

2014-03-11 Thread Julius Werner
I don't know enough about how runtime PM works in this driver to really review that patch, sorry. (Would this do a complete resume of the device if we call usbnet_stop() while it was suspended? Is that what we want?) I think you could have also preserved the original logic of using dev-wait as a

Re: usbnet: driver_info-stop required to stop USB interrupts?

2014-03-11 Thread Grant Grundler
On Mon, Mar 10, 2014 at 7:53 PM, Julius Werner jwer...@chromium.org wrote: I think usbnet_stop() raced with the dev-bh tasklet, which by itself might not be a problem (usbnet_stop() later kills the tasklet itself, so it should expect that it can be running before that). The issue is that it

Re: usbnet: driver_info-stop required to stop USB interrupts?

2014-03-11 Thread Grant Grundler
On Tue, Mar 11, 2014 at 10:31 AM, Grant Grundler grund...@google.com wrote: ... FWIW, I've reproduced this on Samsung Chromebook (Exynos 5250) with AX88772 USB dongle using the instructions I posted before (ie bouncing the USB port with reload_asix script). Sorry - with AX88178 dongle - not

Re: usbnet: driver_info-stop required to stop USB interrupts?

2014-03-11 Thread Grant Grundler
On Tue, Mar 11, 2014 at 10:31 AM, Grant Grundler grund...@google.com wrote: FWIW, I've reproduced this on Samsung Chromebook (Exynos 5250) with FYA - I just noticed the system crashed on the 2048th iteration :) ... RELOAD 2046 20140310202523 LINK 1000_full (3 sec) IP 192.168.1.116 (1 Sec)

usbnet: driver_info-stop required to stop USB interrupts?

2014-03-10 Thread Grant Grundler
I've trying to unravel a page fault panic I've run into a few times while testing load/unload of asix driver with ChromeOS 3.8.11 based kernel. I'm running into this crash on both ARM and X86. Panic output below is from Exynos 5422 system. Test script attached. My _guess_ is usbnet_stop() is

Re: usbnet: driver_info-stop required to stop USB interrupts?

2014-03-10 Thread Grant Grundler
On Mon, Mar 10, 2014 at 11:33 AM, Grant Grundler grund...@google.com wrote: I've trying to unravel a page fault panic I've run into a few times while testing load/unload of asix driver with ChromeOS 3.8.11 based kernel. I'm running into this crash on both ARM and X86. Correction - I can only

Re: usbnet: driver_info-stop required to stop USB interrupts?

2014-03-10 Thread Julius Werner
I think usbnet_stop() raced with the dev-bh tasklet, which by itself might not be a problem (usbnet_stop() later kills the tasklet itself, so it should expect that it can be running before that). The issue is that it calls usbnet_terminate_urbs() before that, which temporarily installs a waitqueue