svn commit: r188096 - in head/sys: dev/snp kern sys

Ed Schouten ed at FreeBSD.org
Tue Feb 3 11:58:34 PST 2009


Author: ed
Date: Tue Feb  3 19:58:28 2009
New Revision: 188096
URL: http://svn.freebsd.org/changeset/base/188096

Log:
  Slightly improve the design of the TTY buffer.
  
  The TTY buffers used the standard <sys/queue.h> lists. Unfortunately
  they have a big shortcoming. If you want to have a double linked list,
  but no tail pointer, it's still not possible to obtain the previous
  element in the list. Inside the buffers we don't need them. This is why
  I switched to custom linked list macros. The macros will also keep track
  of the amount of items in the list. Because it doesn't use a sentinel,
  we can just initialize the queues with zero.
  
  In its simplest form (the output queue), we will only keep two
  references to blocks in the queue, namely the head of the list and the
  last block in use. All free blocks are stored behind the last block in
  use.
  
  I noticed there was a very subtle bug in the previous code: in a very
  uncommon corner case, it would uma_zfree() a block in the queue before
  calling memcpy() to extract the data from the block.

Modified:
  head/sys/dev/snp/snp.c
  head/sys/kern/tty.c
  head/sys/kern/tty_inq.c
  head/sys/kern/tty_outq.c
  head/sys/sys/ttyqueue.h

Modified: head/sys/dev/snp/snp.c
==============================================================================
--- head/sys/dev/snp/snp.c	Tue Feb  3 19:50:11 2009	(r188095)
+++ head/sys/dev/snp/snp.c	Tue Feb  3 19:58:28 2009	(r188096)
@@ -127,7 +127,6 @@ snp_open(struct cdev *dev, int flag, int
 
 	/* Allocate per-snoop data. */
 	ss = malloc(sizeof(struct snp_softc), M_SNP, M_WAITOK|M_ZERO);
-	ttyoutq_init(&ss->snp_outq);
 	cv_init(&ss->snp_outwait, "snp out");
 
 	devfs_set_cdevpriv(ss, snp_dtor);

Modified: head/sys/kern/tty.c
==============================================================================
--- head/sys/kern/tty.c	Tue Feb  3 19:50:11 2009	(r188095)
+++ head/sys/kern/tty.c	Tue Feb  3 19:58:28 2009	(r188096)
@@ -884,9 +884,6 @@ tty_alloc(struct ttydevsw *tsw, void *sc
 	cv_init(&tp->t_bgwait, "ttybg");
 	cv_init(&tp->t_dcdwait, "ttydcd");
 
-	ttyinq_init(&tp->t_inq);
-	ttyoutq_init(&tp->t_outq);
-
 	/* Allow drivers to use a custom mutex to lock the TTY. */
 	if (mutex != NULL) {
 		tp->t_mtx = mutex;

Modified: head/sys/kern/tty_inq.c
==============================================================================
--- head/sys/kern/tty_inq.c	Tue Feb  3 19:50:11 2009	(r188095)
+++ head/sys/kern/tty_inq.c	Tue Feb  3 19:58:28 2009	(r188096)
@@ -79,13 +79,43 @@ SYSCTL_LONG(_kern, OID_AUTO, tty_inq_nsl
 	((tib)->tib_quotes[(boff) / BMSIZE] &= ~(1 << ((boff) % BMSIZE)))
 
 struct ttyinq_block {
-	TAILQ_ENTRY(ttyinq_block) tib_list;
-	uint32_t	tib_quotes[TTYINQ_QUOTESIZE];
-	char		tib_data[TTYINQ_DATASIZE];
+	struct ttyinq_block	*tib_prev;
+	struct ttyinq_block	*tib_next;
+	uint32_t		tib_quotes[TTYINQ_QUOTESIZE];
+	char			tib_data[TTYINQ_DATASIZE];
 };
 
 static uma_zone_t ttyinq_zone;
 
+#define	TTYINQ_INSERT_TAIL(ti, tib) do {				\
+	if (ti->ti_end == 0) {						\
+		tib->tib_prev = NULL;					\
+		tib->tib_next = ti->ti_firstblock;			\
+		ti->ti_firstblock = tib;				\
+	} else {							\
+		tib->tib_prev = ti->ti_lastblock;			\
+		tib->tib_next = ti->ti_lastblock->tib_next;		\
+		ti->ti_lastblock->tib_next = tib;			\
+	}								\
+	if (tib->tib_next != NULL)					\
+		tib->tib_next->tib_prev = tib;				\
+	ti->ti_nblocks++;						\
+} while (0)
+
+#define	TTYINQ_REMOVE_HEAD(ti) do {					\
+	ti->ti_firstblock = ti->ti_firstblock->tib_next;		\
+	if (ti->ti_firstblock != NULL)					\
+		ti->ti_firstblock->tib_prev = NULL;			\
+	ti->ti_nblocks--;						\
+} while (0)
+
+#define	TTYINQ_RECYCLE(ti, tib) do {					\
+	if (ti->ti_quota <= ti->ti_nblocks)				\
+		uma_zfree(ttyinq_zone, tib);				\
+	else								\
+		TTYINQ_INSERT_TAIL(ti, tib);				\
+} while (0)
+
 void
 ttyinq_setsize(struct ttyinq *ti, struct tty *tp, size_t size)
 {
@@ -108,8 +138,7 @@ ttyinq_setsize(struct ttyinq *ti, struct
 		tib = uma_zalloc(ttyinq_zone, M_WAITOK);
 		tty_lock(tp);
 
-		TAILQ_INSERT_TAIL(&ti->ti_list, tib, tib_list);
-		ti->ti_nblocks++;
+		TTYINQ_INSERT_TAIL(ti, tib);
 	}
 }
 
@@ -121,10 +150,9 @@ ttyinq_free(struct ttyinq *ti)
 	ttyinq_flush(ti);
 	ti->ti_quota = 0;
 
-	while ((tib = TAILQ_FIRST(&ti->ti_list)) != NULL) {
-		TAILQ_REMOVE(&ti->ti_list, tib, tib_list);
+	while ((tib = ti->ti_firstblock) != NULL) {
+		TTYINQ_REMOVE_HEAD(ti);
 		uma_zfree(ttyinq_zone, tib);
-		ti->ti_nblocks--;
 	}
 
 	MPASS(ti->ti_nblocks == 0);
@@ -145,7 +173,7 @@ ttyinq_read_uio(struct ttyinq *ti, struc
 		/* See if there still is data. */
 		if (ti->ti_begin == ti->ti_linestart)
 			return (0);
-		tib = TAILQ_FIRST(&ti->ti_list);
+		tib = ti->ti_firstblock;
 		if (tib == NULL)
 			return (0);
 
@@ -176,8 +204,7 @@ ttyinq_read_uio(struct ttyinq *ti, struc
 			 * Fast path: zero copy. Remove the first block,
 			 * so we can unlock the TTY temporarily.
 			 */
-			TAILQ_REMOVE(&ti->ti_list, tib, tib_list);
-			ti->ti_nblocks--;
+			TTYINQ_REMOVE_HEAD(ti);
 			ti->ti_begin = 0;
 
 			/*
@@ -185,11 +212,10 @@ ttyinq_read_uio(struct ttyinq *ti, struc
 			 * fix up the block offsets.
 			 */
 #define CORRECT_BLOCK(t) do {			\
-	if (t <= TTYINQ_DATASIZE) {		\
+	if (t <= TTYINQ_DATASIZE)		\
 		t = 0;				\
-	} else {				\
+	else					\
 		t -= TTYINQ_DATASIZE;		\
-	}					\
 } while (0)
 			CORRECT_BLOCK(ti->ti_linestart);
 			CORRECT_BLOCK(ti->ti_reprint);
@@ -207,12 +233,7 @@ ttyinq_read_uio(struct ttyinq *ti, struc
 			tty_lock(tp);
 
 			/* Block can now be readded to the list. */
-			if (ti->ti_quota <= ti->ti_nblocks) {
-				uma_zfree(ttyinq_zone, tib);
-			} else {
-				TAILQ_INSERT_TAIL(&ti->ti_list, tib, tib_list);
-				ti->ti_nblocks++;
-			}
+			TTYINQ_RECYCLE(ti, tib);
 		} else {
 			char ob[TTYINQ_DATASIZE - 1];
 			atomic_add_long(&ttyinq_nslow, 1);
@@ -264,25 +285,27 @@ ttyinq_write(struct ttyinq *ti, const vo
 	size_t l;
 	
 	while (nbytes > 0) {
-		tib = ti->ti_lastblock;
 		boff = ti->ti_end % TTYINQ_DATASIZE;
 
 		if (ti->ti_end == 0) {
 			/* First time we're being used or drained. */
 			MPASS(ti->ti_begin == 0);
-			tib = ti->ti_lastblock = TAILQ_FIRST(&ti->ti_list);
+			tib = ti->ti_firstblock;
 			if (tib == NULL) {
 				/* Queue has no blocks. */
 				break;
 			}
+			ti->ti_lastblock = tib;
 		} else if (boff == 0) {
 			/* We reached the end of this block on last write. */
-			tib = TAILQ_NEXT(tib, tib_list);
+			tib = ti->ti_lastblock->tib_next;
 			if (tib == NULL) {
 				/* We've reached the watermark. */
 				break;
 			}
 			ti->ti_lastblock = tib;
+		} else {
+			tib = ti->ti_lastblock;
 		}
 
 		/* Don't copy more than was requested. */
@@ -328,7 +351,7 @@ size_t
 ttyinq_findchar(struct ttyinq *ti, const char *breakc, size_t maxlen,
     char *lastc)
 {
-	struct ttyinq_block *tib = TAILQ_FIRST(&ti->ti_list);
+	struct ttyinq_block *tib = ti->ti_firstblock;
 	unsigned int boff = ti->ti_begin;
 	unsigned int bend = MIN(MIN(TTYINQ_DATASIZE, ti->ti_linestart),
 	    ti->ti_begin + maxlen);
@@ -402,8 +425,7 @@ ttyinq_unputchar(struct ttyinq *ti)
 
 	if (--ti->ti_end % TTYINQ_DATASIZE == 0) {
 		/* Roll back to the previous block. */
-		ti->ti_lastblock = TAILQ_PREV(ti->ti_lastblock,
-		    ttyinq_bhead, tib_list);
+		ti->ti_lastblock = ti->ti_lastblock->tib_prev;
 		/*
 		 * This can only fail if we are unputchar()'ing the
 		 * first character in the queue.
@@ -437,7 +459,7 @@ ttyinq_line_iterate(struct ttyinq *ti,
 
 	/* Use the proper block when we're at the queue head. */
 	if (offset == 0)
-		tib = TAILQ_FIRST(&ti->ti_list);
+		tib = ti->ti_firstblock;
 
 	/* Iterate all characters and call the iterator function. */
 	for (; offset < ti->ti_end; offset++) {
@@ -449,7 +471,7 @@ ttyinq_line_iterate(struct ttyinq *ti,
 
 		/* Last byte iterated - go to the next block. */
 		if (boff == TTYINQ_DATASIZE - 1)
-			tib = TAILQ_NEXT(tib, tib_list);
+			tib = tib->tib_next;
 		MPASS(tib != NULL);
 	}
 }

Modified: head/sys/kern/tty_outq.c
==============================================================================
--- head/sys/kern/tty_outq.c	Tue Feb  3 19:50:11 2009	(r188095)
+++ head/sys/kern/tty_outq.c	Tue Feb  3 19:58:28 2009	(r188096)
@@ -61,12 +61,35 @@ SYSCTL_LONG(_kern, OID_AUTO, tty_outq_ns
 	&ttyoutq_nslow, 0, "Buffered reads to userspace on output");
 
 struct ttyoutq_block {
-	STAILQ_ENTRY(ttyoutq_block) tob_list;
-	char	tob_data[TTYOUTQ_DATASIZE];
+	struct ttyoutq_block	*tob_next;
+	char			tob_data[TTYOUTQ_DATASIZE];
 };
 
 static uma_zone_t ttyoutq_zone;
 
+#define	TTYOUTQ_INSERT_TAIL(to, tob) do {				\
+	if (to->to_end == 0) {						\
+		tob->tob_next = to->to_firstblock;			\
+		to->to_firstblock = tob;				\
+	} else {							\
+		tob->tob_next = to->to_lastblock->tob_next;		\
+		to->to_lastblock->tob_next = tob;			\
+	}								\
+	to->to_nblocks++;						\
+} while (0)
+
+#define	TTYOUTQ_REMOVE_HEAD(to) do {					\
+	to->to_firstblock = to->to_firstblock->tob_next;		\
+	to->to_nblocks--;						\
+} while (0)
+
+#define	TTYOUTQ_RECYCLE(to, tob) do {					\
+	if (to->to_quota <= to->to_nblocks)				\
+		uma_zfree(ttyoutq_zone, tob);				\
+	else								\
+		TTYOUTQ_INSERT_TAIL(to, tob);				\
+} while(0)
+
 void
 ttyoutq_flush(struct ttyoutq *to)
 {
@@ -97,8 +120,7 @@ ttyoutq_setsize(struct ttyoutq *to, stru
 		tob = uma_zalloc(ttyoutq_zone, M_WAITOK);
 		tty_lock(tp);
 
-		STAILQ_INSERT_TAIL(&to->to_list, tob, tob_list);
-		to->to_nblocks++;
+		TTYOUTQ_INSERT_TAIL(to, tob);
 	}
 }
 
@@ -110,10 +132,9 @@ ttyoutq_free(struct ttyoutq *to)
 	ttyoutq_flush(to);
 	to->to_quota = 0;
 
-	while ((tob = STAILQ_FIRST(&to->to_list)) != NULL) {
-		STAILQ_REMOVE_HEAD(&to->to_list, tob_list);
+	while ((tob = to->to_firstblock) != NULL) {
+		TTYOUTQ_REMOVE_HEAD(to);
 		uma_zfree(ttyoutq_zone, tob);
-		to->to_nblocks--;
 	}
 
 	MPASS(to->to_nblocks == 0);
@@ -131,7 +152,7 @@ ttyoutq_read(struct ttyoutq *to, void *b
 		/* See if there still is data. */
 		if (to->to_begin == to->to_end)
 			break;
-		tob = STAILQ_FIRST(&to->to_list);
+		tob = to->to_firstblock;
 		if (tob == NULL)
 			break;
 
@@ -146,30 +167,25 @@ ttyoutq_read(struct ttyoutq *to, void *b
 		    TTYOUTQ_DATASIZE);
 		clen = cend - cbegin;
 
-		if (cend == TTYOUTQ_DATASIZE || cend == to->to_end) {
+		/* Copy the data out of the buffers. */
+		memcpy(cbuf, tob->tob_data + cbegin, clen);
+		cbuf += clen;
+		len -= clen;
+
+		if (cend == to->to_end) {
+			/* Read the complete queue. */
+			to->to_begin = 0;
+			to->to_end = 0;
+		} else if (cend == TTYOUTQ_DATASIZE) {
 			/* Read the block until the end. */
-			STAILQ_REMOVE_HEAD(&to->to_list, tob_list);
-			if (to->to_quota < to->to_nblocks) {
-				uma_zfree(ttyoutq_zone, tob);
-				to->to_nblocks--;
-			} else {
-				STAILQ_INSERT_TAIL(&to->to_list, tob, tob_list);
-			}
+			TTYOUTQ_REMOVE_HEAD(to);
 			to->to_begin = 0;
-			if (to->to_end <= TTYOUTQ_DATASIZE) {
-				to->to_end = 0;
-			} else {
-				to->to_end -= TTYOUTQ_DATASIZE;
-			}
+			to->to_end -= TTYOUTQ_DATASIZE;
+			TTYOUTQ_RECYCLE(to, tob);
 		} else {
 			/* Read the block partially. */
 			to->to_begin += clen;
 		}
-
-		/* Copy the data out of the buffers. */
-		memcpy(cbuf, tob->tob_data + cbegin, clen);
-		cbuf += clen;
-		len -= clen;
 	}
 
 	return (cbuf - (char *)buf);
@@ -197,7 +213,7 @@ ttyoutq_read_uio(struct ttyoutq *to, str
 		/* See if there still is data. */
 		if (to->to_begin == to->to_end)
 			return (0);
-		tob = STAILQ_FIRST(&to->to_list);
+		tob = to->to_firstblock;
 		if (tob == NULL)
 			return (0);
 
@@ -226,14 +242,12 @@ ttyoutq_read_uio(struct ttyoutq *to, str
 			 * Fast path: zero copy. Remove the first block,
 			 * so we can unlock the TTY temporarily.
 			 */
-			STAILQ_REMOVE_HEAD(&to->to_list, tob_list);
-			to->to_nblocks--;
+			TTYOUTQ_REMOVE_HEAD(to);
 			to->to_begin = 0;
-			if (to->to_end <= TTYOUTQ_DATASIZE) {
+			if (to->to_end <= TTYOUTQ_DATASIZE)
 				to->to_end = 0;
-			} else {
+			else
 				to->to_end -= TTYOUTQ_DATASIZE;
-			}
 
 			/* Temporary unlock and copy the data to userspace. */
 			tty_unlock(tp);
@@ -241,12 +255,7 @@ ttyoutq_read_uio(struct ttyoutq *to, str
 			tty_lock(tp);
 
 			/* Block can now be readded to the list. */
-			if (to->to_quota <= to->to_nblocks) {
-				uma_zfree(ttyoutq_zone, tob);
-			} else {
-				STAILQ_INSERT_TAIL(&to->to_list, tob, tob_list);
-				to->to_nblocks++;
-			}
+			TTYOUTQ_RECYCLE(to, tob);
 		} else {
 			char ob[TTYOUTQ_DATASIZE - 1];
 			atomic_add_long(&ttyoutq_nslow, 1);
@@ -280,26 +289,27 @@ ttyoutq_write(struct ttyoutq *to, const 
 	size_t l;
 
 	while (nbytes > 0) {
-		/* Offset in current block. */
-		tob = to->to_lastblock;
 		boff = to->to_end % TTYOUTQ_DATASIZE;
 
 		if (to->to_end == 0) {
 			/* First time we're being used or drained. */
 			MPASS(to->to_begin == 0);
-			tob = to->to_lastblock = STAILQ_FIRST(&to->to_list);
+			tob = to->to_firstblock;
 			if (tob == NULL) {
 				/* Queue has no blocks. */
 				break;
 			}
+			to->to_lastblock = tob;
 		} else if (boff == 0) {
 			/* We reached the end of this block on last write. */
-			tob = STAILQ_NEXT(tob, tob_list);
+			tob = to->to_lastblock->tob_next;
 			if (tob == NULL) {
 				/* We've reached the watermark. */
 				break;
 			}
 			to->to_lastblock = tob;
+		} else {
+			tob = to->to_lastblock;
 		}
 
 		/* Don't copy more than was requested. */

Modified: head/sys/sys/ttyqueue.h
==============================================================================
--- head/sys/sys/ttyqueue.h	Tue Feb  3 19:50:11 2009	(r188095)
+++ head/sys/sys/ttyqueue.h	Tue Feb  3 19:58:28 2009	(r188096)
@@ -43,29 +43,29 @@ struct uio;
 
 /* Data input queue. */
 struct ttyinq {
-	TAILQ_HEAD(ttyinq_bhead, ttyinq_block) ti_list;
-	struct ttyinq_block		*ti_startblock;
-	struct ttyinq_block		*ti_reprintblock;
-	struct ttyinq_block		*ti_lastblock;
-	unsigned int			ti_begin;
-	unsigned int			ti_linestart;
-	unsigned int			ti_reprint;
-	unsigned int			ti_end;
-	unsigned int			ti_nblocks;
-	unsigned int			ti_quota;
+	struct ttyinq_block	*ti_firstblock;
+	struct ttyinq_block	*ti_startblock;
+	struct ttyinq_block	*ti_reprintblock;
+	struct ttyinq_block	*ti_lastblock;
+	unsigned int		ti_begin;
+	unsigned int		ti_linestart;
+	unsigned int		ti_reprint;
+	unsigned int		ti_end;
+	unsigned int		ti_nblocks;
+	unsigned int		ti_quota;
 };
 #define TTYINQ_DATASIZE 128
 
 /* Data output queue. */
 struct ttyoutq {
-	STAILQ_HEAD(, ttyoutq_block)	to_list;
-	struct ttyoutq_block		*to_lastblock;
-	unsigned int			to_begin;
-	unsigned int			to_end;
-	unsigned int			to_nblocks;
-	unsigned int			to_quota;
+	struct ttyoutq_block	*to_firstblock;
+	struct ttyoutq_block	*to_lastblock;
+	unsigned int		to_begin;
+	unsigned int		to_end;
+	unsigned int		to_nblocks;
+	unsigned int		to_quota;
 };
-#define TTYOUTQ_DATASIZE (256 - sizeof(STAILQ_ENTRY(ttyoutq_block)))
+#define TTYOUTQ_DATASIZE (256 - sizeof(struct ttyoutq_block *))
 
 #ifdef _KERNEL
 /* Input queue handling routines. */
@@ -86,13 +86,6 @@ void	ttyinq_unputchar(struct ttyinq *ti)
 void	ttyinq_reprintpos_set(struct ttyinq *ti);
 void	ttyinq_reprintpos_reset(struct ttyinq *ti);
 
-static __inline void
-ttyinq_init(struct ttyinq *ti)
-{
-
-	TAILQ_INIT(&ti->ti_list);
-}
-
 static __inline size_t
 ttyinq_getsize(struct ttyinq *ti)
 {
@@ -143,13 +136,6 @@ int	ttyoutq_read_uio(struct ttyoutq *to,
 size_t	ttyoutq_write(struct ttyoutq *to, const void *buf, size_t len);
 int	ttyoutq_write_nofrag(struct ttyoutq *to, const void *buf, size_t len);
 
-static __inline void
-ttyoutq_init(struct ttyoutq *to)
-{
-
-	STAILQ_INIT(&to->to_list);
-}
-
 static __inline size_t
 ttyoutq_getsize(struct ttyoutq *to)
 {


More information about the svn-src-all mailing list