[comp.os.minix] insufficient buffer size in tty code; linewrap

hedrick@athos.rutgers.edu (Charles Hedrick) (10/03/88)

I've now seen a couple of pieces of evidence pointing to insufficient
buffer sizes in the rs232 code.  (Actually, I think there's a
scheduling latency issue, but it shows up as buffer overflow and can
be fixed in a brute force way by increasing the buffer sizes.)  A
friend tried some operations at 9600 baud on a pair of AT's.  It
doesn't look like he's losing any characters at interrupt level.  So I
think the interrupt latency is now good enough.  But when he put out a
large amount of text, he began losing pieces of it.  This suggests
that he was overflowing a buffer.  (If you have an interrupt latency
problem, you'll get random drops interspersed throughout your output,
usually fairly often.  If you are overflowing a buffer, you'll lose
large chunks spaced fairly far apart, and only where you're displaying
a lot.)  I had occasion to use kermit to retrieve some files directly
onto a floppy.  (I'm running short of hard disk space on my minix
partition.)  This lost badly.  Lots of bad packets and timeouts from
kermit.  I did eventually get the data, but it was very slow.  I put
code into the kernel to output messages when various buffers
overflowed.  I found that I was overflowing both the interrupt-level
buffer and the tty input buffer.  I should have expected the tty input
buffer problem.  When you're using a packet protocol, you should in
general have enough buffering to handle at least one packet (depends
upon whether sliding window is in use -- with kermit one packet is
enough.  I use 1000 byte packets, which causes kermit to get quite
good line utilization.)  The interrupt level buffer was a surprise.
The buffer had 100 characters, which at 2400 baud should have lasted
for almost 1/2 second.  Surely the tty task should get to run within
1/2 second.  About all I can think is that there's some problem in the
way communication is being done such that when the floppy starts its
motor (which takes 1/2 sec, if remember correctly), the tty task gets
locked out for that period.  Note that this is not an interrupt
latency problem.  We weren't losing interrupts, and the floppy code
definitely doesn't disable interrupts for long times.  Rather it's a
problem of tasks getting scheduled.

The following changes give big enough buffers to use kermit with
a packet size of 1000 to a floppy.  I haven't tried to find the
absolute minimum sizes that work.  However a factor of two less
on either of the parameters didn't work.  Note that I had to make a 
couple of changes in addition to parameters:
  - the interrupt buffer was keeping a count in the first byte.
	I changed it to use 2 bytes.
  - makefile didn't list dependencies for rs232, so it wasn't
	getting recompiled.  This is fatal.
The other makefile changes are for my personal taste.  Take them
or not as you like.  -DLINEWRAP allows line wrap.

By the way, ast and I are having sort of an argument about line wrap.
He would like to remove it.  I left it in under a conditional because
there had been a number of requests on this group for it.  I agree
with him that correct programs don't put out more than 80 chars on a
line.  Unfortunately I don't always work with correct programs.  The
last thing I want is that when I get an unexpected message, or
unexpected data, things disappear from the screen.  Debugging is hard
enough as is without having critical data go away because it overflows
the line.  I'm not sure what I'm asking, but if others feel that
linewrap is useful, you may want to mention something to ast, because
otherwise it's likely to go away (though it will be easy enough to add
back of course).  (My perference, by the way, would be to supply one
of those ESC [ mode setting commands to toggle it, rather than 
requiring people to rebuild the system.  I don't mind rebuilding the
kernel, but somebody with a small machine may find it much less
pleasant.)

*** tty.c.ORIG	Sun Oct  2 00:01:25 1988
--- tty.c	Sun Oct  2 18:06:25 1988
***************
*** 160,173 ****
    old_state = lock();
    ptr = m_ptr->ADDRESS;		/* pointer to accumulated char array */
    copy_ptr = tty_copy_buf;	/* ptr to shadow array where chars copied */
!   n = *ptr;			/* how many chars have been accumulated */
!   count = n;			/* save the character count */
!   n = n + n;			/* each char occupies 2 bytes */
!   ptr += 2;			/* skip count field at start of array */
    while (n-- > 0)
  	*copy_ptr++ = *ptr++;	/* copy the array to safety */
    ptr = m_ptr->ADDRESS;
!   *ptr = 0;			/* accumulation count set to 0 */
    restore(old_state);
  
    /* Loop on the accumulated characters, processing each in turn. */
--- 160,173 ----
    old_state = lock();
    ptr = m_ptr->ADDRESS;		/* pointer to accumulated char array */
    copy_ptr = tty_copy_buf;	/* ptr to shadow array where chars copied */
!   n = tty_buf_count(ptr);	/* how many chars have been accumulated */
!   count = n;			/* save the character count */
!   n = n + n;			/* each char occupies 2 bytes */
!   ptr += 4;			/* skip count field at start of array */
    while (n-- > 0)
  	*copy_ptr++ = *ptr++;	/* copy the array to safety */
    ptr = m_ptr->ADDRESS;
!   tty_buf_count(ptr) = 0;		/* accumulation count set to 0 */
    restore(old_state);
  
    /* Loop on the accumulated characters, processing each in turn. */
*** console.c.ORIG	Sat Oct  1 23:49:20 1988
--- console.c	Sun Oct  2 13:41:37 1988
***************
*** 149,163 ****
    if (control && alt && code == DEL_CODE) reboot();	/* CTRL-ALT-DEL */
  
    /* Store the character in memory so the task can get at it later.
!    * tty_driver_buf[0] is the current count, and tty_driver_buf[1] is the
!    * maximum allowed to be stored.
!    */
!   if ( (k = tty_driver_buf[0]) < tty_driver_buf[1]) {
! 	/* There is room to store this character; do it. */
! 	k = k + k;			/* each entry contains two bytes */
! 	tty_driver_buf[k+2] = code;	/* store the scan code */
! 	tty_driver_buf[k+3] = CONSOLE;	/* tell which line it came from */
! 	tty_driver_buf[0]++;		/* increment counter */
  
  	/* Build and send the interrupt message. */
  	keybd_mess.m_type = TTY_CHAR_INT;
--- 149,163 ----
    if (control && alt && code == DEL_CODE) reboot();	/* CTRL-ALT-DEL */
  
    /* Store the character in memory so the task can get at it later.
!    * tty_driver_buf[0] is the current count, and tty_driver_buf[2] is the
!    * maximum allowed to be stored.
!    */
!   if ( (k = tty_buf_count(tty_driver_buf)) < tty_buf_max(tty_driver_buf)) {
! 	/* There is room to store this character; do it. */
! 	k = k + k;			/* each entry contains two bytes */
! 	tty_driver_buf[k+4] = code;	/* store the scan code */
! 	tty_driver_buf[k+5] = CONSOLE;	/* tell which line it came from */
! 	tty_buf_count(tty_driver_buf)++;		/* increment counter */
  
  	/* Build and send the interrupt message. */
  	keybd_mess.m_type = TTY_CHAR_INT;
***************
*** 1103,1109 ****
  	 */
  	char_height = get_byte(0x40, 0x85);  
    }
!   tty_driver_buf[1] = MAX_OVERRUN;	/* set up limit on keyboard buffering*/
    set_6845(CUR_SIZE, 31);		/* set cursor shape */
    set_6845(VID_ORG, 0);			/* use page 0 of video ram */
    move_to(&tty_struct[0], 0, SCR_LINES-1); /* move cursor to lower left */
--- 1103,1109 ----
  	 */
  	char_height = get_byte(0x40, 0x85);  
    }
!   tty_buf_max(tty_driver_buf) = MAX_OVERRUN;	/* set up limit on keyboard buffering*/
    set_6845(CUR_SIZE, 31);		/* set cursor shape */
    set_6845(VID_ORG, 0);			/* use page 0 of video ram */
    move_to(&tty_struct[0], 0, SCR_LINES-1); /* move cursor to lower left */
*** rs232.c.ORIG	Sat Oct  1 23:59:39 1988
--- rs232.c	Sun Oct  2 13:43:24 1988
***************
*** 135,148 ****
    port_in(base + RS232_RECEIVER_DATA_REG, &val);
  
    /* Store the character in memory so the task can get at it later */
!   if ((k = tty_driver_buf[0]) < tty_driver_buf[1]) {
! 	/* There is room to store this character, do it */
! 	k = k + k;			/* each entry contains two bytes */
! 	tty_driver_buf[k + 2] = val;	/* store the ascii code */
! 	tty_driver_buf[k + 3] = line;	/* tell wich line it came from */ 
! 	tty_driver_buf[0]++;		/* increment counter */
! 
! 	if (tty_driver_buf[0] < THRESHOLD) {
  		/* Don't send message.  Just accumulate.  Let clock do it. */
  		port_out(INT_CTL, ENABLE);
  		flush_flag++;
--- 135,148 ----
    port_in(base + RS232_RECEIVER_DATA_REG, &val);
  
    /* Store the character in memory so the task can get at it later */
!   if ((k = tty_buf_count(tty_driver_buf)) < tty_buf_max(tty_driver_buf)) {
! 	/* There is room to store this character, do it */
! 	k = k + k;			/* each entry contains two bytes */
! 	tty_driver_buf[k + 4] = val;	/* store the ascii code */
! 	tty_driver_buf[k + 5] = line;	/* tell wich line it came from */ 
! 	tty_buf_count(tty_driver_buf)++;		/* increment counter */
! 
! 	if (tty_buf_count(tty_driver_buf) < THRESHOLD) {
  		/* Don't send message.  Just accumulate.  Let clock do it. */
  		port_out(INT_CTL, ENABLE);
  		flush_flag++;
***************
*** 167,173 ****
  
    /* Build and send the interrupt message */ 
    flush_flag = 0;
!   if (tty_driver_buf[0] == 0) return;	/* nothing to flush */
    rs232_rd_mess.m_type = TTY_CHAR_INT;
    rs232_rd_mess.ADDRESS = tty_driver_buf;
    interrupt(TTY, &rs232_rd_mess);	/* send a message to the tty task */
--- 167,173 ----
  
    /* Build and send the interrupt message */ 
    flush_flag = 0;
!   if (tty_buf_count(tty_driver_buf) == 0) return;	/* nothing to flush */
    rs232_rd_mess.m_type = TTY_CHAR_INT;
    rs232_rd_mess.ADDRESS = tty_driver_buf;
    interrupt(TTY, &rs232_rd_mess);	/* send a message to the tty task */
*** tty.h.ORIG	Sun Oct  2 00:01:56 1988
--- tty.h	Mon Oct  3 00:57:12 1988
***************
*** 1,13 ****
  #define NR_CONS            1	/* how many consoles can system handle */
  #define	NR_RS_LINES	   1	/* how many rs232 terminals can system handle*/
! #define TTY_IN_BYTES     200	/* input queue size */
  #define TTY_RAM_WORDS    320	/* ram buffer size */
  #define TTY_BUF_SIZE     256	/* unit for copying to/from queues */
  #define TAB_SIZE           8	/* distance between tabs */
  #define TAB_MASK          07	/* mask for tty_column when tabbing */
  #define WORD_MASK     0xFFFF	/* mask for 16 bits */
  #define OFF_MASK      0x000F	/* mask for  4 bits */
! #define MAX_OVERRUN      100	/* size of overrun input buffer */
  #define MAX_ESC_PARMS      2	/* number of escape sequence params allowed */
  
  #define ERASE_CHAR      '\b'	/* default erase character */
--- 1,13 ----
  #define NR_CONS            1	/* how many consoles can system handle */
  #define	NR_RS_LINES	   1	/* how many rs232 terminals can system handle*/
! #define TTY_IN_BYTES     1600	/* input queue size */
  #define TTY_RAM_WORDS    320	/* ram buffer size */
  #define TTY_BUF_SIZE     256	/* unit for copying to/from queues */
  #define TAB_SIZE           8	/* distance between tabs */
  #define TAB_MASK          07	/* mask for tty_column when tabbing */
  #define WORD_MASK     0xFFFF	/* mask for 16 bits */
  #define OFF_MASK      0x000F	/* mask for  4 bits */
! #define MAX_OVERRUN      800	/* size of overrun input buffer */
  #define MAX_ESC_PARMS      2	/* number of escape sequence params allowed */
  
  #define ERASE_CHAR      '\b'	/* default erase character */
***************
*** 102,108 ****
  
    /* Miscellaneous. */
    int tty_ioport;		/* I/O port number for this terminal */
! } tty_struct[NR_CONS+NR_RS_LINES];
  
  /* Values for the fields. */
  #define NOT_ESCAPED        0	/* previous character on this line not '\' */
--- 102,110 ----
  
    /* Miscellaneous. */
    int tty_ioport;		/* I/O port number for this terminal */
! 
! } tty_struct[NR_CONS+NR_RS_LINES];
! 
  
  /* Values for the fields. */
  #define NOT_ESCAPED        0	/* previous character on this line not '\' */
***************
*** 117,123 ****
  #define WAITING            1	/* an output process is waiting for a reply */
  #define COMPLETED          2	/* output done; send a completion message */
  
! EXTERN char tty_driver_buf[2*MAX_OVERRUN+2]; /* driver collects chars here */
  EXTERN char tty_copy_buf[2*MAX_OVERRUN];  /* copy buf used to avoid races */
  EXTERN char tty_buf[TTY_BUF_SIZE];	/* scratch buffer to/from user space */
  EXTERN int shift1, shift2, capslock, numlock;	/* keep track of shift keys */
--- 119,127 ----
  #define WAITING            1	/* an output process is waiting for a reply */
  #define COMPLETED          2	/* output done; send a completion message */
  
! EXTERN char tty_driver_buf[2*MAX_OVERRUN+4]; /* driver collects chars here */
! #define tty_buf_count(p) (((int *)(p))[0])
! #define tty_buf_max(p) (((int *)(p))[1])
  EXTERN char tty_copy_buf[2*MAX_OVERRUN];  /* copy buf used to avoid races */
  EXTERN char tty_buf[TTY_BUF_SIZE];	/* scratch buffer to/from user space */
  EXTERN int shift1, shift2, capslock, numlock;	/* keep track of shift keys */
*** at_makefile	Sat Oct  1 23:46:39 1988
--- makefile	Mon Oct  3 00:47:48 1988
***************
*** 11,29 ****
  #  -F		- run cpp and cem sequentially (used when memory is tight)
  #  -T.		- put temporaries in working directory (when RAM disk is small)
  #
! CFLAGS= -Di8088 -F -T.
  h=../h
  l=/usr/lib
  
  obj =	mpx88.s main.s tty.s floppy.s wini.s system.s proc.s clock.s memory.s \
! 	console.s rs232.s printer.s table.s klib88.s dmp.s
  
  cobjs =	main.s tty.s floppy.s wini.s system.s proc.s clock.s memory.s \
  	console.s rs232.s printer.s table.s dmp.s
  
  kernel:	makefile $(obj) $l/libc.a
  	@echo "Start linking Kernel."
! 	@asld -o kernel -T. $(obj) $l/libc.a $l/end.s
  	@echo "Kernel done.  "
  
  clean:
--- 11,29 ----
  #  -F		- run cpp and cem sequentially (used when memory is tight)
  #  -T.		- put temporaries in working directory (when RAM disk is small)
  #  -i flag to asld gives separate I&D space, but only on the newest version
! CFLAGS= -Di8088 -DLINEWRAP
  h=../h
  l=/usr/lib
  
  obj =	mpx88.s main.s tty.s floppy.s wini.s system.s proc.s clock.s memory.s \
! 	console.s rs232.s printer.s table.s klib88.s dmp.s portio.s
  
  cobjs =	main.s tty.s floppy.s wini.s system.s proc.s clock.s memory.s \
  	console.s rs232.s printer.s table.s dmp.s
  
  kernel:	makefile $(obj) $l/libc.a
  	@echo "Start linking Kernel."
! 	@asld -o kernel -s -i -T. $(obj) $l/libc.a $l/end.s >/tmp/syms
  	@echo "Kernel done.  "
  
  clean:
***************
*** 48,53 ****
--- 48,64 ----
  console.s:	tty.h
  console.s:	ttymaps.h
  
+ rs232.s:	const.h type.h $h/const.h $h/type.h
+ rs232.s:	$h/callnr.h
+ rs232.s:	$h/com.h
+ rs232.s:	$h/error.h
+ rs232.s:	$h/sgtty.h
+ rs232.s:	$h/signal.h
+ rs232.s:	glo.h
+ rs232.s:	proc.h
+ rs232.s:	tty.h
+ rs232.s:	ttymaps.h
+ 
  floppy.s:	const.h type.h $h/const.h $h/type.h
  floppy.s:	$h/callnr.h
  floppy.s:	$h/com.h

brucee@runx.ips.oz (Bruce Evans) (10/08/88)

Charles Hedrick writes:

>good line utilization.)  The interrupt level buffer was a surprise.
>The buffer had 100 characters, which at 2400 baud should have lasted
>for almost 1/2 second.  Surely the tty task should get to run within
>1/2 second.  About all I can think is that there's some problem in the
>way communication is being done such that when the floppy starts its
>motor (which takes 1/2 sec, if remember correctly), the tty task gets
>locked out for that period.  Note that this is not an interrupt

It must be the little-discussed FS bottleneck. A user calls FS, FS
calls FLOPPY, and FLOPPY can take up to 3/4 sec waiting for the motor.
Meanwhile FS is blocked. TTY buffers fill up because no user task can
get through FS to unload them. This is a fancy form of busy waiting
and is unacceptable for 1 millisec delays, let alone 750. It will be
difficult to fix. This has been ignored for too long :-(.

>[Politics of getting line wrap into kernel]

Line wrap is essential while running dumb tty style programs, and cleverer
programs should be able to  change it.

>[Fixes for makefile]

Don't forget to fix at_makefile and pc_makefile as well as makefile. All the
duplications should be eliminated by having dependencies for things like
atkernel and xtkernel. No underlines, please. This eliminates wini.c too.

Bruce Evans
Internet: brucee@runx.ips.oz.au    UUCP: uunet!runx.ips.oz.au!brucee