Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 23 Aug 2008 14:20:23 GMT
From:      Ed Schouten <ed@FreeBSD.org>
To:        Perforce Change Reviews <perforce@FreeBSD.org>
Subject:   PERFORCE change 148185 for review
Message-ID:  <200808231420.m7NEKN7m003551@repoman.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=148185

Change 148185 by ed@ed_dull on 2008/08/23 14:19:22

	Somewhat change the ttyinq/ttyoutq block deallocation policy.
	
	One thing I couldn't really understand about 4 weeks ago, was
	why the TTY queues still had blocks allocated inside
	tty_rel_free(), so I just added a second pass of TTY queue block
	deallocation.
	
	After my TTY queue commit today, I suddenly understood why.
	After ttydev_leave(), it is still possible for blocks to be
	returned to the TTY queues, which means we could still have
	blocks sitting in the queues.
	
	Remove some XXX'es from the TTY queues by implementing what I
	already wanted to do earlier: proper block quotas. If you now
	decrease the baud rate, the queue size will not be decreased,
	but blocks will be removed during read()/getc() calls. Introduce
	ttyinq_free() and ttyoutq_free() to force deallocation of queue
	blocks, which is now used by ttydev_leave().

Affected files ...

.. //depot/projects/mpsafetty/sys/dev/snp/snp.c#12 edit
.. //depot/projects/mpsafetty/sys/kern/tty.c#31 edit
.. //depot/projects/mpsafetty/sys/kern/tty_inq.c#6 edit
.. //depot/projects/mpsafetty/sys/kern/tty_outq.c#7 edit
.. //depot/projects/mpsafetty/sys/sys/ttyqueue.h#8 edit

Differences ...

==== //depot/projects/mpsafetty/sys/dev/snp/snp.c#12 (text+ko) ====

@@ -107,12 +107,7 @@
 	tp = ss->snp_tty;
 	if (tp != NULL) {
 		tty_lock(tp);
-
-		/* Deallocate buffers. */
-		ttyoutq_flush(&ss->snp_outq);
-		ttyoutq_setsize(&ss->snp_outq, NULL, 0);
-		MPASS(ttyoutq_getsize(&ss->snp_outq) == 0);
-
+		ttyoutq_free(&ss->snp_outq);
 		ttyhook_unregister(tp);
 	}
 

==== //depot/projects/mpsafetty/sys/kern/tty.c#31 (text+ko) ====

@@ -113,23 +113,6 @@
 	tp->t_outlow = (ttyoutq_getsize(&tp->t_outq) * 9) / 10;
 }
 
-static void
-tty_freebuffers(struct tty *tp)
-{
-
-	/* Destroy input buffers. */
-	ttyinq_flush(&tp->t_inq);
-	ttyinq_setsize(&tp->t_inq, NULL, 0);
-	MPASS(ttyinq_getsize(&tp->t_inq) == 0);
-	tp->t_inlow = 0;
-
-	/* Destroy output buffers. */
-	ttyoutq_flush(&tp->t_outq);
-	ttyoutq_setsize(&tp->t_outq, NULL, 0);
-	MPASS(ttyoutq_getsize(&tp->t_outq) == 0);
-	tp->t_outlow = 0;
-}
-
 static int
 tty_drain(struct tty *tp)
 {
@@ -200,7 +183,10 @@
 	ttydisc_close(tp);
 
 	/* Destroy associated buffers already. */
-	tty_freebuffers(tp);
+	ttyinq_free(&tp->t_inq);
+	tp->t_inlow = 0;
+	ttyoutq_free(&tp->t_outq);
+	tp->t_outlow = 0;
 
 	knlist_clear(&tp->t_inpoll.si_note, 1);
 	knlist_clear(&tp->t_outpoll.si_note, 1);
@@ -937,7 +923,8 @@
 		return;
 	}
 
-	tty_freebuffers(tp);
+	MPASS(ttyinq_getsize(&tp->t_inq) == 0);
+	MPASS(ttyoutq_getsize(&tp->t_outq) == 0);
 
 	/* TTY can be deallocated. */
 	dev = tp->t_dev;

==== //depot/projects/mpsafetty/sys/kern/tty_inq.c#6 (text+ko) ====

@@ -89,12 +89,11 @@
 void
 ttyinq_setsize(struct ttyinq *ti, struct tty *tp, size_t size)
 {
-	unsigned int nblocks;
 	struct ttyinq_block *tib;
 
-	nblocks = howmany(size, TTYINQ_DATASIZE);
+	ti->ti_quota = howmany(size, TTYINQ_DATASIZE);
 
-	while (nblocks > ti->ti_nblocks) {
+	while (ti->ti_quota > ti->ti_nblocks) {
 		/*
 		 * List is getting bigger.
 		 * Add new blocks to the tail of the list.
@@ -115,30 +114,23 @@
 		TAILQ_INSERT_TAIL(&ti->ti_list, tib, tib_list);
 		ti->ti_nblocks++;
 	}
+}
 
-	while (nblocks < ti->ti_nblocks) {
-		/*
-		 * List is getting smaller. Remove unused blocks at the
-		 * end. This means we cannot guarantee this routine
-		 * shrinks buffers properly, when we need to reclaim
-		 * more space than there is available.
-		 *
-		 * XXX TODO: Two solutions here:
-		 * - Throw data away
-		 * - Temporarily hit the watermark until enough data has
-		 *   been flushed, so we can remove the blocks.
-		 */
+void
+ttyinq_free(struct ttyinq *ti)
+{
+	struct ttyinq_block *tib;
+	
+	ttyinq_flush(ti);
+	ti->ti_quota = 0;
 
-		if (ti->ti_end == 0)
-			tib = TAILQ_FIRST(&ti->ti_list);
-		else
-			tib = TAILQ_NEXT(ti->ti_lastblock, tib_list);
-		if (tib == NULL)
-			break;
+	while ((tib = TAILQ_FIRST(&ti->ti_list)) != NULL) {
 		TAILQ_REMOVE(&ti->ti_list, tib, tib_list);
 		uma_zfree(ttyinq_zone, tib);
 		ti->ti_nblocks--;
 	}
+
+	MPASS(ti->ti_nblocks == 0);
 }
 
 int
@@ -223,13 +215,12 @@
 				return (ENXIO);
 			}
 			/* Block can now be readded to the list. */
-			/*
-			 * XXX: we could remove the blocks here when the
-			 * queue was shrunk, but still in use. See
-			 * ttyinq_setsize().
-			 */
-			TAILQ_INSERT_TAIL(&ti->ti_list, tib, tib_list);
-			ti->ti_nblocks++;
+			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++;
+			}
 			if (error != 0)
 				return (error);
 		} else {

==== //depot/projects/mpsafetty/sys/kern/tty_outq.c#7 (text+ko) ====

@@ -78,12 +78,11 @@
 void
 ttyoutq_setsize(struct ttyoutq *to, struct tty *tp, size_t size)
 {
-	unsigned int nblocks;
 	struct ttyoutq_block *tob;
 
-	nblocks = howmany(size, TTYOUTQ_DATASIZE);
+	to->to_quota = howmany(size, TTYOUTQ_DATASIZE);
 
-	while (nblocks > to->to_nblocks) {
+	while (to->to_quota > to->to_nblocks) {
 		/*
 		 * List is getting bigger.
 		 * Add new blocks to the tail of the list.
@@ -104,34 +103,23 @@
 		STAILQ_INSERT_TAIL(&to->to_list, tob, tob_list);
 		to->to_nblocks++;
 	}
+}
 
-	while (nblocks < to->to_nblocks) {
-		/*
-		 * List is getting smaller. Remove unused blocks at the
-		 * end. This means we cannot guarantee this routine
-		 * shrinks buffers properly, when we need to reclaim
-		 * more space than there is available.
-		 *
-		 * XXX TODO: Two solutions here:
-		 * - Throw data away
-		 * - Temporarily hit the watermark until enough data has
-		 *   been flushed, so we can remove the blocks.
-		 */
+void
+ttyoutq_free(struct ttyoutq *to)
+{
+	struct ttyoutq_block *tob;
+	
+	ttyoutq_flush(to);
+	to->to_quota = 0;
 
-		if (to->to_end == 0) {
-			tob = STAILQ_FIRST(&to->to_list);
-			if (tob == NULL)
-				break;
-			STAILQ_REMOVE_HEAD(&to->to_list, tob_list);
-		} else {
-			tob = STAILQ_NEXT(to->to_lastblock, tob_list);
-			if (tob == NULL)
-				break;
-			STAILQ_REMOVE_NEXT(&to->to_list, to->to_lastblock, tob_list);
-		}
+	while ((tob = STAILQ_FIRST(&to->to_list)) != NULL) {
+		STAILQ_REMOVE_HEAD(&to->to_list, tob_list);
 		uma_zfree(ttyoutq_zone, tob);
 		to->to_nblocks--;
 	}
+
+	MPASS(to->to_nblocks == 0);
 }
 
 size_t
@@ -164,7 +152,12 @@
 		if (cend == TTYOUTQ_DATASIZE || cend == to->to_end) {
 			/* Read the block until the end. */
 			STAILQ_REMOVE_HEAD(&to->to_list, tob_list);
-			STAILQ_INSERT_TAIL(&to->to_list, tob, 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);
+			}
 			to->to_begin = 0;
 			if (to->to_end <= TTYOUTQ_DATASIZE) {
 				to->to_end = 0;
@@ -251,13 +244,12 @@
 			tty_lock(tp);
 
 			/* Block can now be readded to the list. */
-			/*
-			 * XXX: we could remove the blocks here when the
-			 * queue was shrunk, but still in use. See
-			 * ttyoutq_setsize().
-			 */
-			STAILQ_INSERT_TAIL(&to->to_list, tob, tob_list);
-			to->to_nblocks++;
+			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++;
+			}
 			if (error != 0)
 				return (error);
 		} else {

==== //depot/projects/mpsafetty/sys/sys/ttyqueue.h#8 (text+ko) ====

@@ -52,6 +52,7 @@
 	unsigned int			ti_reprint;
 	unsigned int			ti_end;
 	unsigned int			ti_nblocks;
+	unsigned int			ti_quota;
 };
 #define TTYINQ_DATASIZE 128
 
@@ -62,12 +63,14 @@
 	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)))
 
 #ifdef _KERNEL
 /* Input queue handling routines. */
 void	ttyinq_setsize(struct ttyinq *, struct tty *, size_t);
+void	ttyinq_free(struct ttyinq *);
 int	ttyinq_read_uio(struct ttyinq *, struct tty *, struct uio *,
     size_t, size_t);
 size_t	ttyinq_write(struct ttyinq *, const void *, size_t, int);
@@ -131,6 +134,7 @@
 /* Output queue handling routines. */
 void	ttyoutq_flush(struct ttyoutq *);
 void	ttyoutq_setsize(struct ttyoutq *, struct tty *, size_t);
+void	ttyoutq_free(struct ttyoutq *);
 size_t	ttyoutq_read(struct ttyoutq *, void *, size_t);
 int	ttyoutq_read_uio(struct ttyoutq *, struct tty *, struct uio *);
 size_t	ttyoutq_write(struct ttyoutq *, const void *, size_t);



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