From owner-freebsd-current@FreeBSD.ORG Thu Nov 27 10:29:28 2003 Return-Path: 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 F3A0D16A4CF for ; Thu, 27 Nov 2003 10:29:27 -0800 (PST) Received: from hak.cnd.mcgill.ca (hak.cnd.mcgill.ca [132.216.11.133]) by mx1.FreeBSD.org (Postfix) with ESMTP id 91B5D43FE5 for ; Thu, 27 Nov 2003 10:29:21 -0800 (PST) (envelope-from mat@hak.cnd.mcgill.ca) Received: from hak.cnd.mcgill.ca (localhost [127.0.0.1]) by hak.cnd.mcgill.ca (8.12.9/8.12.8) with ESMTP id hARIQi15025339 for ; Thu, 27 Nov 2003 13:26:44 -0500 (EST) (envelope-from mat@hak.cnd.mcgill.ca) Received: (from mat@localhost) by hak.cnd.mcgill.ca (8.12.9/8.12.8/Submit) id hARIQiEj025338 for freebsd-current@freebsd.org; Thu, 27 Nov 2003 13:26:44 -0500 (EST) Date: Thu, 27 Nov 2003 13:26:44 -0500 From: Mathew Kanner To: freebsd-current@freebsd.org Message-ID: <20031127182644.GK18555@cnd.mcgill.ca> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="/2994txjAzEdQwm5" Content-Disposition: inline User-Agent: Mutt/1.4.1i Organization: I speak for myself, operating in Montreal, CANADA X-Spam-Status: No, hits=0.0 required=5.0 tests=none autolearn=no version=2.60 X-Spam-Checker-Version: SpamAssassin 2.60 (1.212-2003-09-23-exp) on hak.cnd.mcgill.ca Subject: please test sndstat lor patch X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 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, 27 Nov 2003 18:29:28 -0000 --/2994txjAzEdQwm5 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hello Gang, I wrote this to cleanup /dev/sndstat a little and fix a LOR. Then I couldn't remember if there is a LOR or not. Could someone please remind me? If we do, could someone please test this patch. Thanks, --Mat -- The beaver, which has come to represent Canada as the eagle does the United States and the lion Britain, is a flat-tailed, slow-witted, toothy rodent known to bite off it's own testicles or to stand under its own falling trees. - June Callwood --/2994txjAzEdQwm5 Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="sndstat_lor.patch" Index: sndstat.c =================================================================== RCS file: /home/ncvs/src/sys/dev/sound/pcm/sndstat.c,v retrieving revision 1.14 diff -u -r1.14 sndstat.c --- sndstat.c 7 Sep 2003 16:28:03 -0000 1.14 +++ sndstat.c 27 Nov 2003 18:21:06 -0000 @@ -56,44 +56,41 @@ int type, unit; }; -#ifdef USING_MUTEX -static struct mtx sndstat_lock; -#endif -static struct sbuf sndstat_sbuf; static dev_t sndstat_dev = 0; static int sndstat_isopen = 0; -static int sndstat_bufptr; + +struct sndstat { + void *rawbuf; + struct sbuf sbuf; + int bufptr; + int isopen; +}; + +static struct mtx sndstat_lock; static int sndstat_maxunit = -1; static int sndstat_files = 0; static SLIST_HEAD(, sndstat_entry) sndstat_devlist = SLIST_HEAD_INITIALIZER(none); static int sndstat_verbose = 1; -#ifdef USING_MUTEX TUNABLE_INT("hw.snd.verbose", &sndstat_verbose); -#else -TUNABLE_INT_DECL("hw.snd.verbose", 1, sndstat_verbose); -#endif static int sndstat_prepare(struct sbuf *s); static int sysctl_hw_sndverbose(SYSCTL_HANDLER_ARGS) { - intrmask_t s; int error, verbose; verbose = sndstat_verbose; error = sysctl_handle_int(oidp, &verbose, sizeof(verbose), req); if (error == 0 && req->newptr != NULL) { - s = spltty(); mtx_lock(&sndstat_lock); if (verbose < 0 || verbose > 3) error = EINVAL; else sndstat_verbose = verbose; mtx_unlock(&sndstat_lock); - splx(s); } return error; } @@ -103,32 +100,26 @@ static int sndstat_open(dev_t i_dev, int flags, int mode, struct thread *td) { - intrmask_t s; + struct sndstat *s = i_dev->si_drv1; int error; - s = spltty(); + if (s == NULL) + return ENXIO; + mtx_lock(&sndstat_lock); if (sndstat_isopen) { mtx_unlock(&sndstat_lock); - splx(s); return EBUSY; } - sndstat_isopen = 1; + sndstat_isopen = s->isopen = 1; mtx_unlock(&sndstat_lock); - splx(s); - if (sbuf_new(&sndstat_sbuf, NULL, 4096, 0) == NULL) { - error = ENXIO; - goto out; - } - sndstat_bufptr = 0; - error = (sndstat_prepare(&sndstat_sbuf) > 0) ? 0 : ENOMEM; -out: + sbuf_clear(&s->sbuf); + s->bufptr = 0; + error = (sndstat_prepare(&s->sbuf) > 0) ? 0 : ENOMEM; if (error) { - s = spltty(); mtx_lock(&sndstat_lock); - sndstat_isopen = 0; + sndstat_isopen = s->isopen = 0; mtx_unlock(&sndstat_lock); - splx(s); } return (error); } @@ -136,42 +127,40 @@ static int sndstat_close(dev_t i_dev, int flags, int mode, struct thread *td) { - intrmask_t s; + struct sndstat *s = i_dev->si_drv1; + if (s == NULL) + return ENXIO; - s = spltty(); mtx_lock(&sndstat_lock); if (!sndstat_isopen) { mtx_unlock(&sndstat_lock); - splx(s); return EBADF; } - sbuf_delete(&sndstat_sbuf); - sndstat_isopen = 0; + sndstat_isopen = s->isopen = 0; mtx_unlock(&sndstat_lock); - splx(s); return 0; } static int sndstat_read(dev_t i_dev, struct uio *buf, int flag) { - intrmask_t s; + struct sndstat *s = i_dev->si_drv1; int l, err; - s = spltty(); + if (s == NULL) + return ENXIO; + mtx_lock(&sndstat_lock); if (!sndstat_isopen) { mtx_unlock(&sndstat_lock); - splx(s); return EBADF; } - l = min(buf->uio_resid, sbuf_len(&sndstat_sbuf) - sndstat_bufptr); - err = (l > 0)? uiomove(sbuf_data(&sndstat_sbuf) + sndstat_bufptr, l, buf) : 0; - sndstat_bufptr += l; + l = min(buf->uio_resid, sbuf_len(&s->sbuf) - s->bufptr); + err = (l > 0)? uiomove(sbuf_data(&s->sbuf) + s->bufptr, l, buf) : 0; + s->bufptr += l; mtx_unlock(&sndstat_lock); - splx(s); return err; } @@ -193,7 +182,6 @@ int sndstat_register(device_t dev, char *str, sndstat_handler handler) { - intrmask_t s; struct sndstat_entry *ent; const char *devtype; int type, unit; @@ -214,7 +202,7 @@ unit = -1; } - ent = malloc(sizeof *ent, M_DEVBUF, M_ZERO | M_WAITOK); + ent = malloc(sizeof *ent, M_DEVBUF, M_ZERO | M_NOWAIT); if (!ent) return ENOSPC; @@ -224,14 +212,12 @@ ent->unit = unit; ent->handler = handler; - s = spltty(); mtx_lock(&sndstat_lock); SLIST_INSERT_HEAD(&sndstat_devlist, ent, link); if (type == SS_TYPE_MODULE) sndstat_files++; sndstat_maxunit = (unit > sndstat_maxunit)? unit : sndstat_maxunit; mtx_unlock(&sndstat_lock); - splx(s); return 0; } @@ -245,23 +231,19 @@ int sndstat_unregister(device_t dev) { - intrmask_t s; struct sndstat_entry *ent; - s = spltty(); mtx_lock(&sndstat_lock); SLIST_FOREACH(ent, &sndstat_devlist, link) { if (ent->dev == dev) { SLIST_REMOVE(&sndstat_devlist, ent, sndstat_entry, link); mtx_unlock(&sndstat_lock); - splx(s); free(ent, M_DEVBUF); return 0; } } mtx_unlock(&sndstat_lock); - splx(s); return ENXIO; } @@ -269,24 +251,20 @@ int sndstat_unregisterfile(char *str) { - intrmask_t s; struct sndstat_entry *ent; - s = spltty(); mtx_lock(&sndstat_lock); SLIST_FOREACH(ent, &sndstat_devlist, link) { if (ent->dev == NULL && ent->str == str) { SLIST_REMOVE(&sndstat_devlist, ent, sndstat_entry, link); sndstat_files--; mtx_unlock(&sndstat_lock); - splx(s); free(ent, M_DEVBUF); return 0; } } mtx_unlock(&sndstat_lock); - splx(s); return ENXIO; } @@ -338,32 +316,83 @@ } static int -sndstat_init(void) -{ +sndstat_init(void) { + struct sndstat *s; + int ret; + mtx_init(&sndstat_lock, "sndstat", NULL, 0); - sndstat_dev = make_dev(&sndstat_cdevsw, SND_DEV_STATUS, UID_ROOT, GID_WHEEL, 0444, "sndstat"); - return (sndstat_dev != 0)? 0 : ENXIO; + /* + * Except for sndstat_lock, this is a generic device creation + * suitable for multiple devices. I don't know why you would + * want that though. + */ + ret = ENOMEM; + + + s = malloc(sizeof(s), M_DEVBUF, M_ZERO|M_NOWAIT); + if (s == NULL) + goto err; + + s->rawbuf = malloc(4096, M_TEMP, M_NOWAIT); + if (s->rawbuf == NULL) + goto err; + + sbuf_new(&s->sbuf, s->rawbuf, 4096, SBUF_FIXEDLEN); + + sndstat_dev = make_dev(&sndstat_cdevsw, SND_DEV_STATUS, UID_ROOT, GID_WHEEL, 0444, "sndstat"); + if (sndstat_dev == NULL) { + return ENXIO; + goto err; + } + sndstat_dev->si_drv1 = s; + + /* + * Device created OK + */ + ret = 0; + goto ok; + +err: if (s != NULL) { + if (s->rawbuf != NULL) { + free(s->rawbuf, M_TEMP); + sbuf_delete(&s->sbuf); + } + free(s, M_DEVBUF); + } +ok: + return ret; } static int -sndstat_uninit(void) -{ - intrmask_t s; +sndstat_uninit(void) { + + struct sndstat *s = sndstat_dev->si_drv1; + + if (s == NULL) + return ENXIO; + + /* + * XXX: I'm puzzled at what the correct behaviour should be since + * uninit might fail, do we destroy the lock anyway? Since init/unit + * is called from SYSINIT the return value is ignored and we can't + * veto. We aren't allowed to fail. + * Use old semantics until this is figured out. + */ - s = spltty(); mtx_lock(&sndstat_lock); if (sndstat_isopen) { mtx_unlock(&sndstat_lock); - splx(s); return EBUSY; } - if (sndstat_dev) - destroy_dev(sndstat_dev); + sbuf_delete(&s->sbuf); + free(s->rawbuf, M_TEMP); + free(s, M_DEVBUF); + + destroy_dev(sndstat_dev); sndstat_dev = 0; - splx(s); mtx_destroy(&sndstat_lock); return 0; } --/2994txjAzEdQwm5--