Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 9 Aug 2012 08:51:56 +0200
From:      Hans Petter Selasky <hselasky@c2i.net>
To:        Ed Schouten <ed@80386.nl>
Cc:        Konstantin Belousov <kostikbel@gmail.com>, freebsd-current@freebsd.org
Subject:   Re: ttydev_cdevsw has no d_purge
Message-ID:  <201208090851.56972.hselasky@c2i.net>
In-Reply-To: <CAJOYFBBPBva6KUvX7dxgDRD_Y3uH=OkWx6MCAh4pUWdGXCi6dg@mail.gmail.com>
References:  <20120801160323.GN2676@deviant.kiev.zoral.com.ua> <201208081941.17860.hselasky@c2i.net> <CAJOYFBBPBva6KUvX7dxgDRD_Y3uH=OkWx6MCAh4pUWdGXCi6dg@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
--Boundary-00=_M41IQdlR19hhEdx
Content-Type: Text/Plain;
  charset="utf-8"
Content-Transfer-Encoding: 7bit

Hi,

I've updated the ucom patch.

1) Use unrhdr. Suggested by ed.
2) Implement tty_set_softc() and use that when dereffing ucom unit numbers.

Please test and report back.

--HPS

--Boundary-00=_M41IQdlR19hhEdx
Content-Type: text/x-patch; charset="utf-8";
	name="ucom_unit_number_and_tty.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
	filename="ucom_unit_number_and_tty.patch"

=== sys/tty.h
==================================================================
--- sys/tty.h	(revision 239054)
+++ sys/tty.h	(local)
@@ -199,6 +199,7 @@
 #define	tty_opened(tp)		((tp)->t_flags & TF_OPENED)
 #define	tty_gone(tp)		((tp)->t_flags & TF_GONE)
 #define	tty_softc(tp)		((tp)->t_devswsoftc)
+#define	tty_set_softc(tp,sc)	do { (tp)->t_devswsoftc = (sc); } while (0)
 #define	tty_devname(tp)		devtoname((tp)->t_dev)
 
 /* Status line printing. */
=== dev/usb/serial/usb_serial.c
==================================================================
--- dev/usb/serial/usb_serial.c	(revision 239054)
+++ dev/usb/serial/usb_serial.c	(local)
@@ -146,7 +146,7 @@
 static int	ucom_unit_alloc(void);
 static void	ucom_unit_free(int);
 static int	ucom_attach_tty(struct ucom_super_softc *, struct ucom_softc *);
-static void	ucom_detach_tty(struct ucom_softc *);
+static void	ucom_detach_tty(struct ucom_super_softc *, struct ucom_softc *);
 static void	ucom_queue_command(struct ucom_softc *,
 		    usb_proc_callback_t *, struct termios *pt,
 		    struct usb_proc_msg *t0, struct usb_proc_msg *t1);
@@ -178,14 +178,33 @@
 MODULE_DEPEND(ucom, usb, 1, 1, 1);
 MODULE_VERSION(ucom, 1);
 
-#define	UCOM_UNIT_MAX 		128	/* limits size of ucom_bitmap */
+#define	UCOM_UNIT_MAX 		128	/* maximum number of units */
+#define	UCOM_TTY_PREFIX		"U"
 
-static uint8_t ucom_bitmap[(UCOM_UNIT_MAX + 7) / 8];
-static struct mtx ucom_bitmap_mtx;
-MTX_SYSINIT(ucom_bitmap_mtx, &ucom_bitmap_mtx, "ucom bitmap", MTX_DEF);
+static struct unrhdr *ucom_unrhdr;
 
-#define UCOM_TTY_PREFIX		"U"
+static void
+ucom_init(void *arg)
+{
+	DPRINTF("\n");
+	ucom_unrhdr = new_unrhdr(0, UCOM_UNIT_MAX - 1, NULL);
+}
+SYSINIT(ucom_init, SI_SUB_KLD, SI_ORDER_ANY, ucom_init, NULL);
 
+static void
+ucom_uninit(void *arg)
+{
+	struct unrhdr *hdr;
+	hdr = ucom_unrhdr;
+	ucom_unrhdr = NULL;
+
+	DPRINTF("\n");
+
+	if (hdr != NULL)
+		delete_unrhdr(hdr);
+}
+SYSUNINIT(ucom_uninit, SI_SUB_KLD, SI_ORDER_ANY, ucom_uninit, NULL);
+
 /*
  * Mark a unit number (the X in cuaUX) as in use.
  *
@@ -197,21 +216,14 @@
 {
 	int unit;
 
-	mtx_lock(&ucom_bitmap_mtx);
-
-	for (unit = 0; unit < UCOM_UNIT_MAX; unit++) {
-		if ((ucom_bitmap[unit / 8] & (1 << (unit % 8))) == 0) {
-			ucom_bitmap[unit / 8] |= (1 << (unit % 8));
-			break;
-		}
+	/* sanity checks */
+	if (ucom_unrhdr == NULL) {
+		DPRINTF("ucom_unrhdr is NULL\n");
+		return (-1);
 	}
-
-	mtx_unlock(&ucom_bitmap_mtx);
-
-	if (unit == UCOM_UNIT_MAX)
-		return -1;
-	else
-		return unit;
+	unit = alloc_unr(ucom_unrhdr);
+	DPRINTF("unit %d is allocated\n", unit);
+	return (unit);
 }
 
 /*
@@ -220,11 +232,13 @@
 static void
 ucom_unit_free(int unit)
 {
-	mtx_lock(&ucom_bitmap_mtx);
-
-	ucom_bitmap[unit / 8] &= ~(1 << (unit % 8));
-
-	mtx_unlock(&ucom_bitmap_mtx);
+	/* sanity checks */
+	if (unit < 0 || unit >= UCOM_UNIT_MAX || ucom_unrhdr == NULL) {
+		DPRINTF("cannot free unit number\n");
+		return;
+	}
+	DPRINTF("unit %d is freed\n", unit);
+	free_unr(ucom_unrhdr, unit);
 }
 
 /*
@@ -248,11 +262,21 @@
 		return (EINVAL);
 	}
 
+	ssc->sc_ref = malloc(sizeof(*ssc->sc_ref), M_USBDEV,
+	    M_WAITOK | M_ZERO);
+	if (ssc->sc_ref == NULL)
+		return (ENOMEM);
+
 	/* allocate a uniq unit number */
 	ssc->sc_unit = ucom_unit_alloc();
-	if (ssc->sc_unit == -1)
+	if (ssc->sc_unit == -1) {
+		free(ssc->sc_ref, M_USBDEV);
 		return (ENOMEM);
+	}
 
+	/* store unit number for later */
+	ssc->sc_ref->unit = ssc->sc_unit;
+
 	/* generate TTY name string */
 	snprintf(ssc->sc_ttyname, sizeof(ssc->sc_ttyname),
 	    UCOM_TTY_PREFIX "%d", ssc->sc_unit);
@@ -260,7 +284,7 @@
 	/* create USB request handling process */
 	error = usb_proc_create(&ssc->sc_tq, mtx, "ucom", USB_PRI_MED);
 	if (error) {
-		ucom_unit_free(ssc->sc_unit);
+		ucom_free(ssc->sc_ref);
 		return (error);
 	}
 	ssc->sc_subunits = subunits;
@@ -277,6 +301,10 @@
 			ucom_detach(ssc, &sc[0]);
 			return (error);
 		}
+		/* increment unit reference count */
+		ssc->sc_ref->refs++;
+
+		/* set subunit attached */
 		sc[subunit].sc_flag |= UCOM_FLAG_ATTACHED;
 	}
 
@@ -310,16 +338,19 @@
 
 	usb_proc_drain(&ssc->sc_tq);
 
+	/* if there are no TTYs we need to free the ref structure */
+	if (ssc->sc_ref->refs == 0)
+		ucom_free(ssc->sc_ref);
+
 	for (subunit = 0; subunit < ssc->sc_subunits; subunit++) {
 		if (sc[subunit].sc_flag & UCOM_FLAG_ATTACHED) {
 
-			ucom_detach_tty(&sc[subunit]);
+			ucom_detach_tty(ssc, &sc[subunit]);
 
 			/* avoid duplicate detach */
 			sc[subunit].sc_flag &= ~UCOM_FLAG_ATTACHED;
 		}
 	}
-	ucom_unit_free(ssc->sc_unit);
 	usb_proc_free(&ssc->sc_tq);
 }
 
@@ -356,7 +387,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) && 
@@ -388,7 +418,7 @@
 }
 
 static void
-ucom_detach_tty(struct ucom_softc *sc)
+ucom_detach_tty(struct ucom_super_softc *ssc, struct ucom_softc *sc)
 {
 	struct tty *tp = sc->sc_tty;
 
@@ -408,17 +438,17 @@
 	sc->sc_flag |= UCOM_FLAG_GONE;
 	sc->sc_flag &= ~(UCOM_FLAG_HL_READY | UCOM_FLAG_LL_READY);
 	mtx_unlock(sc->sc_mtx);
+
 	if (tp) {
 		tty_lock(tp);
 
 		ucom_close(tp);	/* close, if any */
 
+		tty_set_softc(tp, ssc->sc_ref);
+
 		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 +460,6 @@
 		}
 		mtx_unlock(sc->sc_mtx);
 	}
-	cv_destroy(&sc->sc_cv);
 }
 
 void
@@ -1323,12 +1352,14 @@
 static void
 ucom_free(void *xsc)
 {
-	struct ucom_softc *sc = xsc;
+	struct ucom_ref *sc_ref = (struct ucom_ref *)xsc;
 
-	mtx_lock(sc->sc_mtx);
-	sc->sc_ttyfreed = 1;
-	cv_signal(&sc->sc_cv);
-	mtx_unlock(sc->sc_mtx);
+	if (sc_ref->refs <= 1) {
+		ucom_unit_free(sc_ref->unit);
+		free(sc_ref, M_USBDEV);
+	} else {
+		sc_ref->refs--;
+	}
 }
 
 static cn_probe_t ucom_cnprobe;
=== dev/usb/serial/usb_serial.h
==================================================================
--- dev/usb/serial/usb_serial.h	(revision 239054)
+++ dev/usb/serial/usb_serial.h	(local)
@@ -131,12 +131,18 @@
 	struct termios termios_copy;
 };
 
+struct ucom_ref {
+	int refs;
+	int unit;
+};
+
 struct ucom_super_softc {
 	struct usb_process sc_tq;
 	int sc_unit;
 	int sc_subunits;
 	struct sysctl_oid *sc_sysctl_ttyname;
 	struct sysctl_oid *sc_sysctl_ttyports;
+	struct ucom_ref *sc_ref;
 	char sc_ttyname[16];
 };
 
@@ -158,7 +164,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 +184,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=_M41IQdlR19hhEdx--



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