This is the mail archive of the ecos-discuss@sources.redhat.com mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: Re: Ctrl-C interrupt problem.



(gdb-sources dropped. This is a redboot problem.)

>>>>> Fabrice Gautier writes:

>> -----Original Message-----
>> From: Mark Salter [mailto:msalter@redhat.com]
>> Subject: [ECOS] Re: Ctrl-C interrupt problem.
>> 

>> Yes, it appears that redboot only flushes the output stream when
>> it sees the end of a packet. This causes the problem you see.
>> 
>> Look at .../redboot/current/src/net/net_io.c:net_io_putc().
>> 
>> A quick workaround would be to also flush if a '+' is seen. This
>> has a downside as it may severely impact download speeds, but at
>> least it is functionally correct. YMMV.

> You're right, this is slow as hell....

Here's a description of the problem as I described it in a private
mail. I ran into this issue some time back when I originally wrote
the tcp stack used by RedBoot (written originally for CygMon).

Consider what happens on a download.

          Host               Target

   (1)    data pkt ------->
   (2)             <------     '+'
   (3)             <------     OK pkt
   (4)     '+'     ------->
          data pkt ------->
                     .....

The simplistic tcp stack used by the stub has a very limited number of
buffers and will wait for a tcp segment to be ack'd before sending
another tcp segment. The host side stack is full featured and will
hold off sending a tcp ack to see if it can be piggybacked with a
data segment. So in (1) above, gdb sends the download data, and the
target stack sends the tcp ack. Now comes the performance problem.
Assume as in (2) above, the target sends the + immediately. The
stub will wait for a tcp ack before it can throw away the buffer
used for the '+'. The host side stack will delay milliseconds
hoping to get a data packet to use for the tcp ack of the target's
'+'. That won't happen because gdb is waiting for the 'OK' packet.
After changing the stub to delay sending the '+' until it can be
prepended to the 'OK' packet, I was able to save 10ms or more per
download data packet because the host will send the tcp ack along
with the next download data packet.

>> This needs to be fixed with a flush of the output stream prior
>> to continuing or stepping the target. Its not clear to me right
>> now how this can be done.

> I think the logical way should be to remove the ACK thing from the GDB
> protocol when the transport protocol is reliable and already have an ACK
> thing as TCP.

> However that would not be a minor change.

> Another way would be to add a flush function in the interface functions. As
> a new function entry like read or write, or as an command entry for the ctrl
> function.

I prefer the latter. Care to try this patch?


Index: hal/common/current/include/hal_if.h
===================================================================
RCS file: /home/cvs/ecc/ecc/hal/common/current/include/hal_if.h,v
retrieving revision 1.13
diff -u -p -5 -r1.13 hal_if.h
--- hal/common/current/include/hal_if.h	2000/10/18 09:43:06	1.13
+++ hal/common/current/include/hal_if.h	2000/11/17 01:20:19
@@ -110,10 +110,18 @@ typedef enum {
      * Timeout resolution is in milliseconds.
      *   old_timeout = (*__control)(__COMMCTL_SET_TIMEOUT, 
      *                              cyg_int32 new_timeout);
      */
     __COMMCTL_SET_TIMEOUT,
+
+    /*
+     * Forces driver to send all characters which may be buffered in
+     * the driver. This only flushes the driver buffers, not necessarily
+     * any hardware FIFO, etc.
+     */
+    __COMMCTL_FLUSH_OUTPUT,
+
 } __comm_control_cmd_t;
 
 
 #define CYGNUM_COMM_IF_CH_DATA                    0
 #define CYGNUM_COMM_IF_WRITE                      1
Index: hal/common/current/include/hal_stub.h
===================================================================
RCS file: /home/cvs/ecc/ecc/hal/common/current/include/hal_stub.h,v
retrieving revision 1.15
diff -u -p -5 -r1.15 hal_stub.h
--- hal/common/current/include/hal_stub.h	2000/11/04 21:15:18	1.15
+++ hal/common/current/include/hal_stub.h	2000/11/17 01:20:19
@@ -181,10 +181,13 @@ extern void _breakinst (void);
 
 /* The opcode for a breakpoint instruction.  */
 
 extern unsigned long __break_opcode (void);
 
+/* Function to flush output buffers */
+extern void hal_flush_output(void);
+
 #ifdef CYGDBG_HAL_DEBUG_GDB_BREAK_SUPPORT
 // This one may assume a valid saved interrupt context on some platforms
 extern void cyg_hal_gdb_interrupt    (target_register_t pc);
 // This one does not; use from CRITICAL_IO_REGION macros below.
 extern void cyg_hal_gdb_place_break  (target_register_t pc);
Index: hal/common/current/src/generic-stub.c
===================================================================
RCS file: /home/cvs/ecc/ecc/hal/common/current/src/generic-stub.c,v
retrieving revision 1.39
diff -u -p -5 -r1.39 generic-stub.c
--- hal/common/current/src/generic-stub.c	2000/11/07 01:57:33	1.39
+++ hal/common/current/src/generic-stub.c	2000/11/17 01:20:22
@@ -1264,18 +1264,22 @@ __process_packet (char *packet)
       /* Need to flush the data and instruction cache here, as we may have
          deposited a breakpoint in __single_step. */
 
         __data_cache (CACHE_FLUSH) ;
         __instruction_cache (CACHE_FLUSH) ;
+	hal_flush_output();
 #endif
 
         return -1;
       }
 
       /* kill the program */
     case 'k' :
       __process_exit_vec ();
+#ifdef __ECOS__
+      hal_flush_output();
+#endif
       return 1;
 
     case 'r':           /* Reset */
       __reset ();
       break;
Index: hal/common/current/src/hal_stub.c
===================================================================
RCS file: /home/cvs/ecc/ecc/hal/common/current/src/hal_stub.c,v
retrieving revision 1.52
diff -u -p -5 -r1.52 hal_stub.c
--- hal/common/current/src/hal_stub.c	2000/11/04 21:24:00	1.52
+++ hal/common/current/src/hal_stub.c	2000/11/17 01:20:24
@@ -168,10 +168,21 @@ getDebugChar (void)
 #else
     return HAL_STUB_PLATFORM_GET_CHAR();
 #endif
 }
 
+// Flush output channel
+void
+hal_flush_output(void)
+{
+#ifdef CYGSEM_HAL_VIRTUAL_VECTOR_SUPPORT
+    __call_if_debug_procs_t __debug_procs = CYGACC_CALL_IF_DEBUG_PROCS();
+    CYGACC_COMM_IF_CONTROL(*__debug_procs, __COMMCTL_FLUSH_OUTPUT);
+#endif
+}
+
+
 // Set the baud rate for the current serial port.
 void 
 __set_baud_rate (int baud) 
 {
 #ifdef CYGSEM_HAL_VIRTUAL_VECTOR_SUPPORT
Index: redboot/current/src/net/net_io.c
===================================================================
RCS file: /home/cvs/ecc/ecc/redboot/current/src/net/net_io.c,v
retrieving revision 1.14
diff -u -p -5 -r1.14 net_io.c
--- redboot/current/src/net/net_io.c	2000/11/06 09:39:34	1.14
+++ redboot/current/src/net/net_io.c	2000/11/17 01:20:32
@@ -318,11 +318,15 @@ net_io_control(void *__ch_data, __comm_c
 
         ret = _timeout;
         _timeout = va_arg(ap, cyg_uint32);
 
         va_end(ap);
-    }        
+	break;
+    }
+    case __COMMCTL_FLUSH_OUTPUT:
+        net_io_flush();
+	break;
     default:
         break;
     }
     CYGARC_HAL_RESTORE_GP();
     return ret;



--Mark



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]