On Tue, Apr 13, 2010 at 3:50 AM, Attilio Rao <attilio_at_freebsd.org> wrote: > 2010/4/13 Attilio Rao <attilio_at_freebsd.org>: >> 2010/3/13 Garrett Cooper <yanefbsd_at_gmail.com>: >>> On Wed, Mar 10, 2010 at 9:58 PM, Garrett Cooper <yanefbsd_at_gmail.com> wrote: >>>> On Wed, Mar 10, 2010 at 2:07 PM, Tom Couch <tom.couch.storage_at_gmail.com> wrote: >>>>> Hi FreeBSD-current, >>>>> My name is Tom Couch, >>>>> I am part of the 3ware driver team recently acquired by LSI. >>>>> I believe Giovanni's patch, below, is the correct fix for this bug. >>>>> >>>>> I am available to maintain the twa driver, now that I am on this list. >>>>> Let me know how I can help, >>>>> >>>>> Tom >>>>> >>>>> >>>>> On Wed, Mar 10, 2010 at 1:56 PM, Giovanni Trematerra < >>>>> giovanni.trematerra_at_gmail.com> wrote: >>>>> >>>>>> On Sun, Mar 7, 2010 at 11:24 AM, Garrett Cooper <yanefbsd_at_gmail.com> >>>>>> wrote: >>>>>> > On Sun, Mar 7, 2010 at 2:07 AM, Garrett Cooper <yanefbsd_at_gmail.com> >>>>>> wrote: >>>>>> >> Hi Alexander and Hans, >>>>>> >> I recently did the following which generated a panic on a >>>>>> >> 9-CURRENT kernel compiled on the 26th: >>>>>> >> >>>>>> >> 1. Executed reboot >>>>>> >> 2. Removed keyboard. >>>>>> >> 3. Some time after `All buffers synced\nUptime: ...' was displayed, >>>>>> >> the keyboard was registered disconnected. >>>>>> >> 4. The interrupt was delivered to my twa(4) enabled card and the >>>>>> >> kernel panicked, like so: >>>>>> >> >>>>>> >> ugen2.2: <Mitsumi Electric> at usbus2 (disconnected) >>>>>> >> uhub8: at uhub2, port 1, addr 2 (disconnected) >>>>>> >> ugen2.3: <Mitsumi Electric> at usbus2 (disconnected) >>>>>> >> ukbd0: at uhub8, port 3, addr 3 (disconnected) >>>>>> >> uhid0: at uhub8, port 3, addr 3 (disconnected) >>>>>> >> panic: mtx_lock_spin() of destroyed mutex _at_ >>>>>> /usr/src/sys/dev/twa/tw_cl_intr.c:88 >>>>>> >> >>>>>> >> cpuid = 1 >>>>>> >> KDB: enter: panic >>>>>> >> [thread pid 12 tid 100025 ] >>>>>> >> Stopped at kdb_enter+0x3d: movq $0,0x40289c(%rip) >>>>>> >> db> >>>>>> >> >>>>>> >> I wish I could provide you with more details, but unfortunately I >>>>>> >> the USB bus isn't registering the fact that I'm reattaching the >>>>>> >> keyboard right now and the box won't reboot automatically :( (didn't >>>>>> >> set the right sysctl beforehand to panic automatically). I'll try and >>>>>> >> reproduce the issue again, but I was just wondering whether or not you >>>>>> >> guys had seen this problem before. >>>>>> > >>>>>> > Phew... it's reproducible with that kernel. Here's what I did >>>>>> > exactly (because my original directions were incorrect): >>>>>> > 1. Hit power button (for S5). >>>>>> > 2. Disconnect keyboard RIGHT as `Uptime: ...' is displayed. >>>>>> > Kernel panicked on my system again. Now to figure out if it still >>>>>> > exists with a kernel compiled today, and also how to debug it if it >>>>>> > still does exist :/... >>>>>> > Thanks, >>>>>> > -Garrett >>>>>> >>>>>> Hi Garrett, >>>>>> Could you please try the patch below and report back? >>>>>> >>>>>> Thank you >>>>>> >>>>>> diff -r cab6489de66d sys/dev/twa/tw_cl_intr.c >>>>>> --- a/sys/dev/twa/tw_cl_intr.c Wed Mar 03 04:51:13 2010 -0500 >>>>>> +++ b/sys/dev/twa/tw_cl_intr.c Wed Mar 10 06:29:05 2010 -0500 >>>>>> _at__at_ -75,9 +75,12 _at__at_ tw_cl_interrupt(struct tw_cl_ctlr_handle >>>>>> if (ctlr == NULL) >>>>>> goto out; >>>>>> >>>>>> - /* If we get an interrupt while resetting, it is a shared >>>>>> - one for another device, so just bail */ >>>>>> - if (ctlr->state & TW_CLI_CTLR_STATE_RESET_IN_PROGRESS) >>>>>> + /* >>>>>> + * If we get an interrupt while resetting or shutting down, >>>>>> + * it is a shared one for another device, so just bail >>>>>> + */ >>>>>> + if (ctlr->state & TW_CLI_CTLR_STATE_RESET_IN_PROGRESS || >>>>>> + (ctrl->state & TW_CLI_CTLR_STATE_ACTIVE) == 0) >>>>>> goto out; >>>>>> >>>>>> /* >>> >>> Apart from the typo above (s/ctrl/ctlr/), things work appropriately >>> now at reboot. The only problem is that bootup is really wonky now, >>> because the RAID had a LOT of issues attaching to cam(4) (failed in >>> 2/3 cold boot attempts); an additional branch condition may need to be >>> added to the above if-statement if this change didn't take that into >>> account. However, if the old behavior was incorrect and the new >>> behavior is correct, s.t. the RAID controller demonstrating bus >>> detection timeout issue that is occurring with a lot of USB devices >>> and some RAID controllers today, this could be extremely problematic. >>> >>> So, while it looks better than before at reboot, it's not ready yet >>> for prime time; I'd rather that the bug was filed with the patch you >>> provided after the typo fixed, with the caveat mentioned and NOT >>> committed, because the adverse affect(s) seem a bit more annoying than >>> the previous panic issue described. >> >> I looked briefly at it and I think there are 2 bugs, one in >> twa_detach() and another one in twa_shutdown(). >> Basically, locks get destroyed in tw_cl_shutdown_ctlr() which is >> called by twa_shutdown() while interrupts are teared down in >> tw_osli_free_resource(). twa_shutdown() is called in twa_detach() >> before than tw_osli_free_resource(). >> tw_cl_shutdown_ctlr() also takes care to disable the interrupts for >> twa but a problem can arise with shared IRQ. Infact, the handler will >> remain on the IRQ until the bus_intr_teardown() takes place and it may >> run, trying to acknowledge the interrupt, but with destroyed lock, if >> an interrupt is sent by a shared source between twa_shutdown() and >> tw_osli_free_resource() call in twa_detach() or just after a simple >> call to twa_shutdown(). >> >> Problems I see here: >> - twa_shutdown() should not destroy the mutex at all, it is not >> something our shutdown handlers generally do and it might be kept in >> sync >> - twa_detach() might do a first half of tw_cl_shutdown_ctlr(), do the >> resource deallocation and just at the end destroy mutexes. That is how >> generally our detach handler works. >> >> All these solutions would mean refactoring the tw_osli_free_resource() >> and tw_cl_shutdown_ctlr(). I don't know very well the twa code, but it >> seems to me that we want to keep the driver very compatible with any >> vendor version or such? If yes this may be a problem because the >> failing patterns are all located into the shared code and an ideal >> solution could be more difficult to find out. Otherwise a fix might be >> simple to hammer down. > > Forgot to tell: twe might have the same problem even if it doesn't > expose just for luckiness. Hmmm... ok. I don't have a twe enabled card so I can't verify whether or not this problem exists :/. Thanks for the comments! -GarrettReceived on Tue Apr 13 2010 - 12:44:32 UTC
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:02 UTC