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]

Re: [PATCH 2/2] Read memory in multiple lines in dcache_xfer_memory.


On 10/22/2013 07:15 AM, Doug Evans wrote:
> This assert will trigger if the request ends up filling the cache
> and existing lines get pushed out, right?
> [There's a similar assert later in the function too.]
> Could be missing something of course.

I can reproduce it by setting "dcache size" to a small number, 2, for
example, and type commands "up" and "down".

In dcache_xfer_memory, we assume that the cache lines in DCACHE are not
pushed out.  IOW, new cache lines can be added but existing lines can't
be pushed out.  In the updated patch, when adding a new cache line, if
DCACHE is not full, add it, otherwise, save the line in
POSTPONE_INSERT.  When the loop is finished, move lines from
POST_INSERT to DCACHE.  In this way, we don't break the order of
evicting cache lines.

Regression tested on x86_64-linux.  Performance shouldn't be affected.

-- 
Yao (éå)

gdb:

2013-10-22  Yao Qi  <yao@codesourcery.com>

	* dcache.c: Include "memrange.h".
	Update comments.
	(dcache_is_full): New function.
	(dcache_read_line): Remove.
	(dcache_peek_byte): Remove.
	(dcache_ranges_readable): New function.
	(dcache_ranges_uncached): New function.
	(copy_blokc): New function.
	(dcache_xfer_memory): Read multiple cache lines from target
	memory in one time.
---
 gdb/dcache.c |  375 +++++++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 292 insertions(+), 83 deletions(-)

diff --git a/gdb/dcache.c b/gdb/dcache.c
index 316f3dd..a089b46 100644
--- a/gdb/dcache.c
+++ b/gdb/dcache.c
@@ -25,6 +25,7 @@
 #include "target.h"
 #include "inferior.h"
 #include "splay-tree.h"
+#include "memrange.h"
 
 /* Commands with a prefix of `{set,show} dcache'.  */
 static struct cmd_list_element *dcache_set_list = NULL;
@@ -60,8 +61,8 @@ static struct cmd_list_element *dcache_show_list = NULL;
 /* NOTE: Interaction of dcache and memory region attributes
 
    As there is no requirement that memory region attributes be aligned
-   to or be a multiple of the dcache page size, dcache_read_line() and
-   dcache_write_line() must break up the page by memory region.  If a
+   to or be a multiple of the dcache page size, dcache_xfer_memory must
+   break up the page by memory region.  If a
    chunk does not have the cache attribute set, an invalid memory type
    is set, etc., then the chunk is skipped.  Those chunks are handled
    in target_xfer_memory() (or target_xfer_memory_partial()).
@@ -122,8 +123,6 @@ typedef void (block_func) (struct dcache_block *block, void *param);
 
 static struct dcache_block *dcache_hit (DCACHE *dcache, CORE_ADDR addr);
 
-static int dcache_read_line (DCACHE *dcache, struct dcache_block *db);
-
 static struct dcache_block *dcache_alloc (DCACHE *dcache, CORE_ADDR addr);
 
 static void dcache_info (char *exp, int tty);
@@ -305,54 +304,12 @@ dcache_hit (DCACHE *dcache, CORE_ADDR addr)
   return db;
 }
 
-/* Fill a cache line from target memory.
-   The result is 1 for success, 0 if the (entire) cache line
-   wasn't readable.  */
+/* Return true if DCACHE is full.  */
 
 static int
-dcache_read_line (DCACHE *dcache, struct dcache_block *db)
+dcache_is_full (DCACHE *dcache)
 {
-  CORE_ADDR memaddr;
-  gdb_byte *myaddr;
-  int len;
-  int res;
-  int reg_len;
-  struct mem_region *region;
-
-  len = dcache->line_size;
-  memaddr = db->addr;
-  myaddr  = db->data;
-
-  while (len > 0)
-    {
-      /* Don't overrun if this block is right at the end of the region.  */
-      region = lookup_mem_region (memaddr);
-      if (region->hi == 0 || memaddr + len < region->hi)
-	reg_len = len;
-      else
-	reg_len = region->hi - memaddr;
-
-      /* Skip non-readable regions.  The cache attribute can be ignored,
-         since we may be loading this for a stack access.  */
-      if (region->attrib.mode == MEM_WO)
-	{
-	  memaddr += reg_len;
-	  myaddr  += reg_len;
-	  len     -= reg_len;
-	  continue;
-	}
-      
-      res = target_read (&current_target, TARGET_OBJECT_RAW_MEMORY,
-			 NULL, myaddr, memaddr, reg_len);
-      if (res < reg_len)
-	return 0;
-
-      memaddr += res;
-      myaddr += res;
-      len -= res;
-    }
-
-  return 1;
+  return dcache->size >= dcache_size;
 }
 
 /* Get a free cache block, put or keep it on the valid list,
@@ -363,7 +320,7 @@ dcache_alloc (DCACHE *dcache, CORE_ADDR addr)
 {
   struct dcache_block *db;
 
-  if (dcache->size >= dcache_size)
+  if (dcache_is_full (dcache))
     {
       /* Evict the least recently allocated line.  */
       db = dcache->oldest;
@@ -395,28 +352,6 @@ dcache_alloc (DCACHE *dcache, CORE_ADDR addr)
   return db;
 }
 
-/* Using the data cache DCACHE, store in *PTR the contents of the byte at
-   address ADDR in the remote machine.  
-
-   Returns 1 for success, 0 for error.  */
-
-static int
-dcache_peek_byte (DCACHE *dcache, CORE_ADDR addr, gdb_byte *ptr)
-{
-  struct dcache_block *db = dcache_hit (dcache, addr);
-
-  if (!db)
-    {
-      db = dcache_alloc (dcache, addr);
-
-      if (!dcache_read_line (dcache, db))
-         return 0;
-    }
-
-  *ptr = db->data[XFORM (dcache, addr)];
-  return 1;
-}
-
 /* Write the byte at PTR into ADDR in the data cache.
 
    The caller is responsible for also promptly writing the data
@@ -473,6 +408,122 @@ dcache_init (void)
   return dcache;
 }
 
+/* Check the readability of memory range [MEMORY, MEMORY + LEN) and
+   return the readable ranges and caller is responsible to release it.  */
+
+static VEC(mem_range_s) *
+dcache_ranges_readable (CORE_ADDR memaddr, int len)
+{
+  VEC(mem_range_s) *readable_memory = NULL;
+
+  while (len > 0)
+    {
+      struct mem_range *r;
+      int reg_len;
+      /* Don't overrun if this block is right at the end of the region.  */
+      struct mem_region *region = lookup_mem_region (memaddr);
+
+      if (region->hi == 0 || memaddr + len < region->hi)
+	reg_len = len;
+      else
+	reg_len = region->hi - memaddr;
+
+      /* Skip non-readable regions.  The cache attribute can be ignored,
+	 since we may be loading this for a stack access.  */
+      if (region->attrib.mode == MEM_WO)
+	{
+	  memaddr += reg_len;
+	  len -= reg_len;
+	  continue;
+	}
+
+      r = VEC_safe_push (mem_range_s, readable_memory, NULL);
+      r->start = memaddr;
+      r->length = reg_len;
+
+      memaddr += reg_len;
+      len -= reg_len;
+    }
+
+  return readable_memory;
+}
+
+/* Return the uncached ranges from RANGES.   */
+
+static VEC(mem_range_s) *
+dcache_ranges_uncached (DCACHE *dcache, VEC(mem_range_s) *ranges)
+{
+  int b;
+  struct mem_range *rb;
+  VEC(mem_range_s) *uncached = NULL;
+
+  for (b = 0; VEC_iterate (mem_range_s, ranges, b, rb); b++)
+    {
+      CORE_ADDR memaddr_start = rb->start;
+      CORE_ADDR memaddr_end = rb->start;
+
+      while (memaddr_end < rb->start + rb->length)
+	{
+	  struct dcache_block *db = dcache_hit (dcache, memaddr_end);
+
+	  if (db != NULL)
+	    {
+	      /* Set MEMADDR_END to the start address of this cache line.  */
+	      memaddr_end = align_down (memaddr_end, dcache->line_size);
+
+	      if (memaddr_end > memaddr_start)
+		{
+		  struct mem_range *r;
+
+		  r = VEC_safe_push (mem_range_s, uncached, NULL);
+		  r->start = memaddr_start;
+		  r->length = memaddr_end - memaddr_start;
+		}
+	    }
+
+	  /* Increase memaddr_end to a dcache->line_size-aligned value.  */
+	  if (memaddr_end < align_up (memaddr_end, dcache->line_size))
+	    memaddr_end = align_up (memaddr_end, dcache->line_size);
+	  else
+	    memaddr_end += dcache->line_size;
+
+	  if (db != NULL)
+	    memaddr_start = memaddr_end;
+	}
+
+      if (memaddr_end > rb->start + rb->length)
+	memaddr_end = rb->start + rb->length;
+
+      if (memaddr_start < memaddr_end)
+	{
+	  struct mem_range *r;
+
+	  r = VEC_safe_push (mem_range_s, uncached, NULL);
+
+	  r->start = memaddr_start;
+	  r->length = memaddr_end - memaddr_start;
+	}
+    }
+
+  return uncached;
+}
+
+/* Helper function to copy BLOCK to dcache represented by PARAM.  */
+
+static void
+copy_block (struct dcache_block *block, void *param)
+{
+  DCACHE *dcache = param;
+  struct dcache_block *db = dcache_hit (dcache, block->addr);
+
+  /* This function is only used when DCACHE is full and BLOCK doesn't
+     exist in DCACHE.  */
+  gdb_assert (dcache_is_full (dcache));
+  gdb_assert (db == NULL);
+
+  db = dcache_alloc (dcache, block->addr);
+  memcpy (db->data, block->data, dcache->line_size);
+}
 
 /* Read or write LEN bytes from inferior memory at MEMADDR, transferring
    to or from debugger address MYADDR.  Write to inferior if SHOULD_WRITE is
@@ -489,9 +540,6 @@ dcache_xfer_memory (struct target_ops *ops, DCACHE *dcache,
 		    CORE_ADDR memaddr, gdb_byte *myaddr,
 		    int len, int should_write)
 {
-  int i;
-  int res;
-
   /* If this is a different inferior from what we've recorded,
      flush the cache.  */
 
@@ -506,8 +554,10 @@ dcache_xfer_memory (struct target_ops *ops, DCACHE *dcache,
 
   if (should_write)
     {
-      res = target_write (ops, TARGET_OBJECT_RAW_MEMORY,
-			  NULL, myaddr, memaddr, len);
+      int res = target_write (ops, TARGET_OBJECT_RAW_MEMORY,
+			      NULL, myaddr, memaddr, len);
+      int i;
+
       if (res <= 0)
 	return res;
       /* Update LEN to what was actually written.  */
@@ -527,16 +577,175 @@ dcache_xfer_memory (struct target_ops *ops, DCACHE *dcache,
     }
   else
     {
-      for (i = 0; i < len; i++)
+      int i;
+      struct mem_range *r;
+      /* The starting address of each cached range.  */
+      CORE_ADDR cached_addr = memaddr;
+      /* A list of cache blocks should be postponed inserting to
+	 dcache.  */
+      struct dcache_block *postpone_insert = NULL;
+
+      VEC(mem_range_s) *memory;
+      VEC(mem_range_s) *uncached = NULL;
+
+      /* Find readable ranges in range [MEMADDR, MEMADDR + LEN),
+	 supposing write-only regions are wo1 and wo2.  Then,
+	 readable ranges are r1, r2 and r3.
+
+	 MEMADDR                               MEMADDR + LEN
+	 |<------------------------------------------------->|
+		|<-- wo1 -->|        |<-- wo2 -->|
+
+	 |<-r1->|           |<--r2-->|           |<---r3---->|  */
+      memory = dcache_ranges_readable (memaddr, len);
+
+      /* GDB will read from these three readable ranges, r1, r2 and r3.
+	 GDB has to check the corresponding cache lines' state (cached
+	 or uncached) to determine whether to read from the target
+	 memory or the cache lines.
+
+	 MEMADDR                               MEMADDR + LEN
+	 |<------------------------------------------------->|
+		|<-- wo1 -->|        |<-- wo2 -->|
+
+	 |<-r1->|           |<--r2-->|           |<---r3---->|
+
+	 -u-|-----c----|-----u----|-----c----|-----c----|--u--
+	 'u' stands for unchaced 'c' stands for cached.
+
+	 |u1|-c1-|          |  u2 |c2|           |--c3--| u3  |
+
+	 Uncached ranges are u1, u2 and u3, and cached ranges are c1,
+	 c2 and c3.  */
+      uncached = dcache_ranges_uncached (dcache, memory);
+
+      VEC_free (mem_range_s, memory);
+
+      /* Iterate each uncached range.  Read memory from cache lines if
+	 memory address is not within the uncached range, otherwise, read
+	 from the target memory and update corresponding cache lines.  */
+
+      for (i = 0; VEC_iterate (mem_range_s, uncached, i, r); i++)
 	{
-	  if (!dcache_peek_byte (dcache, memaddr + i, myaddr + i))
+	  int j;
+
+	  if (cached_addr < r->start)
 	    {
-	      /* That failed.  Discard its cache line so we don't have a
-		 partially read line.  */
-	      dcache_invalidate_line (dcache, memaddr + i);
-	      return i;
+	      /* Read memory [cached_addr, MIN (r->start, MEMADDR + LEN))
+		 from cache lines.  */
+
+	      for (; cached_addr < r->start && cached_addr < (memaddr + len);
+		   cached_addr++)
+		{
+		  struct dcache_block *db = dcache_hit (dcache, cached_addr);
+
+		  gdb_assert (db != NULL);
+
+		  myaddr[cached_addr - memaddr]
+		    = db->data[XFORM (dcache, cached_addr)];
+		}
+	    }
+	  cached_addr = r->start + r->length;
+
+	  /* Part of the memory range [MEMADDR, MEMADDR + LEN) is
+	     not cached.  */
+	  if (r->start < len + memaddr)
+	    {
+	      /* MEMADDR_START and MEMADDR_END are aligned on
+		 dcache->line_size, because dcache->line_size is the
+		 minimal unit to update cache and fetch from the target
+		 memory.  */
+	      CORE_ADDR memaddr_start
+		= align_down (r->start, dcache->line_size);
+	      CORE_ADDR memaddr_end
+		= align_up (r->start + r->length, dcache->line_size);
+	      int res;
+	      int len1 = memaddr_end - memaddr_start;
+	      int len2;
+	      gdb_byte *buf = xmalloc (len1);
+
+	      /* Read multiple cache lines to cover memory range
+		 [r->start, r->start + MIN (r->length,
+		 LEN + MEMADDR - r->start)) from target.  */
+
+	      res = target_read (&current_target, TARGET_OBJECT_RAW_MEMORY,
+				 NULL, buf, memaddr_start, len1);
+
+	      if (res == -1)
+		{
+		  VEC_free (mem_range_s, uncached);
+		  xfree (buf);
+		  return r->start - memaddr;
+		}
+
+	      /* Copy contents to MYADDR.  */
+	      len2 = r->length;
+	      if (len2 > len + memaddr - r->start)
+		len2 = len + memaddr - r->start;
+
+	      memcpy ((r->start - memaddr) + myaddr,
+		      buf + (r->start - memaddr_start),
+		      len2);
+
+	      /* Update cache lines in range
+		 [MEMADDR_START, MEMADDR_START + LEN1).  */
+	      for (j = 0; j < (len1 / dcache->line_size); j++)
+		{
+		  struct dcache_block *db = NULL;
+
+		  if (dcache_is_full (dcache))
+		    {
+		      /* DCACHE is full, don't put dcache_block in it,
+			 otherwise one dcache_block is evicted, which
+			 may be used later in the loop.  Save the
+			 dcache_block temporarily in POSTPONE_INSERT
+			 and use it to update DCACHE out side the
+			 loop.  */
+		      db = xmalloc (offsetof (struct dcache_block, data)
+				    + dcache->line_size);
+		      db->addr = MASK (dcache,
+				       memaddr_start + j * dcache->line_size);
+		      append_block (&postpone_insert, db);
+		    }
+		  else
+		    {
+		      /* DCACHE is not full, it is OK to put dcache_block in
+			 it.  */
+		      db = dcache_hit (dcache, memaddr_start + j * dcache->line_size);
+
+		      gdb_assert (db == NULL);
+
+		      db = dcache_alloc (dcache, memaddr_start + j * dcache->line_size);
+		    }
+
+		  memcpy (db->data, &buf[j * dcache->line_size], dcache->line_size);
+		}
+
+	      xfree (buf);
+
+	      if (res < len1)
+		{
+		  VEC_free (mem_range_s, uncached);
+		  return r->start - memaddr + res;
+		}
 	    }
 	}
+
+      VEC_free (mem_range_s, uncached);
+
+      for (; cached_addr < (memaddr + len); cached_addr++)
+	{
+	  struct dcache_block *db = dcache_hit (dcache, cached_addr);
+
+	  gdb_assert (db != NULL);
+
+	  myaddr[cached_addr - memaddr]
+	    = db->data[XFORM (dcache, cached_addr)];
+	}
+
+      /* Copy dcache_blocks in POSTPONE_INSERT to DCACHE.  */
+      for_each_block (&postpone_insert, copy_block, dcache);
+      for_each_block (&postpone_insert, free_block, NULL);
     }
 
   return len;
-- 
1.7.7.6


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