panic upon kldunload snd_ich (lor # 159)
Ariff Abdullah
skywizard at MyBSD.org.my
Wed Sep 14 20:47:16 PDT 2005
On Thu, 15 Sep 2005 10:45:09 +0900
Pyun YongHyeon <pyunyh at gmail.com> wrote:
>
> That would be supposed to fix the LOR message. But I'd like to keep
> lock ordering between snd_mutex and sndstat_lock. Since sndstat_read()
> could be called at any time there is an implicit lock order.
> I think switching to sx lock from mutex in sndstat code was to allow
> uiomove with lock held. IMO, it would be even better to rewrite
> sndstat_read() without using uiomove such that it can also use
> standard mutex rather than sx lock.
>
I tend to agree with you. Since that sndstat_busy() isn't enough, how
about we acquire the entire sndstat so nobody can monkey with it (as the
proposed / attached diff) and at the same time avoiding this LOR
message. It seems much better rather than locking sndstat after
sndstat_busy() and much of pcm_unregister() procedures, only to find out
that somebody acquire it within that moment.
--
Ariff Abdullah
MyBSD
http://www.MyBSD.org.my (IPv6/IPv4)
http://staff.MyBSD.org.my (IPv6/IPv4)
http://tomoyo.MyBSD.org.my (IPv6/IPv4)
-------------- next part --------------
--- sys/dev/sound/pcm/sound.h.orig Thu Sep 15 11:25:29 2005
+++ sys/dev/sound/pcm/sound.h Thu Sep 15 11:31:19 2005
@@ -238,11 +238,12 @@
int sysctl_hw_snd_vchans(SYSCTL_HANDLER_ARGS);
typedef int (*sndstat_handler)(struct sbuf *s, device_t dev, int verbose);
+int sndstat_acquire(void);
+int sndstat_release(void);
int sndstat_register(device_t dev, char *str, sndstat_handler handler);
int sndstat_registerfile(char *str);
int sndstat_unregister(device_t dev);
int sndstat_unregisterfile(char *str);
-int sndstat_busy(void);
#define SND_DECLARE_FILE(version) \
_SND_DECLARE_FILE(__LINE__, version)
--- sys/dev/sound/pcm/sndstat.c.orig Thu Sep 15 11:25:01 2005
+++ sys/dev/sound/pcm/sndstat.c Thu Sep 15 11:31:07 2005
@@ -195,6 +195,42 @@
}
int
+sndstat_acquire(void)
+{
+ intrmask_t s;
+
+ s = spltty();
+ sx_xlock(&sndstat_lock);
+ if (sndstat_isopen) {
+ sx_unlock(&sndstat_lock);
+ splx(s);
+ return EBUSY;
+ }
+ sndstat_isopen = 1;
+ sx_unlock(&sndstat_lock);
+ splx(s);
+ return 0;
+}
+
+int
+sndstat_release(void)
+{
+ intrmask_t s;
+
+ s = spltty();
+ sx_xlock(&sndstat_lock);
+ if (!sndstat_isopen) {
+ sx_unlock(&sndstat_lock);
+ splx(s);
+ return EBADF;
+ }
+ sndstat_isopen = 0;
+ sx_unlock(&sndstat_lock);
+ splx(s);
+ return 0;
+}
+
+int
sndstat_register(device_t dev, char *str, sndstat_handler handler)
{
intrmask_t s;
@@ -371,12 +407,6 @@
sx_xunlock(&sndstat_lock);
sx_destroy(&sndstat_lock);
return 0;
-}
-
-int
-sndstat_busy(void)
-{
- return (sndstat_isopen);
}
static void
--- sys/dev/sound/pcm/sound.c.orig Thu Sep 15 11:25:05 2005
+++ sys/dev/sound/pcm/sound.c Thu Sep 15 11:31:07 2005
@@ -758,24 +758,25 @@
struct snddev_channel *sce;
struct pcm_channel *ch;
+ if (sndstat_acquire() != 0) {
+ device_printf(dev, "unregister: sndstat busy\n");
+ return EBUSY;
+ }
+
snd_mtxlock(d->lock);
if (d->inprog) {
device_printf(dev, "unregister: operation in progress\n");
snd_mtxunlock(d->lock);
+ sndstat_release();
return EBUSY;
}
- if (sndstat_busy() != 0) {
- device_printf(dev, "unregister: sndstat busy\n");
- snd_mtxunlock(d->lock);
- return EBUSY;
- }
-
SLIST_FOREACH(sce, &d->channels, link) {
ch = sce->channel;
if (ch->refcount > 0) {
device_printf(dev, "unregister: channel %s busy (pid %d)\n", ch->name, ch->pid);
snd_mtxunlock(d->lock);
+ sndstat_release();
return EBUSY;
}
}
@@ -783,6 +784,7 @@
if (mixer_uninit(dev)) {
device_printf(dev, "unregister: mixer busy\n");
snd_mtxunlock(d->lock);
+ sndstat_release();
return EBUSY;
}
@@ -807,9 +809,10 @@
chn_kill(d->fakechan);
fkchan_kill(d->fakechan);
- sndstat_unregister(dev);
snd_mtxunlock(d->lock);
snd_mtxfree(d->lock);
+ sndstat_unregister(dev);
+ sndstat_release();
return 0;
}
More information about the freebsd-current
mailing list