From owner-p4-projects@FreeBSD.ORG Fri Jul 11 06:21:55 2008 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 8EC1A1065676; Fri, 11 Jul 2008 06:21:55 +0000 (UTC) Delivered-To: perforce@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 51D1B106564A; Fri, 11 Jul 2008 06:21:55 +0000 (UTC) (envelope-from alexander@leidinger.net) Received: from redbull.bpaserver.net (redbullneu.bpaserver.net [213.198.78.217]) by mx1.freebsd.org (Postfix) with ESMTP id 9A6168FC2B; Fri, 11 Jul 2008 06:21:54 +0000 (UTC) (envelope-from alexander@leidinger.net) Received: from outgoing.leidinger.net (p54A56336.dip.t-dialin.net [84.165.99.54]) by redbull.bpaserver.net (Postfix) with ESMTP id CE13C2E0F8; Fri, 11 Jul 2008 08:05:26 +0200 (CEST) Received: from webmail.leidinger.net (webmail.leidinger.net [192.168.1.102]) by outgoing.leidinger.net (Postfix) with ESMTP id 17413133926; Fri, 11 Jul 2008 08:05:15 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=Leidinger.net; s=outgoing-alex; t=1215756315; bh=Pt8N2/sAM6Dc/BXK5easB+AIi+LKTxfYD CK5j9RYWbo=; h=Message-ID:Date:From:To:Cc:Subject:References: In-Reply-To:MIME-Version:Content-Type:Content-Transfer-Encoding; b=UawYkWANcjXgy6HfhOFOQSd/IAks8Yp4dM4HJfN3YcoLL0UryPYhGZn1QxywbRKTR OYYC9hv0F8JNvM48kGS5icsaObxDEUIygRk1++eiQcMM66CuSoebPGitL00Q56v5l9e llTDIUebg3Fcfn5RFjDfJ/3/g3RIqMEzmT2kQTHLv3CQiUn+nJ16lDS4fG2WCV6UCOY 2mh/pzdWRO1qfMhU0Zbz1SDYmqqYy+Dhzj3awFiDfjybJUwjWx2BtRr0JW2jt8l68Vb LOOAi45xe8dq8hdahKLvtozCu72qtXsefS3p9NZCC30FDJrvrBSlOky6QmAu12B+oqq 4Gh3+KNiw== Received: (from www@localhost) by webmail.leidinger.net (8.14.2/8.13.8/Submit) id m6B65E8g055683; Fri, 11 Jul 2008 08:05:14 +0200 (CEST) (envelope-from Alexander@Leidinger.net) Received: from pslux.cec.eu.int (pslux.cec.eu.int [158.169.9.14]) by webmail.leidinger.net (Horde Framework) with HTTP; Fri, 11 Jul 2008 08:05:14 +0200 Message-ID: <20080711080514.92125k684wobbhgk@webmail.leidinger.net> X-Priority: 3 (Normal) Date: Fri, 11 Jul 2008 08:05:14 +0200 From: Alexander Leidinger To: Hans Petter Selasky References: <200807100757.m6A7vbex060400@repoman.freebsd.org> In-Reply-To: <200807100757.m6A7vbex060400@repoman.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; DelSp="Yes"; format="flowed" Content-Disposition: inline Content-Transfer-Encoding: quoted-printable User-Agent: Internet Messaging Program (IMP) H3 (4.2-RC2) / FreeBSD-8.0 X-BPAnet-MailScanner-Information: Please contact the ISP for more information X-BPAnet-MailScanner: Found to be clean X-BPAnet-MailScanner-SpamCheck: not spam, ORDB-RBL, SpamAssassin (not cached, score=-13.427, required 6, BAYES_00 -15.00, DKIM_SIGNED 0.00, DKIM_VERIFIED -0.00, MIME_QP_LONG_LINE 1.40, RDNS_DYNAMIC 0.10, TW_SN 0.08) X-BPAnet-MailScanner-From: alexander@leidinger.net X-Spam-Status: No Cc: Perforce Change Reviews , ariff@FreeBSD.org Subject: Re: PERFORCE change 144991 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 11 Jul 2008 06:21:55 -0000 Quoting Hans Petter Selasky (from Thu, 10 Jul =20 2008 07:57:37 GMT): > http://perforce.freebsd.org/chv.cgi?CH=3D144991 > > Change 144991 by hselasky@hselasky_laptop001 on 2008/07/10 07:56:37 > > > =09These patches into the USB sound system are needed to make > =09the Giant-free USB audio driver work seamlessly. If the mixer uninit can sleep, there's the possibility for a race, =20 isn't it? How do you make sure there's no race after not locking the =20 mixer? What about a different approach: - keep the generic sound code as it is (locking of the mixer) - add some uninit flag to the mixer private data - check the flag + if it is not set on uninit, set it + if it is set on uninit (and maybe other mixer operations), abort - unlock the mutex in the uninit - do what can sleep - lock the mutex - return Bye, Alexander. > Affected files ... > > .. //depot/projects/usb/src/sys/dev/sound/pcm/mixer.c#10 edit > .. //depot/projects/usb/src/sys/dev/sound/pcm/mixer.h#9 edit > .. //depot/projects/usb/src/sys/dev/usb2/sound/uaudio2.c#6 edit > .. //depot/projects/usb/src/sys/dev/usb2/sound/uaudio2_pcm.c#3 edit > > Differences ... > > =3D=3D=3D=3D //depot/projects/usb/src/sys/dev/sound/pcm/mixer.c#10 (text+k= o) =3D=3D=3D=3D > > @@ -589,7 +589,7 @@ > =09KASSERT(m->type =3D=3D MIXER_TYPE_SECONDARY, > =09 ("%s(): illegal mixer type=3D%d", __func__, m->type)); > > -=09snd_mtxlock(m->lock); > +=09/* mixer uninit can sleep --hps */ > > =09MIXER_UNINIT(m); > > @@ -712,6 +712,10 @@ > > =09mixer_setrecsrc(m, SOUND_MASK_MIC); > > +=09snd_mtxunlock(m->lock); > + > +=09/* mixer uninit can sleep --hps */ > + > =09MIXER_UNINIT(m); > > =09snd_mtxfree(m->lock); > @@ -1280,3 +1284,16 @@ > > =09return (EINVAL); > } > + > +/* > + * Allow the sound driver to use the mixer lock to protect its mixer > + * data: > + */ > +struct mtx * > +mixer_get_lock(struct snd_mixer *m) > +{ > +=09if (m->lock =3D=3D NULL) { > +=09=09return (&Giant); > +=09} > +=09return (m->lock); > +} > > =3D=3D=3D=3D //depot/projects/usb/src/sys/dev/sound/pcm/mixer.h#9 (text+ko= ) =3D=3D=3D=3D > > @@ -56,6 +56,7 @@ > u_int32_t mix_getparent(struct snd_mixer *m, u_int32_t dev); > u_int32_t mix_getchild(struct snd_mixer *m, u_int32_t dev); > void *mix_getdevinfo(struct snd_mixer *m); > +struct mtx *mixer_get_lock(struct snd_mixer *m); > > extern int mixer_count; > > > =3D=3D=3D=3D //depot/projects/usb/src/sys/dev/usb2/sound/uaudio2.c#6 (text= +ko) =3D=3D=3D=3D > > @@ -220,7 +220,6 @@ > > =09struct usb2_device *sc_udev; > =09struct usb2_xfer *sc_mixer_xfer[1]; > -=09struct mtx *sc_mixer_lock; > =09struct uaudio_mixer_node *sc_mixer_root; > =09struct uaudio_mixer_node *sc_mixer_curr; > > @@ -3091,12 +3090,11 @@ > { > =09DPRINTF(0, "\n"); > > -=09sc->sc_mixer_lock =3D mixer_get_lock(m); > - > =09if (usb2_transfer_setup(sc->sc_udev, &(sc->sc_mixer_iface_index), > =09 sc->sc_mixer_xfer, uaudio_mixer_config, 1, sc, > -=09 sc->sc_mixer_lock)) { > -=09=09DPRINTF(0, "could not allocate USB transfer for audio mixer!\n"); > +=09 mixer_get_lock(m))) { > +=09=09DPRINTF(-1, "could not allocate USB " > +=09=09 "transfer for audio mixer!\n"); > =09=09return (ENOMEM); > =09} > =09if (!(sc->sc_mix_info & SOUND_MASK_VOLUME)) { > > =3D=3D=3D=3D //depot/projects/usb/src/sys/dev/usb2/sound/uaudio2_pcm.c#3 = =20 > (text+ko) =3D=3D=3D=3D > > @@ -127,10 +127,18 @@ > ua_mixer_set(struct snd_mixer *m, unsigned type, unsigned left, =20 > unsigned right) > { > =09struct mtx *mtx =3D mixer_get_lock(m); > +=09uint8_t do_unlock; > > -=09snd_mtxlock(mtx);=09=09/* XXX */ > +=09if (mtx_owned(mtx)) { > +=09=09do_unlock =3D 0; > +=09} else { > +=09=09do_unlock =3D 1; > +=09=09mtx_lock(mtx); > +=09} > =09uaudio_mixer_set(mix_getdevinfo(m), type, left, right); > -=09snd_mtxunlock(mtx);=09=09/* XXX */ > +=09if (do_unlock) { > +=09=09mtx_unlock(mtx); > +=09} > =09return (left | (right << 8)); > } > > @@ -139,10 +147,18 @@ > { > =09struct mtx *mtx =3D mixer_get_lock(m); > =09int retval; > +=09uint8_t do_unlock; > > -=09snd_mtxlock(mtx);=09=09/* XXX */ > +=09if (mtx_owned(mtx)) { > +=09=09do_unlock =3D 0; > +=09} else { > +=09=09do_unlock =3D 1; > +=09=09mtx_lock(mtx); > +=09} > =09retval =3D uaudio_mixer_setrecsrc(mix_getdevinfo(m), src); > -=09snd_mtxunlock(mtx);=09=09/* XXX */ > +=09if (do_unlock) { > +=09=09mtx_unlock(mtx); > +=09} > =09return (retval); > } > > > --=20 CREDITOR: =09A man who has a better memory than a debtor. http://www.Leidinger.net Alexander @ Leidinger.net: PGP ID =3D B0063FE7 http://www.FreeBSD.org netchild @ FreeBSD.org : PGP ID =3D 72077137