From owner-freebsd-current@FreeBSD.ORG Tue Aug 7 15:02:53 2012 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id F099C106564A for ; Tue, 7 Aug 2012 15:02:52 +0000 (UTC) (envelope-from hselasky@c2i.net) Received: from swip.net (mailfe08.c2i.net [212.247.154.226]) by mx1.freebsd.org (Postfix) with ESMTP id 60EA38FC08 for ; Tue, 7 Aug 2012 15:02:51 +0000 (UTC) X-T2-Spam-Status: No, hits=-0.2 required=5.0 tests=ALL_TRUSTED, BAYES_50 Received: from [176.74.212.201] (account mc467741@c2i.net HELO laptop015.hselasky.homeunix.org) by mailfe08.swip.net (CommuniGate Pro SMTP 5.4.4) with ESMTPA id 306283327; Tue, 07 Aug 2012 17:02:44 +0200 From: Hans Petter Selasky To: freebsd-current@freebsd.org Date: Tue, 7 Aug 2012 17:03:13 +0200 User-Agent: KMail/1.13.7 (FreeBSD/9.0-STABLE; KDE/4.7.4; amd64; ; ) References: <20120801160323.GN2676@deviant.kiev.zoral.com.ua> <201208051033.13486.hselasky@c2i.net> In-Reply-To: <201208051033.13486.hselasky@c2i.net> X-Face: 'mmZ:T{)),Oru^0c+/}w'`gU1$ubmG?lp!=R4Wy\ELYo2)@'UZ24N@d2+AyewRX}mAm; Yp |U[@, _z/([?1bCfM{_"B<.J>mICJCHAzzGHI{y7{%JVz%R~yJHIji`y>Y}k1C4TfysrsUI -%GU9V5]iUZF&nRn9mJ'?&>O MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_x4SIQn2gx+2/oq8" Message-Id: <201208071703.13773.hselasky@c2i.net> Cc: Konstantin Belousov , Ed Schouten Subject: Re: ttydev_cdevsw has no d_purge 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: Tue, 07 Aug 2012 15:02:53 -0000 --Boundary-00=_x4SIQn2gx+2/oq8 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 7bit On Sunday 05 August 2012 10:33:13 Hans Petter Selasky wrote: > On Friday 03 August 2012 10:32:47 Ed Schouten wrote: > > 2012/8/1 Hans Petter Selasky : > > > I think the problem is like this, that in order to re-use the unit > > > numbers for USB serial tty devices, the USB stack needs to wait until a > > > TTY is actually freed, right? Else you will have a panic on creating > > > devfs entries having the same name. > > > > Indeed. So the USB code could simply pick a different unit number. > > Hi Ed, > > USB could use a different Unit number. Some questions: > > When can the previous unit number be re-used? Is there a callback for this? > > When can the USB serial code assume that it will not be called again and > that all callbacks are drained? > Hi, Can you try the attached patch and report back? --HPS --Boundary-00=_x4SIQn2gx+2/oq8 Content-Type: text/x-patch; charset="utf-8"; name="ucom_unit_number.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="ucom_unit_number.patch" === usb_serial.c ================================================================== --- usb_serial.c (revision 239054) +++ usb_serial.c (local) @@ -179,8 +179,11 @@ MODULE_VERSION(ucom, 1); #define UCOM_UNIT_MAX 128 /* limits size of ucom_bitmap */ +#define UCOM_UNIT_MAX_BYTES ((UCOM_UNIT_MAX + 7) / 8) -static uint8_t ucom_bitmap[(UCOM_UNIT_MAX + 7) / 8]; +static uint8_t ucom_bitmap[UCOM_UNIT_MAX_BYTES]; +static uint8_t ucom_closing[UCOM_UNIT_MAX_BYTES]; +static uint32_t ucom_closing_ref; static struct mtx ucom_bitmap_mtx; MTX_SYSINIT(ucom_bitmap_mtx, &ucom_bitmap_mtx, "ucom bitmap", MTX_DEF); @@ -215,16 +218,34 @@ } /* + * Sync the "ucom_bitmap" and the "ucom_closing" one. + */ +static void +ucom_unit_sync(void) +{ + uint32_t x; + + mtx_lock(&ucom_bitmap_mtx); + if (ucom_closing_ref == 0) { + for (x = 0; x != UCOM_UNIT_MAX_BYTES; x++) { + ucom_bitmap[x] &= ~ucom_closing[x]; + ucom_closing[x] = 0; + } + } + mtx_unlock(&ucom_bitmap_mtx); +} + +/* * Mark the unit number as not in use. */ static void ucom_unit_free(int unit) { mtx_lock(&ucom_bitmap_mtx); + ucom_closing[unit / 8] |= (1 << (unit % 8)); + mtx_unlock(&ucom_bitmap_mtx); - ucom_bitmap[unit / 8] &= ~(1 << (unit % 8)); - - mtx_unlock(&ucom_bitmap_mtx); + ucom_unit_sync(); } /* @@ -356,7 +377,6 @@ sc->sc_tty = tp; DPRINTF("ttycreate: %s\n", buf); - cv_init(&sc->sc_cv, "ucom"); /* Check if this device should be a console */ if ((ucom_cons_softc == NULL) && @@ -408,7 +428,13 @@ sc->sc_flag |= UCOM_FLAG_GONE; sc->sc_flag &= ~(UCOM_FLAG_HL_READY | UCOM_FLAG_LL_READY); mtx_unlock(sc->sc_mtx); + if (tp) { + + mtx_lock(&ucom_bitmap_mtx); + ucom_closing_ref++; + mtx_unlock(&ucom_bitmap_mtx); + tty_lock(tp); ucom_close(tp); /* close, if any */ @@ -416,9 +442,6 @@ tty_rel_gone(tp); mtx_lock(sc->sc_mtx); - /* Wait for the callback after the TTY is torn down */ - while (sc->sc_ttyfreed == 0) - cv_wait(&sc->sc_cv, sc->sc_mtx); /* * make sure that read and write transfers are stopped */ @@ -430,7 +453,6 @@ } mtx_unlock(sc->sc_mtx); } - cv_destroy(&sc->sc_cv); } void @@ -1323,12 +1345,11 @@ static void ucom_free(void *xsc) { - struct ucom_softc *sc = xsc; + mtx_lock(&ucom_bitmap_mtx); + ucom_closing_ref--; + mtx_unlock(&ucom_bitmap_mtx); - mtx_lock(sc->sc_mtx); - sc->sc_ttyfreed = 1; - cv_signal(&sc->sc_cv); - mtx_unlock(sc->sc_mtx); + ucom_unit_sync(); } static cn_probe_t ucom_cnprobe; === usb_serial.h ================================================================== --- usb_serial.h (revision 239054) +++ usb_serial.h (local) @@ -158,7 +158,6 @@ struct ucom_cfg_task sc_line_state_task[2]; struct ucom_cfg_task sc_status_task[2]; struct ucom_param_task sc_param_task[2]; - struct cv sc_cv; /* Used to set "UCOM_FLAG_GP_DATA" flag: */ struct usb_proc_msg *sc_last_start_xfer; const struct ucom_callback *sc_callback; @@ -179,7 +178,6 @@ uint8_t sc_lsr; uint8_t sc_msr; uint8_t sc_mcr; - uint8_t sc_ttyfreed; /* set when TTY has been freed */ /* programmed line state bits */ uint8_t sc_pls_set; /* set bits */ uint8_t sc_pls_clr; /* cleared bits */ --Boundary-00=_x4SIQn2gx+2/oq8--