Re: usb wireless detach
On Thu, Dec 16, 2010 at 12:28:56AM +, Jacob Meuser wrote:
> the following diff tries to make sure that no other processes/threads
> are or will be using the drivers software context when the driver is
> detached.
same treatment for otus(4), rsu(4) and urtwn(4)
ok?
--
jake...@sdf.lonestar.org
SDF Public Access UNIX System - http://sdf.lonestar.org
Index: if_otus.c
===
RCS file: /cvs/src/sys/dev/usb/if_otus.c,v
retrieving revision 1.25
diff -u -p -r1.25 if_otus.c
--- if_otus.c 27 Dec 2010 03:03:50 - 1.25
+++ if_otus.c 30 Dec 2010 05:57:42 -
@@ -246,17 +246,32 @@ otus_detach(struct device *self, int fla
struct ifnet *ifp = &sc->sc_ic.ic_if;
int s;
- s = splnet();
-
- /* Wait for all queued asynchronous commands to complete. */
- while (sc->cmdq.queued > 0)
- tsleep(&sc->cmdq, 0, "cmdq", 0);
+ s = splusb();
if (timeout_initialized(&sc->scan_to))
timeout_del(&sc->scan_to);
if (timeout_initialized(&sc->calib_to))
timeout_del(&sc->calib_to);
+ /* Wait for all queued asynchronous commands to complete. */
+#if 0
+ while (sc->cmdq.queued > 0)
+ tsleep(&sc->cmdq, 0, "cmdq", 0);
+#endif
+ /* the async commands are run in a task */
+ usb_rem_wait_task(sc->sc_udev, &sc->sc_task);
+
+ /* but the task might not have run if it did not start before
+* usbd_deactivate() was called, so wakeup now. we're
+* detaching, no need to try to run more commands.
+*/
+ if (sc->cmdq.queued > 0) {
+ sc->cmdq.queued = 0;
+ wakeup(&sc->cmdq);
+ }
+
+ usbd_ref_wait(sc->sc_udev);
+
if (ifp->if_softc != NULL) {
ifp->if_flags &= ~(IFF_RUNNING | IFF_OACTIVE);
ieee80211_ifdetach(ifp);
@@ -732,8 +747,15 @@ otus_next_scan(void *arg)
{
struct otus_softc *sc = arg;
+ if (usbd_is_dying(sc->sc_udev))
+ return;
+
+ usbd_ref_incr(sc->sc_udev);
+
if (sc->sc_ic.ic_state == IEEE80211_S_SCAN)
ieee80211_next_scan(&sc->sc_ic.ic_if);
+
+ usbd_ref_decr(sc->sc_udev);
}
void
@@ -809,7 +831,8 @@ otus_newstate_cb(struct otus_softc *sc,
case IEEE80211_S_SCAN:
(void)otus_set_chan(sc, ic->ic_bss->ni_chan, 0);
- timeout_add_msec(&sc->scan_to, 200);
+ if (!usbd_is_dying(sc->sc_udev))
+ timeout_add_msec(&sc->scan_to, 200);
break;
case IEEE80211_S_AUTH:
@@ -830,7 +853,8 @@ otus_newstate_cb(struct otus_softc *sc,
otus_newassoc(ic, ni, 1);
/* Start calibration timer. */
- timeout_add_sec(&sc->calib_to, 1);
+ if (!usbd_is_dying(sc->sc_udev))
+ timeout_add_sec(&sc->calib_to, 1);
}
break;
}
@@ -1499,6 +1523,11 @@ otus_ioctl(struct ifnet *ifp, u_long cmd
struct ifreq *ifr;
int s, error = 0;
+ if (usbd_is_dying(sc->sc_udev))
+ return ENXIO;
+
+ usbd_ref_incr(sc->sc_udev);
+
s = splnet();
switch (cmd) {
@@ -1555,6 +1584,9 @@ otus_ioctl(struct ifnet *ifp, u_long cmd
}
splx(s);
+
+ usbd_ref_decr(sc->sc_udev);
+
return error;
}
@@ -2176,12 +2208,20 @@ otus_calibrate_to(void *arg)
struct ieee80211_node *ni;
int s;
+ if (usbd_is_dying(sc->sc_udev))
+ return;
+
+ usbd_ref_incr(sc->sc_udev);
+
s = splnet();
ni = ic->ic_bss;
ieee80211_amrr_choose(&sc->amrr, ni, &((struct otus_node *)ni)->amn);
splx(s);
- timeout_add_sec(&sc->calib_to, 1);
+ if (!usbd_is_dying(sc->sc_udev))
+ timeout_add_sec(&sc->calib_to, 1);
+
+ usbd_ref_decr(sc->sc_udev);
}
int
Index: if_rsu.c
===
RCS file: /cvs/src/sys/dev/usb/if_rsu.c,v
retrieving revision 1.8
diff -u -p -r1.8 if_rsu.c
--- if_rsu.c27 Dec 2010 03:03:50 - 1.8
+++ if_rsu.c30 Dec 2010 05:57:42 -
@@ -343,12 +343,29 @@ rsu_detach(struct device *self, int flag
struct ifnet *ifp = &sc->sc_ic.ic_if;
int s;
- s = splnet();
- /* Wait for all async commands to complete. */
- rsu_wait_async(sc);
+ s = splusb();
if (timeout_initialized(&sc->calib_to))
timeout_del(&sc->calib_to);
+
+ /* Wait for all async commands to complete. */
+#if 0
+ rsu_wait_async(sc);
+#endif
+ /* the async commands are run in a task */
+ usb_rem_wait_task(sc->sc_udev, &sc->sc_task);
+
+ /* but the task might not have run if it did not start before
+* usbd_deactivate() was called, so wakeup now. we're
+* deta
Re: usb wireless detach
I will test this patch with my Linksys rum(4) AP tonight and report back
to you.
-luis
On 12/22/10 18:36, Jacob Meuser wrote:
no feedback yet. anyone care to comment on this?
On Thu, Dec 16, 2010 at 12:28:56AM +, Jacob Meuser wrote:
the following diff tries to make sure that no other processes/threads
are or will be using the drivers software context when the driver is
detached.
this diff covers rum(4), run(4), ural(4) and urtw(4). without the
diff, I can get the kernel to crash by starting a scan with
the deice, then ejecting it while the scan is running. with this
diff, I cannot get a crash.
normally, usb device detach happens in the usb_task thread. the sole
exception is when the usb bus itself is being detached. this happens
with cardbus devices, and in that case, the usb device detach happens
in the generic workq thread.
this adds a simple reference counting/waiting mechanism. this is
definitely needed for ioctl(2), which can sleep then start using the
driver again. it may not be needed for timeout(9)s now, but maybe
someday it will. another possible process using the device is the
usb_task thread. in the normal case, this is the same process that
is detaching, so there is no concurrency issue. there is already
usb_rem_wait_task() to deal with detaches from other processes,
because the bus driver needs it.
this also adds more uses of usbd_is_dying() to check if the device
has been detached:
* before adding timeouts
* when entering timeouts, usb_tasks, and ioctl
also of note is the order things are done in the detach routines:
1) remove pending timeouts
2) remove (and possibly wait for) pending usb_tasks
3) wait for other processes
4) detach from the network stack
5) cleanup/free usb resources
thoughts? ok?
--
jake...@sdf.lonestar.org
SDF Public Access UNIX System - http://sdf.lonestar.org
Index: if_ral.c
===
RCS file: /cvs/src/sys/dev/usb/if_ral.c,v
retrieving revision 1.117
diff -u -p -r1.117 if_ral.c
--- if_ral.c6 Dec 2010 04:41:39 - 1.117
+++ if_ral.c15 Dec 2010 02:58:35 -
@@ -363,17 +363,20 @@ ural_detach(struct device *self, int fla
s = splusb();
- if (ifp->if_softc != NULL) {
- ieee80211_ifdetach(ifp);/* free all nodes */
- if_detach(ifp);
- }
-
- usb_rem_task(sc->sc_udev,&sc->sc_task);
if (timeout_initialized(&sc->scan_to))
timeout_del(&sc->scan_to);
if (timeout_initialized(&sc->amrr_to))
timeout_del(&sc->amrr_to);
+ usb_rem_wait_task(sc->sc_udev,&sc->sc_task);
+
+ usbd_ref_wait(sc->sc_udev);
+
+ if (ifp->if_softc != NULL) {
+ ieee80211_ifdetach(ifp);/* free all nodes */
+ if_detach(ifp);
+ }
+
if (sc->amrr_xfer != NULL) {
usbd_free_xfer(sc->amrr_xfer);
sc->amrr_xfer = NULL;
@@ -547,8 +550,15 @@ ural_next_scan(void *arg)
struct ieee80211com *ic =&sc->sc_ic;
struct ifnet *ifp =&ic->ic_if;
+ if (usbd_is_dying(sc->sc_udev))
+ return;
+
+ usbd_ref_incr(sc->sc_udev);
+
if (ic->ic_state == IEEE80211_S_SCAN)
ieee80211_next_scan(ifp);
+
+ usbd_ref_decr(sc->sc_udev);
}
void
@@ -559,6 +569,9 @@ ural_task(void *arg)
enum ieee80211_state ostate;
struct ieee80211_node *ni;
+ if (usbd_is_dying(sc->sc_udev))
+ return;
+
ostate = ic->ic_state;
switch (sc->sc_state) {
@@ -574,7 +587,8 @@ ural_task(void *arg)
case IEEE80211_S_SCAN:
ural_set_chan(sc, ic->ic_bss->ni_chan);
- timeout_add_msec(&sc->scan_to, 200);
+ if (!usbd_is_dying(sc->sc_udev))
+ timeout_add_msec(&sc->scan_to, 200);
break;
case IEEE80211_S_AUTH:
@@ -1324,6 +1338,11 @@ ural_ioctl(struct ifnet *ifp, u_long cmd
struct ifreq *ifr;
int s, error = 0;
+ if (usbd_is_dying(sc->sc_udev))
+ return ENXIO;
+
+ usbd_ref_incr(sc->sc_udev);
+
s = splnet();
switch (cmd) {
@@ -1387,6 +1406,8 @@ ural_ioctl(struct ifnet *ifp, u_long cmd
splx(s);
+ usbd_ref_decr(sc->sc_udev);
+
return error;
}
@@ -2147,7 +2168,8 @@ ural_amrr_start(struct ural_softc *sc, s
i--);
ni->ni_txrate = i;
- timeout_add_sec(&sc->amrr_to, 1);
+ if (!usbd_is_dying(sc->sc_udev))
+ timeout_add_sec(&sc->amrr_to, 1);
}
void
@@ -2157,6 +2179,11 @@ ural_amrr_timeout(void *arg)
usb_device_request_t req;
int s;
+ if (usbd_is_dying(sc->sc_udev))
+ return;
+
+ usbd_ref_incr(sc->sc_udev);
+
s = splusb();
/*
@@ -2174,6 +2201,8 @@ ural_amrr_timeout(void *arg)
(void)usbd_transfer(sc->amrr_xfer);
splx(s);
+
+ usbd_ref_decr(sc->sc_udev);
}
Re: usb wireless detach
On Thu, Dec 23, 2010 at 01:30:15PM +0100, Thomas Pfaff wrote: > On Thu, 23 Dec 2010 00:36:26 + > Jacob Meuser wrote: > > > no feedback yet. anyone care to comment on this? > > > > > this diff covers rum(4), run(4), ural(4) and urtw(4). without the > > > diff, I can get the kernel to crash by starting a scan with > > > the deice, then ejecting it while the scan is running. with this > > > diff, I cannot get a crash. > > I cannot reproduce this problem with my rum(4) device. I've not tried > your diff to look for any regressions, but I'll try to do that later. hmm. indeed, it is difficult to make rum(4) crash in this situation with -current. but not so hard if the rum(4) is attached to a cardbus usb adapter, and that is ejected (the patch addresses that situation too). I did just get crashes on the first try with run(4), ural(4) and utrw(4) though. this patch does the same thing in all these drivers. it shouldn't be hard to use the concepts in other drivers. that was the plan ... do a group of drivers to find the pattern. -- jake...@sdf.lonestar.org SDF Public Access UNIX System - http://sdf.lonestar.org
Re: usb wireless detach
On Thu, 23 Dec 2010 00:36:26 + Jacob Meuser wrote: > no feedback yet. anyone care to comment on this? > > > this diff covers rum(4), run(4), ural(4) and urtw(4). without the > > diff, I can get the kernel to crash by starting a scan with > > the deice, then ejecting it while the scan is running. with this > > diff, I cannot get a crash. I cannot reproduce this problem with my rum(4) device. I've not tried your diff to look for any regressions, but I'll try to do that later. If I issue "ifconfig rum0 scan" and then unplug the device, it simply waits for a while (say for about 30 seconds) and then prints "ifconfig: SIOCGIFFLAGS: Device not configured". The waiting can be cancelled with a ^C (and then no message will be displayed). Attaching and detaching during scans: $ dmesg | tail -7 root on wd0a swap on wd0b dump on wd0b ehci_idone: ex=0x8019c400 is done! rum0 detached rum0 at uhub0 port 1 "Ralink 802.11 bg WLAN" rev 2.00/0.01 addr 2 rum0: MAC/BBP RT2573 (rev 0x2573a), RF RT2528, address 00:1c:f0:a5:bf:57 ehci_idone: ex=0x80658000 is done! rum0 detached Various information: rum0: flags=8802 mtu 1500 lladdr 00:1c:f0:a5:bf:57 priority: 4 groups: wlan media: IEEE802.11 autoselect status: no network ieee80211: nwid "" 100dBm Controller /dev/usb0: addr 1: high speed, self powered, config 1, EHCI root hub(0x), Intel(0x8086), rev 1.00 port 1 powered port 2 addr 2: high speed, power 300 mA, config 1, 802.11 bg WLAN(0x3c03), Ralink(0x07d1), rev 0.01 port 3 powered port 4 powered (source tree from today) OpenBSD 4.8-current (GENERIC.MP) #12: Thu Dec 23 13:11:03 CET 2010 tpf...@ws.tp76.info:/usr/src/sys/arch/amd64/compile/GENERIC.MP real mem = 3152609280 (3006MB) avail mem = 3054718976 (2913MB) mainbus0 at root bios0 at mainbus0: SMBIOS rev. 2.4 @ 0xf06b0 (76 entries) bios0: vendor American Megatrends Inc. version "1704" date 11/27/2007 bios0: ASUSTeK Computer INC. P5B-E acpi0 at bios0: rev 2 acpi0: sleep states S0 S1 S3 S4 S5 acpi0: tables DSDT FACP APIC MCFG OEMB HPET acpi0: wakeup devices P0P2(S4) P0P1(S4) UAR1(S4) PS2K(S4) PS2M(S4) EUSB(S4) USBE(S4) P0P4(S4) P0P5(S4) P0P6(S4) P0P7(S4) P0P8(S4) P0P9(S4) USB0(S4) USB1(S4) USB2(S4) USB3(S4) USB4(S4) USB5(S4) acpitimer0 at acpi0: 3579545 Hz, 24 bits acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: Intel(R) Core(TM)2 CPU 6400 @ 2.13GHz, 2135.36 MHz cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,SBF,SSE3,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,NXE,LONG cpu0: 2MB 64b/line 8-way L2 cache cpu0: apic clock running at 266MHz cpu1 at mainbus0: apid 1 (application processor) cpu1: Intel(R) Core(TM)2 CPU 6400 @ 2.13GHz, 2135.04 MHz cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,SBF,SSE3,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,NXE,LONG cpu1: 2MB 64b/line 8-way L2 cache ioapic0 at mainbus0: apid 2 pa 0xfec0, version 20, 24 pins acpihpet0 at acpi0: 14318179 Hz acpiprt0 at acpi0: bus 0 (PCI0) acpiprt1 at acpi0: bus 1 (P0P2) acpiprt2 at acpi0: bus 5 (P0P1) acpiprt3 at acpi0: bus 4 (P0P4) acpiprt4 at acpi0: bus -1 (P0P5) acpiprt5 at acpi0: bus -1 (P0P6) acpiprt6 at acpi0: bus 3 (P0P7) acpiprt7 at acpi0: bus 2 (P0P8) acpicpu0 at acpi0: PSS acpicpu1 at acpi0: PSS aibs0 at acpi0: RTMP RVLT RFAN GGRP GITM SITM acpibtn0 at acpi0: PWRB pci0 at mainbus0 bus 0 pchb0 at pci0 dev 0 function 0 "Intel 82G965 Host" rev 0x02 ppb0 at pci0 dev 1 function 0 "Intel 82G965 PCIE" rev 0x02: apic 2 int 16 (irq 11) pci1 at ppb0 bus 1 vga1 at pci1 dev 0 function 0 "NVIDIA GeForce 7600 GT" rev 0xa1 wsdisplay0 at vga1 mux 1: console (80x25, vt100 emulation) wsdisplay0: screen 1-5 added (80x25, vt100 emulation) uhci0 at pci0 dev 26 function 0 "Intel 82801H USB" rev 0x02: apic 2 int 16 (irq 11) uhci1 at pci0 dev 26 function 1 "Intel 82801H USB" rev 0x02: apic 2 int 17 (irq 5) ehci0 at pci0 dev 26 function 7 "Intel 82801H USB" rev 0x02: apic 2 int 18 (irq 15) usb0 at ehci0: USB revision 2.0 uhub0 at usb0 "Intel EHCI root hub" rev 2.00/1.00 addr 1 azalia0 at pci0 dev 27 function 0 "Intel 82801H HD Audio" rev 0x02: apic 2 int 22 (irq 3) azalia0: codecs: Analog Devices AD1988A audio0 at azalia0 ppb1 at pci0 dev 28 function 0 "Intel 82801H PCIE" rev 0x02: apic 2 int 16 (irq 11) pci2 at ppb1 bus 4 ppb2 at pci0 dev 28 function 3 "Intel 82801H PCIE" rev 0x02: apic 2 int 19 (irq 10) pci3 at ppb2 bus 3 age0 at pci3 dev 0 function 0 "Attansic Technology L1" rev 0xb0: apic 2 int 19 (irq 10), address 00:18:f3:9d:7d:04 atphy0 at age0 phy 0: F1 10/100/1000 PHY, rev. 5 ppb3 at pci0 dev 28 function 4 "Intel 82801H PCIE" rev 0x02: apic 2 int 16 (irq 11) pci4 at ppb3 bus 2 jmb0 at pci4 dev 0 function 0 "JMicron JMB363 IDE/SATA" rev 0x02 ahci0 at jmb0: apic 2 int 16 (irq 11), AHCI 1.0 scsibus0 at ahci0: 32 targ
Re: usb wireless detach
I have been looking at doing somrhing similar because I have a usb
wlan that for some reason unplugs it self (the device seems to die, I
think it's broken or something).
I will try this diff when I get a chance.
BR
Dunceor
On Thu, Dec 23, 2010 at 1:36 AM, Jacob Meuser
wrote:
> no feedback yet. B anyone care to comment on this?
>
> On Thu, Dec 16, 2010 at 12:28:56AM +, Jacob Meuser wrote:
>> the following diff tries to make sure that no other processes/threads
>> are or will be using the drivers software context when the driver is
>> detached.
>>
>> this diff covers rum(4), run(4), ural(4) and urtw(4). B without the
>> diff, I can get the kernel to crash by starting a scan with
>> the deice, then ejecting it while the scan is running. B with this
>> diff, I cannot get a crash.
>>
>> normally, usb device detach happens in the usb_task thread. B the sole
>> exception is when the usb bus itself is being detached. B this happens
>> with cardbus devices, and in that case, the usb device detach happens
>> in the generic workq thread.
>>
>> this adds a simple reference counting/waiting mechanism. B this is
>> definitely needed for ioctl(2), which can sleep then start using the
>> driver again. B it may not be needed for timeout(9)s now, but maybe
>> someday it will. B another possible process using the device is the
>> usb_task thread. B in the normal case, this is the same process that
>> is detaching, so there is no concurrency issue. B there is already
>> usb_rem_wait_task() to deal with detaches from other processes,
>> because the bus driver needs it.
>>
>> this also adds more uses of usbd_is_dying() to check if the device
>> has been detached:
>> * before adding timeouts
>> * when entering timeouts, usb_tasks, and ioctl
>>
>> also of note is the order things are done in the detach routines:
>> 1) remove pending timeouts
>> 2) remove (and possibly wait for) pending usb_tasks
>> 3) wait for other processes
>> 4) detach from the network stack
>> 5) cleanup/free usb resources
>>
>> thoughts? B ok?
>>
>> --
>> jake...@sdf.lonestar.org
>> SDF Public Access UNIX System - http://sdf.lonestar.org
>>
>>
>> Index: if_ral.c
>> ===
>> RCS file: /cvs/src/sys/dev/usb/if_ral.c,v
>> retrieving revision 1.117
>> diff -u -p -r1.117 if_ral.c
>> --- if_ral.c B 6 Dec 2010 04:41:39 - B B B 1.117
>> +++ if_ral.c B 15 Dec 2010 02:58:35 -
>> @@ -363,17 +363,20 @@ ural_detach(struct device *self, int fla
>>
>> B B B s = splusb();
>>
>> - B B if (ifp->if_softc != NULL) {
>> - B B B B B B ieee80211_ifdetach(ifp); B B B B /* free all nodes
*/
>> - B B B B B B if_detach(ifp);
>> - B B }
>> -
>> - B B usb_rem_task(sc->sc_udev, &sc->sc_task);
>> B B B if (timeout_initialized(&sc->scan_to))
>> B B B B B B B timeout_del(&sc->scan_to);
>> B B B if (timeout_initialized(&sc->amrr_to))
>> B B B B B B B timeout_del(&sc->amrr_to);
>>
>> + B B usb_rem_wait_task(sc->sc_udev, &sc->sc_task);
>> +
>> + B B usbd_ref_wait(sc->sc_udev);
>> +
>> + B B if (ifp->if_softc != NULL) {
>> + B B B B B B ieee80211_ifdetach(ifp); B B B B /* free all nodes
*/
>> + B B B B B B if_detach(ifp);
>> + B B }
>> +
>> B B B if (sc->amrr_xfer != NULL) {
>> B B B B B B B usbd_free_xfer(sc->amrr_xfer);
>> B B B B B B B sc->amrr_xfer = NULL;
>> @@ -547,8 +550,15 @@ ural_next_scan(void *arg)
>> B B B struct ieee80211com *ic = &sc->sc_ic;
>> B B B struct ifnet *ifp = &ic->ic_if;
>>
>> + B B if (usbd_is_dying(sc->sc_udev))
>> + B B B B B B return;
>> +
>> + B B usbd_ref_incr(sc->sc_udev);
>> +
>> B B B if (ic->ic_state == IEEE80211_S_SCAN)
>> B B B B B B B ieee80211_next_scan(ifp);
>> +
>> + B B usbd_ref_decr(sc->sc_udev);
>> B }
>>
>> B void
>> @@ -559,6 +569,9 @@ ural_task(void *arg)
>> B B B enum ieee80211_state ostate;
>> B B B struct ieee80211_node *ni;
>>
>> + B B if (usbd_is_dying(sc->sc_udev))
>> + B B B B B B return;
>> +
>> B B B ostate = ic->ic_state;
>>
>> B B B switch (sc->sc_state) {
>> @@ -574,7 +587,8 @@ ural_task(void *arg)
>>
>> B B B case IEEE80211_S_SCAN:
>> B B B B B B B ural_set_chan(sc, ic->ic_bss->ni_chan);
>> - B B B B B B timeout_add_msec(&sc->scan_to, 200);
>> + B B B B B B if (!usbd_is_dying(sc->sc_udev))
>> + B B B B B B B B B B timeout_add_msec(&sc->scan_to, 200);
>> B B B B B B B break;
>>
>> B B B case IEEE80211_S_AUTH:
>> @@ -1324,6 +1338,11 @@ ural_ioctl(struct ifnet *ifp, u_long cmd
>> B B B struct ifreq *ifr;
>> B B B int s, error = 0;
>>
>> + B B if (usbd_is_dying(sc->sc_udev))
>> + B B B B B B return ENXIO;
>> +
>> + B B usbd_ref_incr(sc->sc_udev);
>> +
>> B B B s = splnet();
>>
>> B B B switch (cmd) {
>> @@ -1387,6 +1406,8 @@ ural_ioctl(struct ifnet *ifp, u_long cmd
>>
>> B B B splx(s);
>>
>> + B B usbd_ref_decr(sc->sc_udev);
>> +
>> B B B return error;
>> B }
>>
Re: usb wireless detach
On Thu, Dec 23, 2010 at 12:36:26AM +, Jacob Meuser wrote:
> no feedback yet. anyone care to comment on this?
Sounds good to me. I can get odd things to happen when I unplug
my ural AP, I just haven't had a chance to test this to see if
it helps. I will try. It's Christmas after all. :-).
Ken
>
> On Thu, Dec 16, 2010 at 12:28:56AM +, Jacob Meuser wrote:
> > the following diff tries to make sure that no other processes/threads
> > are or will be using the drivers software context when the driver is
> > detached.
> >
> > this diff covers rum(4), run(4), ural(4) and urtw(4). without the
> > diff, I can get the kernel to crash by starting a scan with
> > the deice, then ejecting it while the scan is running. with this
> > diff, I cannot get a crash.
> >
> > normally, usb device detach happens in the usb_task thread. the sole
> > exception is when the usb bus itself is being detached. this happens
> > with cardbus devices, and in that case, the usb device detach happens
> > in the generic workq thread.
> >
> > this adds a simple reference counting/waiting mechanism. this is
> > definitely needed for ioctl(2), which can sleep then start using the
> > driver again. it may not be needed for timeout(9)s now, but maybe
> > someday it will. another possible process using the device is the
> > usb_task thread. in the normal case, this is the same process that
> > is detaching, so there is no concurrency issue. there is already
> > usb_rem_wait_task() to deal with detaches from other processes,
> > because the bus driver needs it.
> >
> > this also adds more uses of usbd_is_dying() to check if the device
> > has been detached:
> > * before adding timeouts
> > * when entering timeouts, usb_tasks, and ioctl
> >
> > also of note is the order things are done in the detach routines:
> > 1) remove pending timeouts
> > 2) remove (and possibly wait for) pending usb_tasks
> > 3) wait for other processes
> > 4) detach from the network stack
> > 5) cleanup/free usb resources
> >
> > thoughts? ok?
> >
> > --
> > jake...@sdf.lonestar.org
> > SDF Public Access UNIX System - http://sdf.lonestar.org
> >
> >
> > Index: if_ral.c
> > ===
> > RCS file: /cvs/src/sys/dev/usb/if_ral.c,v
> > retrieving revision 1.117
> > diff -u -p -r1.117 if_ral.c
> > --- if_ral.c6 Dec 2010 04:41:39 - 1.117
> > +++ if_ral.c15 Dec 2010 02:58:35 -
> > @@ -363,17 +363,20 @@ ural_detach(struct device *self, int fla
> >
> > s = splusb();
> >
> > - if (ifp->if_softc != NULL) {
> > - ieee80211_ifdetach(ifp);/* free all nodes */
> > - if_detach(ifp);
> > - }
> > -
> > - usb_rem_task(sc->sc_udev, &sc->sc_task);
> > if (timeout_initialized(&sc->scan_to))
> > timeout_del(&sc->scan_to);
> > if (timeout_initialized(&sc->amrr_to))
> > timeout_del(&sc->amrr_to);
> >
> > + usb_rem_wait_task(sc->sc_udev, &sc->sc_task);
> > +
> > + usbd_ref_wait(sc->sc_udev);
> > +
> > + if (ifp->if_softc != NULL) {
> > + ieee80211_ifdetach(ifp);/* free all nodes */
> > + if_detach(ifp);
> > + }
> > +
> > if (sc->amrr_xfer != NULL) {
> > usbd_free_xfer(sc->amrr_xfer);
> > sc->amrr_xfer = NULL;
> > @@ -547,8 +550,15 @@ ural_next_scan(void *arg)
> > struct ieee80211com *ic = &sc->sc_ic;
> > struct ifnet *ifp = &ic->ic_if;
> >
> > + if (usbd_is_dying(sc->sc_udev))
> > + return;
> > +
> > + usbd_ref_incr(sc->sc_udev);
> > +
> > if (ic->ic_state == IEEE80211_S_SCAN)
> > ieee80211_next_scan(ifp);
> > +
> > + usbd_ref_decr(sc->sc_udev);
> > }
> >
> > void
> > @@ -559,6 +569,9 @@ ural_task(void *arg)
> > enum ieee80211_state ostate;
> > struct ieee80211_node *ni;
> >
> > + if (usbd_is_dying(sc->sc_udev))
> > + return;
> > +
> > ostate = ic->ic_state;
> >
> > switch (sc->sc_state) {
> > @@ -574,7 +587,8 @@ ural_task(void *arg)
> >
> > case IEEE80211_S_SCAN:
> > ural_set_chan(sc, ic->ic_bss->ni_chan);
> > - timeout_add_msec(&sc->scan_to, 200);
> > + if (!usbd_is_dying(sc->sc_udev))
> > + timeout_add_msec(&sc->scan_to, 200);
> > break;
> >
> > case IEEE80211_S_AUTH:
> > @@ -1324,6 +1338,11 @@ ural_ioctl(struct ifnet *ifp, u_long cmd
> > struct ifreq *ifr;
> > int s, error = 0;
> >
> > + if (usbd_is_dying(sc->sc_udev))
> > + return ENXIO;
> > +
> > + usbd_ref_incr(sc->sc_udev);
> > +
> > s = splnet();
> >
> > switch (cmd) {
> > @@ -1387,6 +1406,8 @@ ural_ioctl(struct ifnet *ifp, u_long cmd
> >
> > splx(s);
> >
> > + usbd_ref_decr(sc->sc_udev);
> > +
> > return error;
> > }
> >
> > @@ -2147,7 +2168,8 @@ ural_amrr_start(struct ural_softc *sc, s
> > i--);
> > ni->ni_txrate = i;
> >
> > - timeout_add_sec(&sc->amrr_to, 1);
> >
Re: usb wireless detach
no feedback yet. anyone care to comment on this?
On Thu, Dec 16, 2010 at 12:28:56AM +, Jacob Meuser wrote:
> the following diff tries to make sure that no other processes/threads
> are or will be using the drivers software context when the driver is
> detached.
>
> this diff covers rum(4), run(4), ural(4) and urtw(4). without the
> diff, I can get the kernel to crash by starting a scan with
> the deice, then ejecting it while the scan is running. with this
> diff, I cannot get a crash.
>
> normally, usb device detach happens in the usb_task thread. the sole
> exception is when the usb bus itself is being detached. this happens
> with cardbus devices, and in that case, the usb device detach happens
> in the generic workq thread.
>
> this adds a simple reference counting/waiting mechanism. this is
> definitely needed for ioctl(2), which can sleep then start using the
> driver again. it may not be needed for timeout(9)s now, but maybe
> someday it will. another possible process using the device is the
> usb_task thread. in the normal case, this is the same process that
> is detaching, so there is no concurrency issue. there is already
> usb_rem_wait_task() to deal with detaches from other processes,
> because the bus driver needs it.
>
> this also adds more uses of usbd_is_dying() to check if the device
> has been detached:
> * before adding timeouts
> * when entering timeouts, usb_tasks, and ioctl
>
> also of note is the order things are done in the detach routines:
> 1) remove pending timeouts
> 2) remove (and possibly wait for) pending usb_tasks
> 3) wait for other processes
> 4) detach from the network stack
> 5) cleanup/free usb resources
>
> thoughts? ok?
>
> --
> jake...@sdf.lonestar.org
> SDF Public Access UNIX System - http://sdf.lonestar.org
>
>
> Index: if_ral.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_ral.c,v
> retrieving revision 1.117
> diff -u -p -r1.117 if_ral.c
> --- if_ral.c 6 Dec 2010 04:41:39 - 1.117
> +++ if_ral.c 15 Dec 2010 02:58:35 -
> @@ -363,17 +363,20 @@ ural_detach(struct device *self, int fla
>
> s = splusb();
>
> - if (ifp->if_softc != NULL) {
> - ieee80211_ifdetach(ifp);/* free all nodes */
> - if_detach(ifp);
> - }
> -
> - usb_rem_task(sc->sc_udev, &sc->sc_task);
> if (timeout_initialized(&sc->scan_to))
> timeout_del(&sc->scan_to);
> if (timeout_initialized(&sc->amrr_to))
> timeout_del(&sc->amrr_to);
>
> + usb_rem_wait_task(sc->sc_udev, &sc->sc_task);
> +
> + usbd_ref_wait(sc->sc_udev);
> +
> + if (ifp->if_softc != NULL) {
> + ieee80211_ifdetach(ifp);/* free all nodes */
> + if_detach(ifp);
> + }
> +
> if (sc->amrr_xfer != NULL) {
> usbd_free_xfer(sc->amrr_xfer);
> sc->amrr_xfer = NULL;
> @@ -547,8 +550,15 @@ ural_next_scan(void *arg)
> struct ieee80211com *ic = &sc->sc_ic;
> struct ifnet *ifp = &ic->ic_if;
>
> + if (usbd_is_dying(sc->sc_udev))
> + return;
> +
> + usbd_ref_incr(sc->sc_udev);
> +
> if (ic->ic_state == IEEE80211_S_SCAN)
> ieee80211_next_scan(ifp);
> +
> + usbd_ref_decr(sc->sc_udev);
> }
>
> void
> @@ -559,6 +569,9 @@ ural_task(void *arg)
> enum ieee80211_state ostate;
> struct ieee80211_node *ni;
>
> + if (usbd_is_dying(sc->sc_udev))
> + return;
> +
> ostate = ic->ic_state;
>
> switch (sc->sc_state) {
> @@ -574,7 +587,8 @@ ural_task(void *arg)
>
> case IEEE80211_S_SCAN:
> ural_set_chan(sc, ic->ic_bss->ni_chan);
> - timeout_add_msec(&sc->scan_to, 200);
> + if (!usbd_is_dying(sc->sc_udev))
> + timeout_add_msec(&sc->scan_to, 200);
> break;
>
> case IEEE80211_S_AUTH:
> @@ -1324,6 +1338,11 @@ ural_ioctl(struct ifnet *ifp, u_long cmd
> struct ifreq *ifr;
> int s, error = 0;
>
> + if (usbd_is_dying(sc->sc_udev))
> + return ENXIO;
> +
> + usbd_ref_incr(sc->sc_udev);
> +
> s = splnet();
>
> switch (cmd) {
> @@ -1387,6 +1406,8 @@ ural_ioctl(struct ifnet *ifp, u_long cmd
>
> splx(s);
>
> + usbd_ref_decr(sc->sc_udev);
> +
> return error;
> }
>
> @@ -2147,7 +2168,8 @@ ural_amrr_start(struct ural_softc *sc, s
>i--);
> ni->ni_txrate = i;
>
> - timeout_add_sec(&sc->amrr_to, 1);
> + if (!usbd_is_dying(sc->sc_udev))
> + timeout_add_sec(&sc->amrr_to, 1);
> }
>
> void
> @@ -2157,6 +2179,11 @@ ural_amrr_timeout(void *arg)
> usb_device_request_t req;
> int s;
>
> + if (usbd_is_dying(sc->sc_udev))
> + return;
> +
> + usbd_ref_incr(sc->sc_udev);
> +
> s = splusb();
>
> /*
> @@ -2174,6 +2201,8 @@ ural_amrr_timeout(void *arg)
> (void)usbd_transfer(sc->a
usb wireless detach
the following diff tries to make sure that no other processes/threads
are or will be using the drivers software context when the driver is
detached.
this diff covers rum(4), run(4), ural(4) and urtw(4). without the
diff, I can get the kernel to crash by starting a scan with
the deice, then ejecting it while the scan is running. with this
diff, I cannot get a crash.
normally, usb device detach happens in the usb_task thread. the sole
exception is when the usb bus itself is being detached. this happens
with cardbus devices, and in that case, the usb device detach happens
in the generic workq thread.
this adds a simple reference counting/waiting mechanism. this is
definitely needed for ioctl(2), which can sleep then start using the
driver again. it may not be needed for timeout(9)s now, but maybe
someday it will. another possible process using the device is the
usb_task thread. in the normal case, this is the same process that
is detaching, so there is no concurrency issue. there is already
usb_rem_wait_task() to deal with detaches from other processes,
because the bus driver needs it.
this also adds more uses of usbd_is_dying() to check if the device
has been detached:
* before adding timeouts
* when entering timeouts, usb_tasks, and ioctl
also of note is the order things are done in the detach routines:
1) remove pending timeouts
2) remove (and possibly wait for) pending usb_tasks
3) wait for other processes
4) detach from the network stack
5) cleanup/free usb resources
thoughts? ok?
--
jake...@sdf.lonestar.org
SDF Public Access UNIX System - http://sdf.lonestar.org
Index: if_ral.c
===
RCS file: /cvs/src/sys/dev/usb/if_ral.c,v
retrieving revision 1.117
diff -u -p -r1.117 if_ral.c
--- if_ral.c6 Dec 2010 04:41:39 - 1.117
+++ if_ral.c15 Dec 2010 02:58:35 -
@@ -363,17 +363,20 @@ ural_detach(struct device *self, int fla
s = splusb();
- if (ifp->if_softc != NULL) {
- ieee80211_ifdetach(ifp);/* free all nodes */
- if_detach(ifp);
- }
-
- usb_rem_task(sc->sc_udev, &sc->sc_task);
if (timeout_initialized(&sc->scan_to))
timeout_del(&sc->scan_to);
if (timeout_initialized(&sc->amrr_to))
timeout_del(&sc->amrr_to);
+ usb_rem_wait_task(sc->sc_udev, &sc->sc_task);
+
+ usbd_ref_wait(sc->sc_udev);
+
+ if (ifp->if_softc != NULL) {
+ ieee80211_ifdetach(ifp);/* free all nodes */
+ if_detach(ifp);
+ }
+
if (sc->amrr_xfer != NULL) {
usbd_free_xfer(sc->amrr_xfer);
sc->amrr_xfer = NULL;
@@ -547,8 +550,15 @@ ural_next_scan(void *arg)
struct ieee80211com *ic = &sc->sc_ic;
struct ifnet *ifp = &ic->ic_if;
+ if (usbd_is_dying(sc->sc_udev))
+ return;
+
+ usbd_ref_incr(sc->sc_udev);
+
if (ic->ic_state == IEEE80211_S_SCAN)
ieee80211_next_scan(ifp);
+
+ usbd_ref_decr(sc->sc_udev);
}
void
@@ -559,6 +569,9 @@ ural_task(void *arg)
enum ieee80211_state ostate;
struct ieee80211_node *ni;
+ if (usbd_is_dying(sc->sc_udev))
+ return;
+
ostate = ic->ic_state;
switch (sc->sc_state) {
@@ -574,7 +587,8 @@ ural_task(void *arg)
case IEEE80211_S_SCAN:
ural_set_chan(sc, ic->ic_bss->ni_chan);
- timeout_add_msec(&sc->scan_to, 200);
+ if (!usbd_is_dying(sc->sc_udev))
+ timeout_add_msec(&sc->scan_to, 200);
break;
case IEEE80211_S_AUTH:
@@ -1324,6 +1338,11 @@ ural_ioctl(struct ifnet *ifp, u_long cmd
struct ifreq *ifr;
int s, error = 0;
+ if (usbd_is_dying(sc->sc_udev))
+ return ENXIO;
+
+ usbd_ref_incr(sc->sc_udev);
+
s = splnet();
switch (cmd) {
@@ -1387,6 +1406,8 @@ ural_ioctl(struct ifnet *ifp, u_long cmd
splx(s);
+ usbd_ref_decr(sc->sc_udev);
+
return error;
}
@@ -2147,7 +2168,8 @@ ural_amrr_start(struct ural_softc *sc, s
i--);
ni->ni_txrate = i;
- timeout_add_sec(&sc->amrr_to, 1);
+ if (!usbd_is_dying(sc->sc_udev))
+ timeout_add_sec(&sc->amrr_to, 1);
}
void
@@ -2157,6 +2179,11 @@ ural_amrr_timeout(void *arg)
usb_device_request_t req;
int s;
+ if (usbd_is_dying(sc->sc_udev))
+ return;
+
+ usbd_ref_incr(sc->sc_udev);
+
s = splusb();
/*
@@ -2174,6 +2201,8 @@ ural_amrr_timeout(void *arg)
(void)usbd_transfer(sc->amrr_xfer);
splx(s);
+
+ usbd_ref_decr(sc->sc_udev);
}
void
@@ -2203,7 +2232,8 @@ ural_amrr_update(usbd_xfer_handle xfer,
ieee80211_amrr_choose(&sc->amrr, sc->sc_ic.ic_bss, &sc->amn);
- timeout_add_sec(&sc->amrr_to, 1);
+ if (!usbd_is_dying(sc->sc_udev)
