Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 7 Aug 2012 17:03:13 +0200
From:      Hans Petter Selasky <hselasky@c2i.net>
To:        freebsd-current@freebsd.org
Cc:        Konstantin Belousov <kostikbel@gmail.com>, Ed Schouten <ed@80386.nl>
Subject:   Re: ttydev_cdevsw has no d_purge
Message-ID:  <201208071703.13773.hselasky@c2i.net>
In-Reply-To: <201208051033.13486.hselasky@c2i.net>
References:  <20120801160323.GN2676@deviant.kiev.zoral.com.ua> <CAJOYFBD6Xa08LXuzzYha7Ev9WDJFN5%2BHEqDJ6a2C0puFGZ4-Qg@mail.gmail.com> <201208051033.13486.hselasky@c2i.net>

next in thread | previous in thread | raw e-mail | index | archive | help
--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 <hselasky@c2i.net>:
> > > 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--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201208071703.13773.hselasky>