Date: Thu, 21 Aug 2008 09:05:57 +0200 From: Hans Petter Selasky <hselasky@c2i.net> To: Alfred Perlstein <alfred@freebsd.org> Cc: freebsd-multimedia@freebsd.org, Kris Kennaway <kris@freebsd.org>, freebsd-usb@freebsd.org Subject: Re: usb4bsd patch review [was Re: ...] Message-ID: <200808210905.59150.hselasky@c2i.net> In-Reply-To: <20080820211644.GD16977@elvis.mu.org> References: <20080818205914.GJ16977@elvis.mu.org> <48AC54B8.4040406@FreeBSD.org> <20080820211644.GD16977@elvis.mu.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday 20 August 2008, Alfred Perlstein wrote:
> [[ Reply to Hans at end of this ]]
> [[ Reply to Kris inline ]]
>
Hi,
I've CC'ed multimedia. We have the following diff to mixer.c. Can anyone tell
if it will have any negative consequences:
--- sys/dev/sound/pcm/mixer.c August 2008
+++ sys.new/dev/sound/pcm/mixer.c August 2008
@@ -589,7 +589,7 @@
KASSERT(m->type == MIXER_TYPE_SECONDARY,
("%s(): illegal mixer type=%d", __func__, m->type));
- snd_mtxlock(m->lock);
+ /* mixer uninit can sleep --hps */
MIXER_UNINIT(m);
@@ -704,14 +704,24 @@
return EBUSY;
}
+ /* destroy dev can sleep --hps */
+
+ snd_mtxunlock(m->lock);
+
pdev->si_drv1 = NULL;
destroy_dev(pdev);
+ snd_mtxlock(m->lock);
+
for (i = 0; i < SOUND_MIXER_NRDEVICES; i++)
mixer_set(m, i, 0);
mixer_setrecsrc(m, SOUND_MASK_MIC);
+ snd_mtxunlock(m->lock);
+
+ /* mixer uninit can sleep --hps */
+
MIXER_UNINIT(m);
snd_mtxfree(m->lock);
@@ -1280,3 +1290,16 @@
return (EINVAL);
}
> > >>P.S. There were a number of questions you didn't answer, can I assume
> > >>those will follow later?
> > >
> > >Could you please repeat the questions you did not get an answer to?
> >
> > I've repeated some of them already. e.g. in this mail you still didn't
> > answer the "is it safe to drop the locks" question which was asked
> > twice. Rather than me repeating them yet again, I'd suggest you go back
> > and take another close read over my emails and your responses, and reply
> > to the questions that are still not resolved.
>
> Hans, let's just leave sound the way it is. I will remove that
> part of the patch. It might be better to leave a bad locking
> in that generates a warning rather than obscure the locking issue
> by removing the warning without fully understanding the issues.
USB audio is the only sound device I know about that is regularly plugged in
and out, which is causing this code path to be executed, unless you load
unload a sound module, which I think is very rarely done.
The lock in question has never before been exposed to the underlying layers,
so the funtions that are called should not have any knowledge about it and
consequently not depend on it either.
Alfred: Just make sure that everything builds. We can fix up these issues
afterwards. If USB sound support is broken it is not that critical.
---HPS
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200808210905.59150.hselasky>