Date: Sun, 9 May 2004 12:40:57 -0700 (PDT) From: Don Lewis <truckman@FreeBSD.org> To: junior@p233.if.pwr.wroc.pl Cc: freebsd-hackers@FreeBSD.org Subject: Re: non-recursive mutex, sbc, pcm, kernel panic Message-ID: <200405091941.i49Jev7E096965@gw.catspoiler.org> In-Reply-To: <20040509085356.GA15318@p233.if.pwr.wroc.pl>
next in thread | previous in thread | raw e-mail | index | archive | help
On 9 May, Bartek Marcinkiewicz wrote:
> Hello,
>
> I've just experienced kernel panic while trying
> to play mp3 file. (My sound card: Creative Sound
> Blaster 16, ISA, worked fine on older 5.x system).
>
> While reading code:
>
> sys/dev/sound/pcm/sound.c::snd_mtxcreate() creates
> non-recursive mutex.
>
> But in sys/dev/sound/isa/sb16.c::sb_setup() we have:
>
> sb_setup()
> {
>
> sb_lock(sb);
>
> /* ... */
>
> sb_reset_dsp(sb);
>
> /* ... */
> }
>
> sb_reset_dsp() function locks this mutex again, causing
> panic with message:
> _mtx_lock_sleep: recursed on non-recursive mutex sbc0 @...
>
> Is this known issue? Why this mutex should be non-recursive?
> Attached patch makes it work again.
This isn't a known issue. It looks like you are the first person to
exercise the sb16 driver in FreeBSD-5.x in quite some time. Allowing
recursive locks makes it much more difficult to get the locking correct
since you lose the ability to use assertions about the lock state at
function entry and exit.
Typically the proper fix in this case would be to remove the
sb_lock() call from sb_reset_dsp() and always let the caller do the
locking. The patch below should do the trick, though I believe the
added locking and unlocking (the second section of the patch) could be
omitted in sb16_attach() since no other thread can access the device
while the attach is in progress.
Index: sys/dev/sound/isa/sb16.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/sound/isa/sb16.c,v
retrieving revision 1.83
diff -u -r1.83 sb16.c
--- sys/dev/sound/isa/sb16.c 14 Apr 2004 14:57:48 -0000 1.83
+++ sys/dev/sound/isa/sb16.c 9 May 2004 19:33:09 -0000
@@ -266,12 +266,10 @@
{
u_char b;
- sb_lock(sb);
sb_wr(sb, SBDSP_RST, 3);
DELAY(100);
sb_wr(sb, SBDSP_RST, 0);
b = sb_get_byte(sb);
- sb_unlock(sb);
if (b != 0xAA) {
DEB(printf("sb_reset_dsp 0x%lx failed\n",
rman_get_start(sb->io_base)));
@@ -799,8 +797,12 @@
if (sb16_alloc_resources(sb, dev))
goto no;
- if (sb_reset_dsp(sb))
+ sb_lock(sb);
+ if (sb_reset_dsp(sb)) {
+ sb_unlock(sb);
goto no;
+ }
+ sb_unlock(sb);
if (mixer_init(dev, &sb16mix_mixer_class, sb))
goto no;
if (snd_setup_intr(dev, sb->irq, 0, sb_intr, sb, &sb->ih))
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200405091941.i49Jev7E096965>