On Mon, Nov 22, 2010 at 09:09:17AM -0500, Ted Unangst wrote:
> On Mon, Nov 22, 2010 at 12:06 AM, Jacob Meuser <jake...@sdf.lonestar.org>
> wrote:
> > On Sun, Nov 21, 2010 at 06:47:01PM -0500, Ted Unangst wrote:
> >> instead of faking an assertwaitok check, let's use the real thing. this
> >> is almost the opposite of progress on the whole bluetooth issue, but it
> >> shortens the stack trace considerably. the curproc check isn't terribly
> >> accurate, either, so it misses bugs.
> >>
> >> i don't see any reason to avoid sleeping once we've decided that waiting
> >> is ok, so i changed that too.
> >
> > where was it decided that waiting is ok? because it's not supposed to
> > be interrupt context? I dunno about that. this function makes it into
> > a lot of places, and I don't see any that wait.
>
> Well, it is waiting! running btconfig up will panic later down the
> chain, with this function on the call stack. Sorry, don't have the
> exact trace handy. And it's already checking for curproc. That's
> just a broken test for the same thing.
I don't understand the purpose of this change. do you plan to change
the rest of the stack (usb_allocmem, *hci_allocm, etc) that calls
this function to wait for memory? are you sure the only reason for
the M_NOWAITs is to not wait in interrupt context? I'm not saying
this change is necessarily wrong, but by itself, it raises questions.
> >
> >
> >> Index: usb_mem.c
> >> ===================================================================
> >> RCS file: /home/tedu/cvs/src/sys/dev/usb/usb_mem.c,v
> >> retrieving revision 1.22
> >> diff -u -r1.22 usb_mem.c
> >> --- usb_mem.c 29 Sep 2010 20:06:38 -0000 1.22
> >> +++ usb_mem.c 21 Nov 2010 23:15:26 -0000
> >> @@ -98,12 +98,7 @@
> >> DPRINTFN(5, ("usb_block_allocmem: size=%lu align=%lu\n",
> >> (u_long)size, (u_long)align));
> >>
> >> -#ifdef DIAGNOSTIC
> >> - if (!curproc) {
> >> - printf("usb_block_allocmem: in interrupt context,
> size=%lu\n",
> >> - (unsigned long) size);
> >> - }
> >> -#endif
> >> + assertwaitok();
> >>
> >> s = splusb();
> >> /* First check the free list. */
> >> @@ -120,17 +115,8 @@
> >> }
> >> splx(s);
> >>
> >> -#ifdef DIAGNOSTIC
> >> - if (!curproc) {
> >> - printf("usb_block_allocmem: in interrupt context,
> failed\n");
> >> - return (USBD_NOMEM);
> >> - }
> >> -#endif
> >> -
> >> DPRINTFN(6, ("usb_block_allocmem: no free\n"));
> >> - p = malloc(sizeof *p, M_USB, M_NOWAIT);
> >> - if (p == NULL)
> >> - return (USBD_NOMEM);
> >> + p = malloc(sizeof *p, M_USB, M_WAITOK);
> >>
> >> p->tag = tag;
> >> p->size = size;
> >
> > --
> > jake...@sdf.lonestar.org
> > SDF Public Access UNIX System - http://sdf.lonestar.org
--
jake...@sdf.lonestar.org
SDF Public Access UNIX System - http://sdf.lonestar.org
