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
[-- Attachment #1 --]
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
[-- Attachment #2 --]
=== 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 */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201208071703.13773.hselasky>