RE: [ECOS] Re: Ctrl-C interrupt problem.

RE: [ECOS] Re: Ctrl-C interrupt problem.

[Fabrice Gautier]

Hi,

Well in fact download speed seems to be fine, i just forgot to set
download-write-size to a good value (i.e.: 0)

But there is still one thing which is slower, that's the O packets. In a
loop with diag_printf's it's very slow, like 1s between each line is
printed.

Thanks
-- 
Fabrice Gautier
fabrice_gautier@sdesigns.com 

> -----Original Message-----
> From: Fabrice Gautier [ mailto:Fabrice_Gautier@sdesigns.com ]
> Sent: Friday, November 17, 2000 12:21 PM
> To: 'Mark Salter'
> Cc: ecos-discuss@sourceware.cygnus.com
> Subject: RE: [ECOS] Re: Ctrl-C interrupt problem.
> 
> 
> > -----Original Message-----
> > From: Mark Salter [ mailto:msalter@redhat.com ]
> > Sent: Friday, November 17, 2000 6:22 AM
> > To: Fabrice_Gautier@sdesigns.com
> > Cc: ecos-discuss@sourceware.cygnus.com
> > Subject: Re: [ECOS] Re: Ctrl-C interrupt problem.
> > 
> > 
> > I'm not aware of such a gdb change, but I haven't been 
> > watching gdb development
> > that closely. I cannot believe that my patch alone would slow 
> > you down as the
> > flush is only done when stepping, continuing, or killing.
> 
> Yes i don't believe it either, it's why I think of some 
> modification in gdb.
> 
> Or may be I badly applied your patch. 
> I'll check this and keep you informed.
> 
> Regards,
> 
> -- 
> Fabrice Gautier
> fabrice_gautier@sdesigns.com 	
> 

RE: [ECOS] Re: Ctrl-C interrupt problem.

[Fabrice Gautier]

> -----Original Message-----
> From: Mark Salter [ mailto:msalter@redhat.com ]
> Sent: Friday, November 17, 2000 6:22 AM
> To: Fabrice_Gautier@sdesigns.com
> Cc: ecos-discuss@sourceware.cygnus.com
> Subject: Re: [ECOS] Re: Ctrl-C interrupt problem.
> 
> 
> I'm not aware of such a gdb change, but I haven't been 
> watching gdb development
> that closely. I cannot believe that my patch alone would slow 
> you down as the
> flush is only done when stepping, continuing, or killing.

Yes i don't believe it either, it's why I think of some modification in gdb.

Or may be I badly applied your patch. 
I'll check this and keep you informed.

Regards,

-- 
Fabrice Gautier
fabrice_gautier@sdesigns.com 	

Re: [ECOS] Re: Ctrl-C interrupt problem.

[Mark Salter]

>>>>> Fabrice Gautier writes:

> It's much better but it's still 5 times slower than before...

> One strange thing is that i cna't exceed 511 bytes/write where before i
> could go up to about 1500. 

> I can't see why download speed would suffer from your patch, the only reason
> I see is some change in GDB beteween 5.0 and cvs. Do you know if there was
> some change ? Some limitation for the 511 bytes/write?

I'm not aware of such a gdb change, but I haven't been watching gdb development
that closely. I cannot believe that my patch alone would slow you down as the
flush is only done when stepping, continuing, or killing.

--Mark

RE: [ECOS] Re: Ctrl-C interrupt problem.

[Fabrice Gautier]

It's much better but it's still 5 times slower than before...
	
One strange thing is that i cna't exceed 511 bytes/write where before i
could go up to about 1500. 

I can't see why download speed would suffer from your patch, the only reason
I see is some change in GDB beteween 5.0 and cvs. Do you know if there was
some change ? Some limitation for the 511 bytes/write?

Thanks
-- 
Fabrice Gautier
fabrice_gautier@sdesigns.com 

Re: [ECOS] Re: Ctrl-C interrupt problem.

[Mark Salter]

(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

RE: [ECOS] Re: Ctrl-C interrupt problem.

[Fabrice Gautier]

> -----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....

> 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.

-- 
Fabrice Gautier
fabrice_gautier@sdesigns.com 

[ECOS] Re: Ctrl-C interrupt problem.

[Fernando Nasser]

Nice catch Fabrice.

Congratulations on your persistence.

Fernando



Mark Salter wrote:
> 
> >>>>> Fabrice Gautier writes:
> 
> > [eCos and RedBoot guys, I think this may concern you
> >  its about a ctrl-C problem which only happen when debugging via ethernet.
> >  full story in the gdb list archives ]
> 
> > I Got it!
> > Well, i hope....
> 
> > And it make sens why it only happens with TCP...
> 
> > The problem is that when gdb send the continue packet to the target, it wait
> > for the target to send the '+' Ack. During this time the SIGINT handler is
> > not set.
> 
> > My guess is that RedBoot intedn to send it BUT (tata) the ethernet driver
> > just wait to have a full packet before really sending! So this happen when a
> > O packet is sent.
> 
> > So i guess the only thing needed in RedBoot is to add a "flush" somewhere.
> > I'm not sure at what level, but i'll look at it tomorrow if nobody does it
> > before.
> 
> > Thanks
> 
> > --
> > Fabrice Gautier
> > fabrice_gautier@sdesigns.com
> 
> 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.
> 
> 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.
> 
> --Mark


-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

[ECOS] Re: Ctrl-C interrupt problem.

[Mark Salter]

>>>>> Fabrice Gautier writes:

> [eCos and RedBoot guys, I think this may concern you 
>  its about a ctrl-C problem which only happen when debugging via ethernet.
>  full story in the gdb list archives ]

> I Got it! 
> Well, i hope....

> And it make sens why it only happens with TCP...

> The problem is that when gdb send the continue packet to the target, it wait
> for the target to send the '+' Ack. During this time the SIGINT handler is
> not set.

> My guess is that RedBoot intedn to send it BUT (tata) the ethernet driver
> just wait to have a full packet before really sending! So this happen when a
> O packet is sent.

> So i guess the only thing needed in RedBoot is to add a "flush" somewhere.
> I'm not sure at what level, but i'll look at it tomorrow if nobody does it
> before.

> Thanks

> -- 
> Fabrice Gautier
> fabrice_gautier@sdesigns.com 


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.

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.

--Mark

[ECOS] Re: Ctrl-C interrupt problem.

[Fernando Nasser]

Fabrice Gautier wrote:
> 
> [eCos and RedBoot guys, I think this may concern you
>  its about a ctrl-C problem which only happen when debugging via ethernet.
>  full story in the gdb list archives ]
> 
> I Got it!
> Well, i hope....
> 
> And it make sens why it only happens with TCP...
> 
> The problem is that when gdb send the continue packet to the target, it wait
> for the target to send the '+' Ack. During this time the SIGINT handler is
> not set.
> 
> My guess is that RedBoot intedn to send it BUT (tata) the ethernet driver
> just wait to have a full packet before really sending! So this happen when a
> O packet is sent.
> 
> So i guess the only thing needed in RedBoot is to add a "flush" somewhere.
> I'm not sure at what level, but i'll look at it tomorrow if nobody does it
> before.
> 

Fabrice, if this is true (and it is a very plausible explanation), you have
caught a very nasty bug that was going to bother many people.

For instance, the GUI presently (with the new sources you got from cvs) is
kept active while the target is running, but not while waiting for an ACK!


-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

[ECOS] RE: Ctrl-C interrupt problem.

[Fabrice Gautier]

[eCos and RedBoot guys, I think this may concern you 
 its about a ctrl-C problem which only happen when debugging via ethernet.
 full story in the gdb list archives ]

I Got it! 
Well, i hope....

And it make sens why it only happens with TCP...

The problem is that when gdb send the continue packet to the target, it wait
for the target to send the '+' Ack. During this time the SIGINT handler is
not set.

My guess is that RedBoot intedn to send it BUT (tata) the ethernet driver
just wait to have a full packet before really sending! So this happen when a
O packet is sent.

So i guess the only thing needed in RedBoot is to add a "flush" somewhere.
I'm not sure at what level, but i'll look at it tomorrow if nobody does it
before.

Thanks

-- 
Fabrice Gautier
fabrice_gautier@sdesigns.com