From owner-freebsd-current@FreeBSD.ORG Mon Jan 26 13:08:37 2004 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id BB33A16A4CE; Mon, 26 Jan 2004 13:08:37 -0800 (PST) Received: from gw.catspoiler.org (217-ip-163.nccn.net [209.79.217.163]) by mx1.FreeBSD.org (Postfix) with ESMTP id 0B0AC43D3F; Mon, 26 Jan 2004 13:08:36 -0800 (PST) (envelope-from truckman@FreeBSD.org) Received: from FreeBSD.org (mousie.catspoiler.org [192.168.101.2]) by gw.catspoiler.org (8.12.9p2/8.12.9) with ESMTP id i0QL7j7E078697; Mon, 26 Jan 2004 13:07:49 -0800 (PST) (envelope-from truckman@FreeBSD.org) Message-Id: <200401262107.i0QL7j7E078697@gw.catspoiler.org> Date: Mon, 26 Jan 2004 13:07:45 -0800 (PST) From: Don Lewis To: gbergling@0xfce3.net, mat@cnd.mcgill.ca, cg@FreeBSD.org In-Reply-To: <20040126175249.GA2500@nemesis.md.0xfce3.net> MIME-Version: 1.0 Content-Type: TEXT/plain; charset=us-ascii cc: freebsd-current@FreeBSD.org Subject: Re: dev/sound/pcm/* patch testers wanted X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 26 Jan 2004 21:08:37 -0000 On 26 Jan, Gordon Bergling wrote: > On Mon Jan 26, 2004 at 09:25AM -0800, Don Lewis wrote: >> On 26 Jan, Gordon Bergling wrote: >> >> > The "Danger!" message wasn't appeared, but I got the following on the >> > console. >> > >> > malloc() of "1024" with the following non-sleepable locks held: >> > exclusive sleep mutex pcm0:play:0 (pcm play channel) r = 0 (0xc2be8a80) >> > locked @ >> > /storage/os/src/freebsd-current/src/sys/dev/sound/pcm/buffer.c:195 >> > malloc() of "4096" with the following non-sleepable locks held: >> > exclusive sleep mutex pcm0:play:0 (pcm play channel) r = 0 (0xc2be8a80) >> > locked @ >> > /storage/os/src/freebsd-current/src/sys/dev/sound/pcm/buffer.c:195 >> >> A stack trace would be helpful so that we know where malloc() was being >> called. I've got a hunch, though. What hardware specific driver are >> you using? > > There was no stack trace. Can I print a stack trace with gdb or > something simliar? > > The hardware is the following: > pcm0: port 0xe000-0xe03f irq 11 at device 12.0 on pci0 > pcm0: Ok, the problem is that the setblocksize method in the es137x driver (as well as many others), calls sndbuf_resize(), which calls malloc(), and this patch changed the malloc() call from M_NOWAIT to M_WAITOK. Quick and dirty solutions: Switch back to M_NOWAIT even though we don't handle malloc() failure gracefully. Unlock the channel before calling the setblocksize method, but an interrupt happening in the middle of sndbuf_resize() would not be a good thing. Add a pointer back to the channel back to the snd_dbuf structure so that sndbuf_resize() could do the proper unlocking and locking. This is another manifestation of an API problem related to the setblocksize method. /* Increase the size of bufsoft if before increasing bufhard. */ maxsize = sndbuf_getsize(b); if (sndbuf_getsize(bs) > maxsize) maxsize = sndbuf_getsize(bs); if (reqblksz * blkcnt > maxsize) maxsize = reqblksz * blkcnt; if (sndbuf_getsize(bs) != maxsize || sndbuf_getblksz(bs) != reqblksz) { ret = sndbuf_remalloc(bs, maxsize/reqblksz, reqblksz, c->lock); if (ret) goto out1; } sndbuf_setblksz(b, CHANNEL_SETBLOCKSIZE(c->methods, c->devinfo, blksz)); /* Decrease the size of bufsoft after decreasing bufhard. */ maxsize = sndbuf_getsize(b); if (reqblksz * blkcnt > maxsize) maxsize = reqblksz * blkcnt; if (maxsize > sndbuf_getsize(bs)) printf("Danger! %s bufsoft size increasing from %d to %d after C HANNEL_SETBLOCKSIZE()\n", c->name, sndbuf_getsize(bs), maxsize); if (sndbuf_getsize(bs) != maxsize || sndbuf_getblksz(bs) != reqblksz) { ret = sndbuf_remalloc(bs, maxsize/reqblksz, reqblksz, c->lock); if (ret) goto out1; } Ideally, the the setblocksize method would calculate its desired blocksize and then perform a callback into the channel or buffer code that would resize both bufhard and bufsoft if necessary, with all the malloc() calls grouped together so that the channel lock only had to be dropped and grabbed once. The setblocksize method would have to pass through the desired bufsoft blksz (before scaling for format & speed) so that bufsoft is properly resized. This would eliminate the need for the two sndbuf_remalloc() calls above since we would know the actual size needed. Also, calling sndbuf_setblksz() on the return value from the setblocksize method is dangerous, since we end up with blksz*blkcnt != bufsize.