From owner-freebsd-current@FreeBSD.ORG Thu Sep 15 03:47:16 2005 Return-Path: X-Original-To: freebsd-current@freebsd.org Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id BE00C16A41F for ; Thu, 15 Sep 2005 03:47:16 +0000 (GMT) (envelope-from skywizard@MyBSD.org.my) Received: from tomoyo.MyBSD.org.my (tomoyo.mybsd.org.my [202.157.186.227]) by mx1.FreeBSD.org (Postfix) with ESMTP id EF57C43D45 for ; Thu, 15 Sep 2005 03:47:11 +0000 (GMT) (envelope-from skywizard@MyBSD.org.my) Received: from localhost (localhost [127.0.0.1]) by tomoyo.MyBSD.org.my (Postfix) with ESMTP id 2B1366CC2D; Thu, 15 Sep 2005 11:55:12 +0800 (MYT) Received: from tomoyo.MyBSD.org.my ([127.0.0.1]) by localhost (tomoyo.MyBSD.org.my [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 75120-09; Thu, 15 Sep 2005 11:55:10 +0800 (MYT) Received: from kasumi.MyBSD.org.my (unknown [218.111.181.140]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by tomoyo.MyBSD.org.my (Postfix) with ESMTP id 3E2BC6CC3D; Thu, 15 Sep 2005 11:55:09 +0800 (MYT) Date: Thu, 15 Sep 2005 11:46:53 +0800 From: Ariff Abdullah To: pyunyh@gmail.com, minimarmot@gmail.com Message-Id: <20050915114653.431f17c5.skywizard@MyBSD.org.my> In-Reply-To: <20050915014509.GA17602@rndsoft.co.kr> References: <47d0403c05091121047a037946@mail.gmail.com> <20050912044212.GC5182@rndsoft.co.kr> <47d0403c05091122276fd0a231@mail.gmail.com> <20050913070149.GE9481@rndsoft.co.kr> <47d0403c0509131235ed58122@mail.gmail.com> <20050914014830.GA13631@rndsoft.co.kr> <20050915033848.2d87da42.skywizard@MyBSD.org.my> <20050915014509.GA17602@rndsoft.co.kr> Organization: MyBSD X-Mailer: /usr/local/lib/ruby/1.8/net/smtp.rb Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="Multipart=_Thu__15_Sep_2005_11_46_53_+0800_M9_q1d0zk/zgEu9E" X-Virus-Scanned: by amavisd-new-antivirus-mail-gateway at TOMOYO.MYBSD.ORG.MY Cc: freebsd-current@freebsd.org Subject: Re: panic upon kldunload snd_ich (lor # 159) X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 15 Sep 2005 03:47:16 -0000 This is a multi-part message in MIME format. --Multipart=_Thu__15_Sep_2005_11_46_53_+0800_M9_q1d0zk/zgEu9E Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Thu, 15 Sep 2005 10:45:09 +0900 Pyun YongHyeon 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) --Multipart=_Thu__15_Sep_2005_11_46_53_+0800_M9_q1d0zk/zgEu9E Content-Type: text/plain; name="nuke_sndstat_lor.diff" Content-Disposition: attachment; filename="nuke_sndstat_lor.diff" Content-Transfer-Encoding: 7bit --- 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; } --Multipart=_Thu__15_Sep_2005_11_46_53_+0800_M9_q1d0zk/zgEu9E--