Date: Tue, 16 Sep 2008 22:06:46 +0900 From: Weongyo Jeong <weongyo.jeong@gmail.com> To: Sam Leffler <sam@freebsd.org> Cc: cvs-src@freebsd.org, src-committers@freebsd.org, cvs-all@freebsd.org Subject: Re: cvs commit: src/sys/dev/usb if_zyd.c if_zydreg.h Message-ID: <20080916130646.GA52103@freebsd.weongyo.org> In-Reply-To: <48C93C1B.2080800@freebsd.org> References: <200809100341.m8A3f5n4001038@repoman.freebsd.org> <48C7EF65.3050204@freebsd.org> <20080911073458.GB23009@freebsd.weongyo.org> <48C93C1B.2080800@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Sep 11, 2008 at 08:41:15AM -0700, Sam Leffler wrote:
> Weongyo Jeong wrote:
> >On Wed, Sep 10, 2008 at 09:01:41AM -0700, Sam Leffler wrote:
> >
> >>Weongyo Jeong wrote:
> >>
> >>>weongyo 2008-09-10 03:40:51 UTC
> >>>
> >>> FreeBSD src repository
> >>>
> >>> Modified files:
> >>> sys/dev/usb if_zyd.c if_zydreg.h
> >>> Log:
> >>> SVN rev 182900 on 2008-09-10 03:40:51Z by weongyo
> >>>
> >>> rename flags and add a ZYD_FLAG_DETACHING flag to indicate we're
> >>> detaching that when the USB is pulled out forcibly during the driver is
> >>> running background scan, a page fault can be occurred even if we called
> >>> usb_rem_task() when detaching. It looks like a kind of races.
> >>>
> >>>
> >>>
> >>If I understand the issue, it should be handled in the 802.11 state
> >>machine. The device should be clocked to the INIT state and as a result
> >>clear any outstanding tasks, timers, etc. The only reason you need to
> >>do something special is if the h/w is gone and you need to guard against
> >>accessing it.
> >>
> >> Sam
> >>
> >>
> >
> >This patch is to fix the below panic that it looks that it sometimes
> >occurs when we pull out USB stick forcibly during the driver's trying to
> >search channels or run background scan.
> >
> >If we have a method to detect whether detach() is called by being pulled
> >out USB stick unexpectedly or detach() is called by operations of
> >kldunload(8), I think I can handle this case more flexibly. (I'm not
> >sure it's true. :-) Is there a way to detect this case or something I
> >missed?
> >
> >I have no ideas yet how I can handle it in 802.11 state machine.
> >
> >[root@kkk /usr/src/sys/modules/zyd]# zyd0: at uhub3 port 4 (addr 2)
> >disconnected
> >zyd0: zyd_read sleep timeout
> >zyd0: could not send command (error=IOERROR)
> >zyd0: could not send command (error=IOERROR)
> >zyd0: could not send command (error=IOERROR)
> >zyd0: zyd_read sleep timeout
> >zyd0: could not send command (error=IOERROR)
> >zyd0: zyd_read sleep timeout
> >zyd0: could not send command (error=IOERROR)
> >zyd0: could not send command (error=IOERROR)
> >zyd0: could not send command (error=IOERROR)
> >zyd0: detached
> >
> >Fatal trap 12: page fault while in kernel mode
> >cpuid = 0; apic id = 00
> >fault virtual address = 0x6a626f7f
> >fault code = supervisor read, page not present
> >instruction pointer = 0x20:0xc07908b6
> >stack pointer = 0x28:0xc3e30aec
> >frame pointer = 0x28:0xc3e30aec
> >code segment = base 0x0, limit 0xfffff, type 0x1b
> >= DPL 0, pres 1, def32 1, gran 1
> >processor eflags = interrupt enabled, resume, IOPL = 0
> >current process = 16 (usbtask-dr)
> >[thread pid 16 tid 100026 ]
> >Stopped at devclass_get_name+0x6: movl 0x14(%eax),%eax
> >db> bt
> >Tracing pid 16 tid 100026 td 0xc40cc8c0
> >devclass_get_name(6a626f6b,c3e30b14,c0790e77,deadc0de,ffffffff,...) at
> >devclass_get_name+0x6
> >device_get_name(deadc0de,ffffffff,0,c69c2a00,1,...) at device_get_name+0x1c
> >device_print_prettyname(deadc0de,c69c2a00,1,c69c2a00,c3e30bc4,...) at
> >device_print_prettyname+0x17
> >device_printf(deadc0de,c69cc297,100,c69cc290,3e8,...) at device_printf+0x12
> >zyd_cmd(2,c3e30be0,1,1,c3e5932c,...) at zyd_cmd+0x1f0
> >zyd_read16(c69cc567,c0bd8504,c3e30c10,c07aa70b,c69cc567,...) at
> >zyd_read16+0x38
> >zyd_rfwrite(1,c69c2a00,c3e30c98,c69c9124,c69c2a08,...) at zyd_rfwrite+0x1c
> >zyd_al2230_set_channel(c69c2a08,2,c69cc567,ac7,0,...) at
> >zyd_al2230_set_channel+0x21
> >zyd_set_chan(c0ca30d0,0,c69cc567,ac7,c0c38d34,...) at zyd_set_chan+0x54
> >zyd_scantask(c69c2a00,0,5c,c0af321d,0,...) at zyd_scantask+0xe1
> >usb_task_thread(c0c38d34,c3e30d38,c0afb326,322,c40cc8c0,...) at
> >usb_task_thread+0xca
> >fork_exit(c06dcfd0,c0c38d34,c3e30d38) at fork_exit+0x112
> >fork_trampoline() at fork_trampoline+0x8
> >--- trap 0, eip = 0, esp = 0xc3e30d70, ebp = 0 ---
> >db>
> >
> >
>
> In zyd_newstate the current code does this:
>
> usb_rem_task(sc->sc_udev, &sc->sc_task);
>
> /* do it in a process context */
> sc->sc_state = nstate;
> sc->sc_arg = arg;
>
> if (nstate == IEEE80211_S_INIT) {
> zvp->newstate(vap, nstate, arg);
> return 0;
> } else {
> usb_add_task(sc->sc_udev, &sc->sc_task, USB_TASKQ_DRIVER);
> return EINPROGRESS;
> }
>
> You need to clear any pending task callbacks, not just the one
> associated with sc_udev (unless your callbacks do evil things like check
> the current state to deal with races). The transition to the INIT state
> must execute synchronously for multiple reasons (including detach); in
> that case it looks like you must explicitly cancel the task that handles
> scan-related work.
Oh. I see. :-) I'll will test and commit it. Thanks!
regards,
Weongyo Jeong
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080916130646.GA52103>