This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[rfc] Attempt to align writes for remote targets


This is a minor optimization we've had kicking around for a couple of weeks.
One of our customers pointed out that it's more friendly to remote targets
to offer them writes with some minimal alignment, because many hardware
debug interfaces have limited ability to do writes of less than some length
(usually a word; I picked something larger just to be cautious).  There's
an extra read-modify-write cycle which can be quite slow.

If we're going to do a big write across a lot of packets, e.g. for "load",
it would be nice if we didn't incur that extra legwork for every single
packet.  So instead of truncating packets wherever they happen to fall,
for big packets, attempt to round off to a "nice" boundary.

This is just an opportunistic improvement; the stub still needs to support
unaligned writes.  But it seems reasonable to help out where we can.  I'll
plan to commit this after my qXfer patches; it doesn't actually depend on
them, but I've only tested them together.

-- 
Daniel Jacobowitz
CodeSourcery

2006-07-05  Daniel Jacobowitz  <dan@codesourcery.com>
	    Nathan Sidwell  <nathan@codesourcery.com>

	* remote.c (REMOTE_ALIGN_WRITES): New.
	(remote_write_bytes): Align large write packets.  Remove unused
	payload_start variable.

Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c	2006-07-05 14:01:44.000000000 -0400
+++ src/gdb/remote.c	2006-07-05 14:52:22.000000000 -0400
@@ -60,6 +60,18 @@
 
 #include "remote-fileio.h"
 
+/* The size to align memory write packets, when practical.  The protocol
+   does not guarantee any alignment, and gdb will generate short
+   writes and unaligned writes, but even as a best-effort attempt this
+   can improve bulk transfers.  For instance, if a write is misaligned
+   relative to the target's data bus, the stub may need to make an extra
+   round trip fetching data from the target.  This doesn't make a
+   huge difference, but it's easy to do, so we try to be helpful.
+
+   The alignment chosen is arbitrary; usually data bus width is
+   important here, not the possibly larger cache line size.  */
+enum { REMOTE_ALIGN_WRITES = 16 };
+
 /* Prototypes for local functions.  */
 static void cleanup_sigint_signal_handler (void *dummy);
 static void initialize_sigint_signal_handler (void);
@@ -3851,7 +3863,7 @@ remote_write_bytes (CORE_ADDR memaddr, g
   int todo;
   int nr_bytes;
   int payload_size;
-  char *payload_start;
+  int payload_length;
 
   /* Verify that the target can support a binary download.  */
   check_binary_download (memaddr);
@@ -3899,6 +3911,11 @@ remote_write_bytes (CORE_ADDR memaddr, g
     internal_error (__FILE__, __LINE__,
 		    _("minumum packet size too small to write data"));
 
+  /* If we already need another packet, then try to align the end
+     of this packet to a useful boundary.  */
+  if (todo > 2 * REMOTE_ALIGN_WRITES && todo < len)
+    todo = ((memaddr + todo) & ~(REMOTE_ALIGN_WRITES - 1)) - memaddr;
+
   /* Append "<memaddr>".  */
   memaddr = remote_address_masked (memaddr);
   p += hexnumstr (p, (ULONGEST) memaddr);
@@ -3917,14 +3934,30 @@ remote_write_bytes (CORE_ADDR memaddr, g
   *p = '\0';
 
   /* Append the packet body.  */
-  payload_start = p;
   switch (remote_protocol_packets[PACKET_X].support)
     {
     case PACKET_ENABLE:
       /* Binary mode.  Send target system values byte by byte, in
 	 increasing byte addresses.  Only escape certain critical
 	 characters.  */
-      p += remote_escape_output (myaddr, todo, p, &nr_bytes, payload_size);
+      payload_length = remote_escape_output (myaddr, todo, p, &nr_bytes,
+					     payload_size);
+
+      /* If not all TODO bytes fit, then we'll need another packet.  Make
+	 a second try to keep the end of the packet aligned.  */
+      if (nr_bytes < todo)
+	{
+	  int new_nr_bytes;
+
+	  new_nr_bytes = (((memaddr + nr_bytes) & ~(REMOTE_ALIGN_WRITES - 1))
+			  - memaddr);
+	  if (new_nr_bytes != nr_bytes)
+	    payload_length = remote_escape_output (myaddr, new_nr_bytes,
+						   p, &nr_bytes,
+						   payload_size);
+	}
+
+      p += payload_length;
       if (nr_bytes < todo)
 	{
 	  /* Escape chars have filled up the buffer prematurely,


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