On 23 Feb, Brian O'Shea wrote:
> --- Don Lewis <[EMAIL PROTECTED]> wrote:
>> 
>> The cause of deadlocks is more likely to be caught by WITNESS.  In this
> 
> With WITNESS the hang still occurrs, and still no panic.  It's hard to
> tell since the problem tends to happen at random times, but it seems like
> it happens more quickly with the kernel that has WITNESS and INVARIANTS
> enabled.

Can you see any kernel diagnostic messages, or are you running X?  If
you are running X, can you set up a serial console, or can you reproduce
the hang while switched to a text console?

Depending on the cause of the problem, you might need to build a kernel
with DDB support and break into DDB from the console while the machine
is hung to figure out the cause.

>> case it might be the result of a malloc() call while a mutex is held.
>> Even the version of the sound code in the most recent -CURRENT has some
>> problems in this area.  I've got a patch out for testing that will this
>> problem to some extent, but it might not be enough.
>> 
> 
> I'd be willing to test your patch, if that would help.  Where can I
> find it?

The patch below should be applied to a recent version of -CURRENT.

Index: sys/dev/sound/pcm/dsp.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/sound/pcm/dsp.c,v
retrieving revision 1.73
diff -u -r1.73 dsp.c
--- sys/dev/sound/pcm/dsp.c     21 Feb 2004 21:10:47 -0000      1.73
+++ sys/dev/sound/pcm/dsp.c     22 Feb 2004 23:11:43 -0000
@@ -444,7 +444,7 @@
 static int
 dsp_ioctl(dev_t i_dev, u_long cmd, caddr_t arg, int mode, struct thread *td)
 {
-       struct pcm_channel *wrch, *rdch;
+       struct pcm_channel *chn, *rdch, *wrch;
        struct snddev_info *d;
        intrmask_t s;
        int kill;
@@ -477,22 +477,19 @@
        if (kill & 2)
                rdch = NULL;
        
-       if (rdch != NULL)
-               CHN_LOCK(rdch);
-       if (wrch != NULL)
-               CHN_LOCK(wrch);
-
        switch(cmd) {
 #ifdef OLDPCM_IOCTL
        /*
         * we start with the new ioctl interface.
         */
        case AIONWRITE: /* how many bytes can write ? */
+               CHN_LOCK(wrch);
 /*
                if (wrch && wrch->bufhard.dl)
                        while (chn_wrfeed(wrch) == 0);
 */
                *arg_i = wrch? sndbuf_getfree(wrch->bufsoft) : 0;
+               CHN_UNLOCK(wrch);
                break;
 
        case AIOSSIZE:     /* set the current blocksize */
@@ -502,12 +499,16 @@
                        p->play_size = 0;
                        p->rec_size = 0;
                        if (wrch) {
+                               CHN_LOCK(wrch);
                                chn_setblocksize(wrch, 2, p->play_size);
                                p->play_size = sndbuf_getblksz(wrch->bufsoft);
+                               CHN_UNLOCK(wrch);
                        }
                        if (rdch) {
+                               CHN_LOCK(rdch);
                                chn_setblocksize(rdch, 2, p->rec_size);
                                p->rec_size = sndbuf_getblksz(rdch->bufsoft);
+                               CHN_UNLOCK(rdch);
                        }
                }
                break;
@@ -515,37 +516,51 @@
                {
                        struct snd_size *p = (struct snd_size *)arg;
 
-                       if (wrch)
+                       if (wrch) {
+                               CHN_LOCK(wrch);
                                p->play_size = sndbuf_getblksz(wrch->bufsoft);
-                       if (rdch)
+                               CHN_UNLOCK(wrch);
+                       }
+                       if (rdch) {
+                               CHN_LOCK(rdch);
                                p->rec_size = sndbuf_getblksz(rdch->bufsoft);
+                               CHN_UNLOCK(rdch);
+                       }
                }
                break;
 
        case AIOSFMT:
+       case AIOGFMT:
                {
                        snd_chan_param *p = (snd_chan_param *)arg;
 
                        if (wrch) {
-                               chn_setformat(wrch, p->play_format);
-                               chn_setspeed(wrch, p->play_rate);
+                               CHN_LOCK(wrch);
+                               if (cmd == AIOSFMT) {
+                                       chn_setformat(wrch, p->play_format);
+                                       chn_setspeed(wrch, p->play_rate);
+                               }
+                               p->play_rate = wrch->speed;
+                               p->play_format = wrch->format;
+                               CHN_UNLOCK(wrch);
+                       } else {
+                               p->play_rate = 0;
+                               p->play_format = 0;
                        }
                        if (rdch) {
-                               chn_setformat(rdch, p->rec_format);
-                               chn_setspeed(rdch, p->rec_rate);
+                               CHN_LOCK(rdch);
+                               if (cmd == AIOSFMT) {
+                                       chn_setformat(rdch, p->rec_format);
+                                       chn_setspeed(rdch, p->rec_rate);
+                               }
+                               p->rec_rate = rdch->speed;
+                               p->rec_format = rdch->format;
+                               CHN_UNLOCK(rdch);
+                       } else {
+                               p->rec_rate = 0;
+                               p->rec_format = 0;
                        }
                }
-               /* FALLTHROUGH */
-
-       case AIOGFMT:
-               {
-                       snd_chan_param *p = (snd_chan_param *)arg;
-
-                       p->play_rate = wrch? wrch->speed : 0;
-                       p->rec_rate = rdch? rdch->speed : 0;
-                       p->play_format = wrch? wrch->format : 0;
-                       p->rec_format = rdch? rdch->format : 0;
-               }
                break;
 
        case AIOGCAP:     /* get capabilities */
@@ -554,10 +569,14 @@
                        struct pcmchan_caps *pcaps = NULL, *rcaps = NULL;
                        dev_t pdev;
 
-                       if (rdch)
+                       if (rdch) {
+                               CHN_LOCK(rdch);
                                rcaps = chn_getcaps(rdch);
-                       if (wrch)
+                       }
+                       if (wrch) {
+                               CHN_LOCK(wrch);
                                pcaps = chn_getcaps(wrch);
+                       }
                        p->rate_min = max(rcaps? rcaps->minspeed : 0,
                                          pcaps? pcaps->minspeed : 0);
                        p->rate_max = min(rcaps? rcaps->maxspeed : 1000000,
@@ -573,15 +592,23 @@
                        p->mixers = 1; /* default: one mixer */
                        p->inputs = pdev->si_drv1? mix_getdevs(pdev->si_drv1) : 0;
                        p->left = p->right = 100;
+                       if (rdch)
+                               CHN_UNLOCK(rdch);
+                       if (wrch)
+                               CHN_UNLOCK(wrch);
                }
                break;
 
        case AIOSTOP:
-               if (*arg_i == AIOSYNC_PLAY && wrch)
+               if (*arg_i == AIOSYNC_PLAY && wrch) {
+                       CHN_LOCK(wrch);
                        *arg_i = chn_abort(wrch);
-               else if (*arg_i == AIOSYNC_CAPTURE && rdch)
+                       CHN_UNLOCK(wrch);
+               } else if (*arg_i == AIOSYNC_CAPTURE && rdch) {
+                       CHN_LOCK(rdch);
                        *arg_i = chn_abort(rdch);
-               else {
+                       CHN_UNLOCK(rdch);
+               } else {
                        printf("AIOSTOP: bad channel 0x%x\n", *arg_i);
                        *arg_i = 0;
                }
@@ -596,9 +623,15 @@
         * here follow the standard ioctls (filio.h etc.)
         */
        case FIONREAD: /* get # bytes to read */
-/*             if (rdch && rdch->bufhard.dl)
-                       while (chn_rdfeed(rdch) == 0);
-*/             *arg_i = rdch? sndbuf_getready(rdch->bufsoft) : 0;
+               if (rdch) {
+                       CHN_LOCK(rdch);
+/*                     if (rdch && rdch->bufhard.dl)
+                               while (chn_rdfeed(rdch) == 0);
+*/
+                       *arg_i = sndbuf_getready(rdch->bufsoft);
+                       CHN_UNLOCK(rdch);
+               } else
+                       *arg_i = 0;
                break;
 
        case FIOASYNC: /*set/clear async i/o */
@@ -607,15 +640,21 @@
 
        case SNDCTL_DSP_NONBLOCK:
        case FIONBIO: /* set/clear non-blocking i/o */
-               if (rdch)
-                       rdch->flags &= ~CHN_F_NBIO;
-               if (wrch)
-                       wrch->flags &= ~CHN_F_NBIO;
-               if (*arg_i) {
-                       if (rdch)
+               if (rdch) {
+                       CHN_LOCK(rdch);
+                       if (*arg_i)
                                rdch->flags |= CHN_F_NBIO;
-                       if (wrch)
+                       else
+                               rdch->flags &= ~CHN_F_NBIO;
+                       CHN_UNLOCK(rdch);
+               }
+               if (wrch) {
+                       CHN_LOCK(wrch);
+                       if (*arg_i)
                                wrch->flags |= CHN_F_NBIO;
+                       else
+                               wrch->flags &= ~CHN_F_NBIO;
+                       CHN_UNLOCK(wrch);
                }
                break;
 
@@ -625,71 +664,93 @@
 #define THE_REAL_SNDCTL_DSP_GETBLKSIZE _IOWR('P', 4, int)
        case THE_REAL_SNDCTL_DSP_GETBLKSIZE:
        case SNDCTL_DSP_GETBLKSIZE:
-               if (wrch)
-                       *arg_i = sndbuf_getblksz(wrch->bufsoft);
-               else if (rdch)
-                       *arg_i = sndbuf_getblksz(rdch->bufsoft);
-               else
-                       *arg_i = 0;
+               chn = wrch ? wrch : rdch;
+               CHN_LOCK(chn);
+               *arg_i = sndbuf_getblksz(chn->bufsoft);
+               CHN_UNLOCK(chn);
                break ;
 
        case SNDCTL_DSP_SETBLKSIZE:
                RANGE(*arg_i, 16, 65536);
-               if (wrch)
+               if (wrch) {
+                       CHN_LOCK(wrch);
                        chn_setblocksize(wrch, 2, *arg_i);
-               if (rdch)
+                       CHN_UNLOCK(wrch);
+               }
+               if (rdch) {
+                       CHN_LOCK(rdch);
                        chn_setblocksize(rdch, 2, *arg_i);
+                       CHN_UNLOCK(rdch);
+               }
                break;
 
        case SNDCTL_DSP_RESET:
                DEB(printf("dsp reset\n"));
                if (wrch) {
+                       CHN_LOCK(wrch);
                        chn_abort(wrch);
                        chn_resetbuf(wrch);
+                       CHN_UNLOCK(wrch);
                }
                if (rdch) {
+                       CHN_LOCK(rdch);
                        chn_abort(rdch);
                        chn_resetbuf(rdch);
+                       CHN_UNLOCK(rdch);
                }
                break;
 
        case SNDCTL_DSP_SYNC:
                DEB(printf("dsp sync\n"));
                /* chn_sync may sleep */
-               if (wrch)
+               if (wrch) {
+                       CHN_LOCK(wrch);
                        chn_sync(wrch, sndbuf_getsize(wrch->bufsoft) - 4);
+                       CHN_UNLOCK(wrch);
+               }
                break;
 
        case SNDCTL_DSP_SPEED:
                /* chn_setspeed may sleep */
                tmp = 0;
                if (wrch) {
+                       CHN_LOCK(wrch);
                        ret = chn_setspeed(wrch, *arg_i);
                        tmp = wrch->speed;
+                       CHN_UNLOCK(wrch);
                }
                if (rdch && ret == 0) {
+                       CHN_LOCK(rdch);
                        ret = chn_setspeed(rdch, *arg_i);
                        if (tmp == 0)
                                tmp = rdch->speed;
+                       CHN_UNLOCK(rdch);
                }
                *arg_i = tmp;
                break;
 
        case SOUND_PCM_READ_RATE:
-               *arg_i = wrch? wrch->speed : rdch->speed;
+               chn = wrch ? wrch : rdch;
+               CHN_LOCK(chn);
+               *arg_i = chn->speed;
+               CHN_UNLOCK(chn);
                break;
 
        case SNDCTL_DSP_STEREO:
                tmp = -1;
                *arg_i = (*arg_i)? AFMT_STEREO : 0;
                if (wrch) {
+                       CHN_LOCK(wrch);
                        ret = chn_setformat(wrch, (wrch->format & ~AFMT_STEREO) | 
*arg_i);
                        tmp = (wrch->format & AFMT_STEREO)? 1 : 0;
+                       CHN_UNLOCK(wrch);
                }
                if (rdch && ret == 0) {
+                       CHN_LOCK(rdch);
                        ret = chn_setformat(rdch, (rdch->format & ~AFMT_STEREO) | 
*arg_i);
                        if (tmp == -1)
                                tmp = (rdch->format & AFMT_STEREO)? 1 : 0;
+                       CHN_UNLOCK(rdch);
                }
                *arg_i = tmp;
                break;
@@ -700,47 +761,67 @@
                        tmp = 0;
                        *arg_i = (*arg_i != 1)? AFMT_STEREO : 0;
                        if (wrch) {
+                               CHN_LOCK(wrch);
                                ret = chn_setformat(wrch, (wrch->format & 
~AFMT_STEREO) | *arg_i);
                                tmp = (wrch->format & AFMT_STEREO)? 2 : 1;
+                               CHN_UNLOCK(wrch);
                        }
                        if (rdch && ret == 0) {
+                               CHN_LOCK(rdch);
                                ret = chn_setformat(rdch, (rdch->format & 
~AFMT_STEREO) | *arg_i);
                                if (tmp == 0)
                                        tmp = (rdch->format & AFMT_STEREO)? 2 : 1;
+                               CHN_UNLOCK(rdch);
                        }
                        *arg_i = tmp;
-               } else
-                       *arg_i = ((wrch? wrch->format : rdch->format) & AFMT_STEREO)? 
2 : 1;
+               } else {
+                       chn = wrch ? wrch : rdch;
+                       CHN_LOCK(chn);
+                       *arg_i = (chn->format & AFMT_STEREO) ? 2 : 1;
+                       CHN_UNLOCK(chn);
+               }
                break;
 
        case SOUND_PCM_READ_CHANNELS:
-               *arg_i = ((wrch? wrch->format : rdch->format) & AFMT_STEREO)? 2 : 1;
+               chn = wrch ? wrch : rdch;
+               CHN_LOCK(chn);
+               *arg_i = (chn->format & AFMT_STEREO) ? 2 : 1;
+               CHN_UNLOCK(chn);
                break;
 
        case SNDCTL_DSP_GETFMTS:        /* returns a mask of supported fmts */
-               *arg_i = wrch? chn_getformats(wrch) : chn_getformats(rdch);
+               chn = wrch ? wrch : rdch;
+               CHN_LOCK(chn);
+               *arg_i = chn_getformats(chn);
+               CHN_UNLOCK(chn);
                break ;
 
        case SNDCTL_DSP_SETFMT: /* sets _one_ format */
-               /* XXX locking */
                if ((*arg_i != AFMT_QUERY)) {
                        tmp = 0;
                        if (wrch) {
+                               CHN_LOCK(wrch);
                                ret = chn_setformat(wrch, (*arg_i) | (wrch->format & 
AFMT_STEREO));
                                tmp = wrch->format & ~AFMT_STEREO;
+                               CHN_UNLOCK(wrch);
                        }
                        if (rdch && ret == 0) {
+                               CHN_LOCK(rdch);
                                ret = chn_setformat(rdch, (*arg_i) | (rdch->format & 
AFMT_STEREO));
                                if (tmp == 0)
                                        tmp = rdch->format & ~AFMT_STEREO;
+                               CHN_UNLOCK(rdch);
                        }
                        *arg_i = tmp;
-               } else
-                       *arg_i = (wrch? wrch->format : rdch->format) & ~AFMT_STEREO;
+               } else {
+                       chn = wrch ? wrch : rdch;
+                       CHN_LOCK(chn);
+                       *arg_i = chn->format & ~AFMT_STEREO;
+                       CHN_UNLOCK(chn);
+               }
                break;
 
        case SNDCTL_DSP_SETFRAGMENT:
-               /* XXX locking */
                DEB(printf("SNDCTL_DSP_SETFRAGMENT 0x%08x\n", *(int *)arg));
                {
                        u_int32_t fragln = (*arg_i) & 0x0000ffff;
@@ -759,14 +840,18 @@
 
                        DEB(printf("SNDCTL_DSP_SETFRAGMENT %d frags, %d sz\n", 
maxfrags, fragsz));
                        if (rdch) {
+                               CHN_LOCK(rdch);
                                ret = chn_setblocksize(rdch, maxfrags, fragsz);
                                maxfrags = sndbuf_getblkcnt(rdch->bufsoft);
                                fragsz = sndbuf_getblksz(rdch->bufsoft);
+                               CHN_UNLOCK(rdch);
                        }
                        if (wrch && ret == 0) {
+                               CHN_LOCK(wrch);
                                ret = chn_setblocksize(wrch, maxfrags, fragsz);
                                maxfrags = sndbuf_getblkcnt(wrch->bufsoft);
                                fragsz = sndbuf_getblksz(wrch->bufsoft);
+                               CHN_UNLOCK(wrch);
                        }
 
                        fragln = 0;
@@ -785,10 +870,12 @@
                        if (rdch) {
                                struct snd_dbuf *bs = rdch->bufsoft;
 
+                               CHN_LOCK(rdch);
                                a->bytes = sndbuf_getready(bs);
                                a->fragments = a->bytes / sndbuf_getblksz(bs);
                                a->fragstotal = sndbuf_getblkcnt(bs);
                                a->fragsize = sndbuf_getblksz(bs);
+                               CHN_UNLOCK(rdch);
                        }
                }
                break;
@@ -800,11 +887,13 @@
                        if (wrch) {
                                struct snd_dbuf *bs = wrch->bufsoft;
 
+                               CHN_LOCK(wrch);
                                chn_wrupdate(wrch);
                                a->bytes = sndbuf_getfree(bs);
                                a->fragments = a->bytes / sndbuf_getblksz(bs);
                                a->fragstotal = sndbuf_getblkcnt(bs);
                                a->fragsize = sndbuf_getblksz(bs);
+                               CHN_UNLOCK(wrch);
                        }
                }
                break;
@@ -815,11 +904,13 @@
                        if (rdch) {
                                struct snd_dbuf *bs = rdch->bufsoft;
 
+                               CHN_LOCK(rdch);
                                chn_rdupdate(rdch);
                                a->bytes = sndbuf_gettotal(bs);
                                a->blocks = sndbuf_getblocks(bs) - rdch->blocks;
                                a->ptr = sndbuf_getreadyptr(bs);
                                rdch->blocks = sndbuf_getblocks(bs);
+                               CHN_UNLOCK(rdch);
                        } else
                                ret = EINVAL;
                }
@@ -831,11 +922,13 @@
                        if (wrch) {
                                struct snd_dbuf *bs = wrch->bufsoft;
 
+                               CHN_LOCK(wrch);
                                chn_wrupdate(wrch);
                                a->bytes = sndbuf_gettotal(bs);
                                a->blocks = sndbuf_getblocks(bs) - wrch->blocks;
                                a->ptr = sndbuf_getreadyptr(bs);
                                wrch->blocks = sndbuf_getblocks(bs);
+                               CHN_UNLOCK(wrch);
                        } else
                                ret = EINVAL;
                }
@@ -848,32 +941,47 @@
                break;
 
        case SOUND_PCM_READ_BITS:
-               *arg_i = ((wrch? wrch->format : rdch->format) & AFMT_16BIT)? 16 : 8;
+               chn = wrch ? wrch : rdch;
+               CHN_LOCK(chn);
+               *arg_i = (chn->format & AFMT_16BIT) ? 16 : 8;
+               CHN_UNLOCK(chn);
                break;
 
        case SNDCTL_DSP_SETTRIGGER:
                if (rdch) {
+                       CHN_LOCK(rdch);
                        rdch->flags &= ~(CHN_F_TRIGGERED | CHN_F_NOTRIGGER);
                        if (*arg_i & PCM_ENABLE_INPUT)
                                chn_start(rdch, 1);
                        else
                                rdch->flags |= CHN_F_NOTRIGGER;
+                       CHN_UNLOCK(rdch);
                }
                if (wrch) {
+                       CHN_LOCK(wrch);
                        wrch->flags &= ~(CHN_F_TRIGGERED | CHN_F_NOTRIGGER);
                        if (*arg_i & PCM_ENABLE_OUTPUT)
                                chn_start(wrch, 1);
                        else
                                wrch->flags |= CHN_F_NOTRIGGER;
+                       CHN_UNLOCK(wrch);
                }
                break;
 
        case SNDCTL_DSP_GETTRIGGER:
                *arg_i = 0;
-               if (wrch && wrch->flags & CHN_F_TRIGGERED)
-                       *arg_i |= PCM_ENABLE_OUTPUT;
-               if (rdch && rdch->flags & CHN_F_TRIGGERED)
-                       *arg_i |= PCM_ENABLE_INPUT;
+               if (wrch) {
+                       CHN_LOCK(wrch);
+                       if (wrch->flags & CHN_F_TRIGGERED)
+                               *arg_i |= PCM_ENABLE_OUTPUT;
+                       CHN_UNLOCK(wrch);
+               }
+               if (rdch) {
+                       CHN_LOCK(rdch);
+                       if (rdch->flags & CHN_F_TRIGGERED)
+                               *arg_i |= PCM_ENABLE_INPUT;
+                       CHN_UNLOCK(rdch);
+               }
                break;
 
        case SNDCTL_DSP_GETODELAY:
@@ -881,16 +989,20 @@
                        struct snd_dbuf *b = wrch->bufhard;
                        struct snd_dbuf *bs = wrch->bufsoft;
 
+                       CHN_LOCK(wrch);
                        chn_wrupdate(wrch);
                        *arg_i = sndbuf_getready(b) + sndbuf_getready(bs);
+                       CHN_UNLOCK(wrch);
                } else
                        ret = EINVAL;
                break;
 
        case SNDCTL_DSP_POST:
                if (wrch) {
+                       CHN_LOCK(wrch);
                        wrch->flags &= ~CHN_F_NOTRIGGER;
                        chn_start(wrch, 1);
+                       CHN_UNLOCK(wrch);
                }
                break;
 
@@ -908,10 +1020,6 @@
                ret = EINVAL;
                break;
        }
-       if (rdch != NULL)
-               CHN_UNLOCK(rdch);
-       if (wrch != NULL)
-               CHN_UNLOCK(wrch);
        relchns(i_dev, rdch, wrch, 0);
        splx(s);
        return ret;

_______________________________________________
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to