[PATCH 00/10 V2] Cache code access for disassemble

[PATCH 00/10 V2] Cache code access for disassemble

[Yao Qi]

Hi,
This is the V2 of this patch series.  In the discussion of V1, the
major comments are:

 1. dcache.c and target_dcache needs some cleanups and factor.
 2. commands "set dcache size" and "set dcahce line-size" needs a
    description on semantics when dcache is per-address_space.
 3. Use a single global 'target_dcache' for both stack caching and
    code caching.
 4. Replace "executable code" with "code segment" in doc and NEWS.

V2 addresses these comments as follows:

Patch #1 ~ #4 are about cleanups and factors to dcache.c and
target_dcache.  Patch #5 is a minor optimization to target_dcache.
Patch #6 and #7 change target_dcache to per-address_space mode, and
update doc on dcache-related commands.

We decide to use single 'target_dcache' for caching different types
of access, so cache invalidation caused by option "stack-cache"
changes makes troubles.  Think a little, I find we don't have to
invalidate target_dcache when option "stack-cache" is changed.  This
is what patch #8 does, and looks change in patch #8 doesn't affect
the semantics of option "set stack-cache".

Patch #9 adds command "set code-cache", similar to "set stack-cache", and
patch #10 is to use TARGET_OBJECT_CODE_MEMORY to read for disassembly.

Patch 4, 5, 7, and 9 need doc review.

Each patch is regression tested on x86_64-linux.

*** BLURB HERE ***

Yao Qi (10):
  Remove last_cache
  Don't update target_dcache if it is not initialized
  Move target-dcache out of target.c
  Don't stress 'remote' in "Data Caching" in doc
  Invalidate or shrink dcache when setting is changed.
  Add REGISTRY for struct address_space.
  Associate target_dcache to address_space.
  Don't invalidate dcache when option stack-cache is changed
  set/show code-cache
  Use target_read_code in disassemble.

 gdb/Makefile.in     |    6 +-
 gdb/NEWS            |    6 ++
 gdb/dcache.c        |   93 ++++++++++++++++++---------
 gdb/dcache.h        |    4 +
 gdb/disasm.c        |    2 +-
 gdb/doc/gdb.texinfo |   52 ++++++++++-----
 gdb/memattr.c       |    1 +
 gdb/progspace.c     |   14 ++---
 gdb/progspace.h     |   19 +++++-
 gdb/target-dcache.c |  175 +++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/target-dcache.h |   30 +++++++++
 gdb/target.c        |   94 +++++++++-------------------
 gdb/target.h        |    9 ++-
 gdb/top.c           |    1 +
 gdb/tracepoint.c    |    1 +
 15 files changed, 379 insertions(+), 128 deletions(-)
 create mode 100644 gdb/target-dcache.c
 create mode 100644 gdb/target-dcache.h

-- 
1.7.7.6

[PATCH 02/10] Don't update target_dcache if it is not initialized

[Yao Qi]

After previous patch, 'target_dcache' is initialized lazily.  It is
possible that 'target_dcache' is still NULL when GDB writes to memory.
In this case, update to 'target_dcache' can be skipped.

gdb:

2013-11-02  Yao Qi  <yao@codesourcery.com>

	* target.c (memory_xfer_partial_1): Update 'target_dcache' if
	it is initialized.
---
 gdb/target.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/gdb/target.c b/gdb/target.c
index 6403e86..1a076f3 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1667,11 +1667,12 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
   if (res > 0
       && inf != NULL
       && writebuf != NULL
+      && target_dcache_init_p ()
       && !region->attrib.cache
       && stack_cache_enabled_p
       && object != TARGET_OBJECT_STACK_MEMORY)
     {
-      DCACHE *dcache = target_dcache_get_or_init ();
+      DCACHE *dcache = target_dcache_get ();
 
       dcache_update (dcache, memaddr, (void *) writebuf, res);
     }
-- 
1.7.7.6

Re: [PATCH 02/10] Don't update target_dcache if it is not initialized

[Doug Evans]

On Sat, Nov 2, 2013 at 10:54 PM, Yao Qi <yao@codesourcery.com> wrote:
> After previous patch, 'target_dcache' is initialized lazily.  It is
> possible that 'target_dcache' is still NULL when GDB writes to memory.
> In this case, update to 'target_dcache' can be skipped.
>
> gdb:
>
> 2013-11-02  Yao Qi  <yao@codesourcery.com>
>
>         * target.c (memory_xfer_partial_1): Update 'target_dcache' if
>         it is initialized.

Ok.

[PATCH 03/10] Move target-dcache out of target.c

[Yao Qi]

This patch moves target_dcache related code out of target.c.

gdb:

2013-11-02  Yao Qi  <yao@codesourcery.com>

	* Makefile.in (SFILES):Add target-dcache.c.
	(HFILES_NO_SRCDIR): Add target-dcache.h.
	(COMMON_OBS): Add target-dcache.o.
	* dcache.c: Remove inclusion to "target.h".  Include
	"target-dcache.h".
	* memattr.c: Include "target-dcache.h".
	* top.c: Likewise.
	* tracepoint.c: Likewise.
	* target.c: (stack_cache_enabled_p_1): Move to
	target-dcache.c.
	(stack_cache_enabled_p): Likewise.
	(set_stack_cache_enabled_p): Likewise.
	(show_stack_cache_enabled_p): Likewise.
	(target_dcache, target_dcache_init_p): Likewise.
	(target_dcache_invalidate): Likewise.
	(target_dcache_get, target_dcache_get_or_init): Likewise.
	(memory_xfer_partial_1): Call function stack_cache_enabled.
	(initialize_target): Move code to target-dcache.c.
	* target.h (target_dcache_invalidate): Move to
	target-dcache.h.
	(target_dcache_get): Likewise.
	* target-dcache.c: New.
	* target-dcahce.h: New.
---
 gdb/Makefile.in     |    6 +-
 gdb/dcache.c        |    2 +-
 gdb/memattr.c       |    1 +
 gdb/target-dcache.c |  116 +++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/target-dcache.h |   28 ++++++++++++
 gdb/target.c        |   86 +------------------------------------
 gdb/target.h        |    5 --
 gdb/top.c           |    1 +
 gdb/tracepoint.c    |    1 +
 9 files changed, 154 insertions(+), 92 deletions(-)
 create mode 100644 gdb/target-dcache.c
 create mode 100644 gdb/target-dcache.h

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index a9b3c64..25d2448 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -760,7 +760,7 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
 	solib.c solib-target.c source.c \
 	stabsread.c stack.c probe.c stap-probe.c std-regs.c \
 	symfile.c symfile-debug.c symfile-mem.c symmisc.c symtab.c \
-	target.c target-descriptions.c target-memory.c \
+	target.c target-dcache.c target-descriptions.c target-memory.c \
 	thread.c top.c tracepoint.c \
 	trad-frame.c \
 	tramp-frame.c \
@@ -812,7 +812,7 @@ gdb_curses.h bfd-target.h memattr.h inferior.h ax.h dummy-frame.h \
 inflow.h fbsd-nat.h ia64-libunwind-tdep.h completer.h inf-ttrace.h \
 solib-target.h gdb_vfork.h alpha-tdep.h dwarf2expr.h \
 m2-lang.h stack.h charset.h cleanups.h addrmap.h command.h solist.h source.h \
-target.h prologue-value.h cp-abi.h tui/tui-hooks.h tui/tui.h \
+target.h target-dcache.h prologue-value.h cp-abi.h tui/tui-hooks.h tui/tui.h \
 tui/tui-file.h tui/tui-command.h tui/tui-disasm.h tui/tui-wingeneral.h \
 tui/tui-windata.h tui/tui-data.h tui/tui-win.h tui/tui-stack.h \
 tui/tui-winsource.h tui/tui-regs.h tui/tui-io.h tui/tui-layout.h \
@@ -915,7 +915,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
 	event-loop.o event-top.o inf-loop.o completer.o \
 	gdbarch.o arch-utils.o gdbtypes.o gdb_bfd.o gdb_obstack.o \
 	osabi.o copying.o \
-	memattr.o mem-break.o target.o parse.o language.o \
+	memattr.o mem-break.o target.o target-dcache.o parse.o language.o \
 	build-id.o buildsym.o \
 	findcmd.o \
 	std-regs.o \
diff --git a/gdb/dcache.c b/gdb/dcache.c
index f7a9c63..5b32629 100644
--- a/gdb/dcache.c
+++ b/gdb/dcache.c
@@ -22,7 +22,7 @@
 #include "gdbcmd.h"
 #include "gdb_string.h"
 #include "gdbcore.h"
-#include "target.h"
+#include "target-dcache.h"
 #include "inferior.h"
 #include "splay-tree.h"
 
diff --git a/gdb/memattr.c b/gdb/memattr.c
index 5c2adaa..ac1901d 100644
--- a/gdb/memattr.c
+++ b/gdb/memattr.c
@@ -22,6 +22,7 @@
 #include "gdbcmd.h"
 #include "memattr.h"
 #include "target.h"
+#include "target-dcache.h"
 #include "value.h"
 #include "language.h"
 #include "vec.h"
diff --git a/gdb/target-dcache.c b/gdb/target-dcache.c
new file mode 100644
index 0000000..bf04679
--- /dev/null
+++ b/gdb/target-dcache.c
@@ -0,0 +1,116 @@
+/* Copyright (C) 1992-2013 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "target-dcache.h"
+#include "gdbcmd.h"
+
+/* Cache of memory operations, to speed up remote access.  */
+static DCACHE *target_dcache;
+
+/* Target dcache is initialized or not.  */
+
+int
+target_dcache_init_p (void)
+{
+  return (target_dcache != NULL);
+}
+
+/* Invalidate the target dcache.  */
+
+void
+target_dcache_invalidate (void)
+{
+  if (target_dcache_init_p ())
+    dcache_invalidate (target_dcache);
+}
+
+/* Return the target dcache.  Return NULL if target dcache is not
+   initialized yet.  */
+
+DCACHE *
+target_dcache_get (void)
+{
+  return target_dcache;
+}
+
+/* Return the target dcache.  If it is not initialized yet, initialize
+   it.  */
+
+DCACHE *
+target_dcache_get_or_init (void)
+{
+  if (!target_dcache_init_p ())
+    target_dcache = dcache_init ();
+
+  return target_dcache;
+}
+
+/* The option sets this.  */
+static int stack_cache_enabled_p_1 = 1;
+/* And set_stack_cache_enabled_p updates this.
+   The reason for the separation is so that we don't flush the cache for
+   on->on transitions.  */
+static int stack_cache_enabled_p = 1;
+
+/* This is called *after* the stack-cache has been set.
+   Flush the cache for off->on and on->off transitions.
+   There's no real need to flush the cache for on->off transitions,
+   except cleanliness.  */
+
+static void
+set_stack_cache_enabled_p (char *args, int from_tty,
+			   struct cmd_list_element *c)
+{
+  if (stack_cache_enabled_p != stack_cache_enabled_p_1)
+    target_dcache_invalidate ();
+
+  stack_cache_enabled_p = stack_cache_enabled_p_1;
+}
+
+static void
+show_stack_cache_enabled_p (struct ui_file *file, int from_tty,
+			    struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("Cache use for stack accesses is %s.\n"), value);
+}
+
+/* Return true if "stack cache" is enabled, otherwise, return false.  */
+
+int
+stack_cache_enabled (void)
+{
+  return stack_cache_enabled_p;
+}
+
+/* -Wmissing-prototypes */
+extern initialize_file_ftype _initialize_target_dcache;
+
+void
+_initialize_target_dcache (void)
+{
+  add_setshow_boolean_cmd ("stack-cache", class_support,
+			   &stack_cache_enabled_p_1, _("\
+Set cache use for stack access."), _("\
+Show cache use for stack access."), _("\
+When on, use the data cache for all stack access, regardless of any\n\
+configured memory regions.  This improves remote performance significantly.\n\
+By default, caching for stack access is on."),
+			   set_stack_cache_enabled_p,
+			   show_stack_cache_enabled_p,
+			   &setlist, &showlist);
+}
diff --git a/gdb/target-dcache.h b/gdb/target-dcache.h
new file mode 100644
index 0000000..06a938f
--- /dev/null
+++ b/gdb/target-dcache.h
@@ -0,0 +1,28 @@
+/* Copyright (C) 1992-2013 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "dcache.h"
+
+extern void target_dcache_invalidate (void);
+
+extern DCACHE *target_dcache_get (void);
+
+extern DCACHE *target_dcache_get_or_init (void);
+
+extern int target_dcache_init_p (void);
+
+extern int stack_cache_enabled (void);
diff --git a/gdb/target.c b/gdb/target.c
index 1a076f3..1212347 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -23,6 +23,7 @@
 #include <errno.h>
 #include "gdb_string.h"
 #include "target.h"
+#include "target-dcache.h"
 #include "gdbcmd.h"
 #include "symtab.h"
 #include "inferior.h"
@@ -206,76 +207,6 @@ show_targetdebug (struct ui_file *file, int from_tty,
 
 static void setup_target_debug (void);
 
-/* The option sets this.  */
-static int stack_cache_enabled_p_1 = 1;
-/* And set_stack_cache_enabled_p updates this.
-   The reason for the separation is so that we don't flush the cache for
-   on->on transitions.  */
-static int stack_cache_enabled_p = 1;
-
-/* This is called *after* the stack-cache has been set.
-   Flush the cache for off->on and on->off transitions.
-   There's no real need to flush the cache for on->off transitions,
-   except cleanliness.  */
-
-static void
-set_stack_cache_enabled_p (char *args, int from_tty,
-			   struct cmd_list_element *c)
-{
-  if (stack_cache_enabled_p != stack_cache_enabled_p_1)
-    target_dcache_invalidate ();
-
-  stack_cache_enabled_p = stack_cache_enabled_p_1;
-}
-
-static void
-show_stack_cache_enabled_p (struct ui_file *file, int from_tty,
-			    struct cmd_list_element *c, const char *value)
-{
-  fprintf_filtered (file, _("Cache use for stack accesses is %s.\n"), value);
-}
-
-/* Cache of memory operations, to speed up remote access.  */
-static DCACHE *target_dcache;
-
-/* Target dcache is initialized or not.  */
-
-static int
-target_dcache_init_p (void)
-{
-  return (target_dcache != NULL);
-}
-
-/* Invalidate the target dcache.  */
-
-void
-target_dcache_invalidate (void)
-{
-  if (target_dcache_init_p ())
-    dcache_invalidate (target_dcache);
-}
-
-/* Return the target dcache.  Return NULL if target dcache is not
-   initialized yet.  */
-
-DCACHE *
-target_dcache_get (void)
-{
-  return target_dcache;
-}
-
-/* Return the target dcache.  If it is not initialized yet, initialize
-   it.  */
-
-static DCACHE *
-target_dcache_get_or_init (void)
-{
-  if (!target_dcache_init_p ())
-    target_dcache = dcache_init ();
-
-  return target_dcache;
-}
-
 /* The user just typed 'target' without the name of a target.  */
 
 static void
@@ -1616,7 +1547,7 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
 	 the collected memory range fails.  */
       && get_traceframe_number () == -1
       && (region->attrib.cache
-	  || (stack_cache_enabled_p && object == TARGET_OBJECT_STACK_MEMORY)))
+	  || (stack_cache_enabled () && object == TARGET_OBJECT_STACK_MEMORY)))
     {
       DCACHE *dcache = target_dcache_get_or_init ();
 
@@ -1669,7 +1600,7 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
       && writebuf != NULL
       && target_dcache_init_p ()
       && !region->attrib.cache
-      && stack_cache_enabled_p
+      && stack_cache_enabled ()
       && object != TARGET_OBJECT_STACK_MEMORY)
     {
       DCACHE *dcache = target_dcache_get ();
@@ -5155,17 +5086,6 @@ Tells gdb whether to control the inferior in asynchronous mode."),
 			   &setlist,
 			   &showlist);
 
-  add_setshow_boolean_cmd ("stack-cache", class_support,
-			   &stack_cache_enabled_p_1, _("\
-Set cache use for stack access."), _("\
-Show cache use for stack access."), _("\
-When on, use the data cache for all stack access, regardless of any\n\
-configured memory regions.  This improves remote performance significantly.\n\
-By default, caching for stack access is on."),
-			   set_stack_cache_enabled_p,
-			   show_stack_cache_enabled_p,
-			   &setlist, &showlist);
-
   add_setshow_boolean_cmd ("may-write-registers", class_support,
 			   &may_write_registers_1, _("\
 Set permission to write into registers."), _("\
diff --git a/gdb/target.h b/gdb/target.h
index 4cff2aa..7a93646 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1043,11 +1043,6 @@ int target_supports_disable_randomization (void);
 #define target_can_run_breakpoint_commands() \
   (*current_target.to_can_run_breakpoint_commands) ()
 
-/* Invalidate all target dcaches.  */
-extern void target_dcache_invalidate (void);
-
-extern struct dcache_struct *target_dcache_get (void);
-
 extern int target_read_string (CORE_ADDR, char **, int, int *);
 
 extern int target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr,
diff --git a/gdb/top.c b/gdb/top.c
index c473d8c..b065199 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -28,6 +28,7 @@
 #include "exceptions.h"
 #include <signal.h>
 #include "target.h"
+#include "target-dcache.h"
 #include "breakpoint.h"
 #include "gdbtypes.h"
 #include "expression.h"
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index a7ccb50..b6fd059 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -26,6 +26,7 @@
 #include "gdbcmd.h"
 #include "value.h"
 #include "target.h"
+#include "target-dcache.h"
 #include "language.h"
 #include "gdb_string.h"
 #include "inferior.h"
-- 
1.7.7.6

Re: [PATCH 03/10] Move target-dcache out of target.c

[Doug Evans]

On Sat, Nov 2, 2013 at 10:54 PM, Yao Qi <yao@codesourcery.com> wrote:
> This patch moves target_dcache related code out of target.c.
>
> gdb:
>
> 2013-11-02  Yao Qi  <yao@codesourcery.com>
>
>         * Makefile.in (SFILES):Add target-dcache.c.
>         (HFILES_NO_SRCDIR): Add target-dcache.h.
>         (COMMON_OBS): Add target-dcache.o.
>         * dcache.c: Remove inclusion to "target.h".  Include
>         "target-dcache.h".
>         * memattr.c: Include "target-dcache.h".
>         * top.c: Likewise.
>         * tracepoint.c: Likewise.
>         * target.c: (stack_cache_enabled_p_1): Move to
>         target-dcache.c.
>         (stack_cache_enabled_p): Likewise.
>         (set_stack_cache_enabled_p): Likewise.
>         (show_stack_cache_enabled_p): Likewise.
>         (target_dcache, target_dcache_init_p): Likewise.
>         (target_dcache_invalidate): Likewise.
>         (target_dcache_get, target_dcache_get_or_init): Likewise.
>         (memory_xfer_partial_1): Call function stack_cache_enabled.
>         (initialize_target): Move code to target-dcache.c.
>         * target.h (target_dcache_invalidate): Move to
>         target-dcache.h.
>         (target_dcache_get): Likewise.
>         * target-dcache.c: New.
>         * target-dcahce.h: New.

Typo in changelog entry for target-dcache.h.

Plus target-dcache.h needs #ifndef TARGET_DCACHE_H/etc.

Ok with those changes.

[For a later cleanup I would rename stack_cache_enabled() to
stack_cache_enabled_p(), and rename the underlying variables of
course.  I wouldn't do that in this patch of course since it's good to
keep the moving of code in a patch onto itself.]

Re: [PATCH 03/10] Move target-dcache out of target.c

[Pedro Alves]

On 11/17/2013 08:09 PM, Doug Evans wrote:
> [For a later cleanup I would rename stack_cache_enabled() to
> stack_cache_enabled_p(), and rename the underlying variables of
> course.  I wouldn't do that in this patch of course since it's good to
> keep the moving of code in a patch onto itself.]

FWIW, _p is necessary to disambiguate cases like:

  if (enable_stack_cache ())

Because that's ambiguous --- it can be read as either
a predicate (is it enabled?), or an action (do enable now).

To fix that, you'd usually either rename to:

  if (enable_stack_cache_p ())

or

  if (stack_cache_enabled ())

"enabled" is already clearly a predicate.

-- 
Pedro Alves

[PATCH 05/10] Invalidate or shrink dcache when setting is changed.

[Yao Qi]

Nowadays, when cache size or line size is changed by command,
'target_dcache' is invalidated.  It is too conservative.  We can
optimize in the following ways,

 - Don't have to invalidate dcache immediately after cache size or
   line size is changed.  We can postpone the invalidation to the moment
   using 'target_dcache'.
 - Don't have to invalidate dcache if the cache size is changed.  If
   cache size is changed to the value which is still greater than
   dcache's size, nothing should be done.  If change to the value
   which is less than dcache's size, just evict cache lines.

This is what this patch does.

gdb:

2013-11-02  Yao Qi  <yao@codesourcery.com>

	* dcache.c (dcache_evict): New function.
	(dcache_shrink): New function.
	(dcache_invalidate_p): New function.
	(dcache_alloc): Call dcache_evict.
	(set_dcache_size): Remove call target_dcache_invalidate.
	(set_dcache_line_size): Likewise.
	* dcache.h (dcache_shrink): Declare.
	(dcache_invalidate_p): Declare.
	* target-dcache.c (target_dcache_get): Invalidate and shrink
	cache if necessary.
	(target_dcache_get_or_init): Likewise.

gdb/doc:

2013-11-02  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (Caching Target Data): Update document of
	commands 'set dcache size' and 'set dcache line-size'.
---
 gdb/dcache.c        |   47 ++++++++++++++++++++++++++++++++++++++---------
 gdb/dcache.h        |    4 ++++
 gdb/doc/gdb.texinfo |    7 +++++--
 gdb/target-dcache.c |   17 ++++++++++++++++-
 4 files changed, 63 insertions(+), 12 deletions(-)

diff --git a/gdb/dcache.c b/gdb/dcache.c
index 5b32629..09ff2db 100644
--- a/gdb/dcache.c
+++ b/gdb/dcache.c
@@ -243,6 +243,43 @@ invalidate_block (struct dcache_block *block, void *param)
   append_block (&dcache->freelist, block);
 }
 
+/* Evict the cache line decided by the eviction algorithm.  Return the
+   evicted cache line.  */
+
+static struct dcache_block *
+dcache_evict (DCACHE *dcache)
+{
+  /* Evict the least recently allocated line.  */
+  struct dcache_block *db = dcache->oldest;
+
+  remove_block (&dcache->oldest, db);
+  splay_tree_remove (dcache->tree, (splay_tree_key) db->addr);
+
+  return db;
+}
+
+/* Shrink DCACHE if it is over-sized.  */
+
+void
+dcache_shrink (DCACHE *dcache)
+{
+  while (dcache->size > dcache_size)
+    {
+      struct dcache_block *db = dcache_evict (dcache);
+
+      free_block (db, NULL);
+      dcache->size--;
+    }
+}
+
+/* Return true if DCACHE should be invalidated.  */
+
+int
+dcache_invalidate_p (DCACHE *dcache)
+{
+  return (dcache->line_size != dcache_line_size);
+}
+
 /* Free all the data cache blocks, thus discarding all cached data.  */
 
 void
@@ -359,13 +396,7 @@ dcache_alloc (DCACHE *dcache, CORE_ADDR addr)
   struct dcache_block *db;
 
   if (dcache->size >= dcache_size)
-    {
-      /* Evict the least recently allocated line.  */
-      db = dcache->oldest;
-      remove_block (&dcache->oldest, db);
-
-      splay_tree_remove (dcache->tree, (splay_tree_key) db->addr);
-    }
+    db = dcache_evict (dcache);
   else
     {
       db = dcache->freelist;
@@ -669,7 +700,6 @@ set_dcache_size (char *args, int from_tty,
       dcache_size = DCACHE_DEFAULT_SIZE;
       error (_("Dcache size must be greater than 0."));
     }
-  target_dcache_invalidate ();
 }
 
 static void
@@ -683,7 +713,6 @@ set_dcache_line_size (char *args, int from_tty,
       dcache_line_size = DCACHE_DEFAULT_LINE_SIZE;
       error (_("Invalid dcache line size: %u (must be power of 2)."), d);
     }
-  target_dcache_invalidate ();
 }
 
 static void
diff --git a/gdb/dcache.h b/gdb/dcache.h
index 720a887..101c2ce 100644
--- a/gdb/dcache.h
+++ b/gdb/dcache.h
@@ -40,4 +40,8 @@ int dcache_xfer_memory (struct target_ops *ops, DCACHE *cache, CORE_ADDR mem,
 void dcache_update (DCACHE *dcache, CORE_ADDR memaddr, gdb_byte *myaddr,
 		    int len);
 
+void dcache_shrink (DCACHE *dcache);
+
+int dcache_invalidate_p (DCACHE *dcache);
+
 #endif /* DCACHE_H */
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 4d72983..561243b 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -10863,13 +10863,16 @@ printed in hex.
 @item set dcache size @var{size}
 @cindex dcache size
 @kindex set dcache size
-Set maximum number of entries in dcache (dcache depth above).
+Set maximum number of entries in dcache (dcache depth above).  When
+the dcache is being used, its size is greater than this setting, it
+is shrunk.
 
 @item set dcache line-size @var{line-size}
 @cindex dcache line-size
 @kindex set dcache line-size
 Set number of bytes each dcache entry caches (dcache width above).
-Must be a power of 2.
+Must be a power of 2.  When the dcache is being used, its width is
+different from this setting, it is invalidated.
 
 @item show dcache size
 @kindex show dcache size
diff --git a/gdb/target-dcache.c b/gdb/target-dcache.c
index bf04679..28f1aa6 100644
--- a/gdb/target-dcache.c
+++ b/gdb/target-dcache.c
@@ -45,6 +45,14 @@ target_dcache_invalidate (void)
 DCACHE *
 target_dcache_get (void)
 {
+  if (target_dcache_init_p ())
+    {
+      if (dcache_invalidate_p (target_dcache))
+	dcache_invalidate (target_dcache);
+
+      dcache_shrink (target_dcache);
+    }
+
   return target_dcache;
 }
 
@@ -54,7 +62,14 @@ target_dcache_get (void)
 DCACHE *
 target_dcache_get_or_init (void)
 {
-  if (!target_dcache_init_p ())
+  if (target_dcache_init_p ())
+    {
+      if (dcache_invalidate_p (target_dcache))
+	dcache_invalidate (target_dcache);
+
+      dcache_shrink (target_dcache);
+    }
+  else
     target_dcache = dcache_init ();
 
   return target_dcache;
-- 
1.7.7.6

Re: [PATCH 05/10] Invalidate or shrink dcache when setting is changed.

[Eli Zaretskii]

> From: Yao Qi <yao@codesourcery.com>
> Date: Sun, 3 Nov 2013 13:54:05 +0800
> 
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -10863,13 +10863,16 @@ printed in hex.
>  @item set dcache size @var{size}
>  @cindex dcache size
>  @kindex set dcache size
> -Set maximum number of entries in dcache (dcache depth above).
> +Set maximum number of entries in dcache (dcache depth above).  When
> +the dcache is being used, its size is greater than this setting, it
> +is shrunk.                ^^^^^^^^^^^^^^^^^^^^^^^^

"if its size is greater than ..."

>  @kindex set dcache line-size
>  Set number of bytes each dcache entry caches (dcache width above).
> -Must be a power of 2.
> +Must be a power of 2.  When the dcache is being used, its width is
> +different from this setting, it is invalidated.

Likewise.

OK with those changes.

Thanks.

Re: [PATCH 05/10] Invalidate or shrink dcache when setting is changed.

[Doug Evans]

On Sat, Nov 2, 2013 at 10:54 PM, Yao Qi <yao@codesourcery.com> wrote:
> Nowadays, when cache size or line size is changed by command,
> 'target_dcache' is invalidated.  It is too conservative.  We can
> optimize in the following ways,
>
>  - Don't have to invalidate dcache immediately after cache size or
>    line size is changed.  We can postpone the invalidation to the moment
>    using 'target_dcache'.
>  - Don't have to invalidate dcache if the cache size is changed.  If
>    cache size is changed to the value which is still greater than
>    dcache's size, nothing should be done.  If change to the value
>    which is less than dcache's size, just evict cache lines.
>
> This is what this patch does.
>
> gdb:
>
> 2013-11-02  Yao Qi  <yao@codesourcery.com>
>
>         * dcache.c (dcache_evict): New function.
>         (dcache_shrink): New function.
>         (dcache_invalidate_p): New function.
>         (dcache_alloc): Call dcache_evict.
>         (set_dcache_size): Remove call target_dcache_invalidate.
>         (set_dcache_line_size): Likewise.
>         * dcache.h (dcache_shrink): Declare.
>         (dcache_invalidate_p): Declare.
>         * target-dcache.c (target_dcache_get): Invalidate and shrink
>         cache if necessary.
>         (target_dcache_get_or_init): Likewise.
> [...]
> diff --git a/gdb/target-dcache.c b/gdb/target-dcache.c
> index bf04679..28f1aa6 100644
> --- a/gdb/target-dcache.c
> +++ b/gdb/target-dcache.c
> @@ -45,6 +45,14 @@ target_dcache_invalidate (void)
>  DCACHE *
>  target_dcache_get (void)
>  {
> +  if (target_dcache_init_p ())
> +    {
> +      if (dcache_invalidate_p (target_dcache))
> +       dcache_invalidate (target_dcache);
> +
> +      dcache_shrink (target_dcache);
> +    }
> +
>    return target_dcache;
>  }

It feels simpler if this were:

  if (target_dcache_init_p ())
    dcache_check_resize (target_dcache);
  return target_dcache;

There may be a better name than dcache_check_resize, one may want to
call it for situations other than resizings, but here target-dcache.c
is using 3 API routines from dcache.c to achieve what is essentially
one operation: check if the cache needs to be reconfigured and update
if so.

> @@ -54,7 +62,14 @@ target_dcache_get (void)
>  DCACHE *
>  target_dcache_get_or_init (void)
>  {
> -  if (!target_dcache_init_p ())
> +  if (target_dcache_init_p ())
> +    {
> +      if (dcache_invalidate_p (target_dcache))
> +       dcache_invalidate (target_dcache);
> +
> +      dcache_shrink (target_dcache);
> +    }
> +  else
>      target_dcache = dcache_init ();
>
>    return target_dcache;

I think it would be better to remove the duplication and have:

  if (target_dcache_init_p ())
    return target_dcache_get ();
  ...

Re: [PATCH 05/10] Invalidate or shrink dcache when setting is changed.

[Yao Qi]

On 11/18/2013 04:34 AM, Doug Evans wrote:
> It feels simpler if this were:
> 
>    if (target_dcache_init_p ())
>      dcache_check_resize (target_dcache);
>    return target_dcache;
> 
> There may be a better name than dcache_check_resize, one may want to
> call it for situations other than resizings, but here target-dcache.c
> is using 3 API routines from dcache.c to achieve what is essentially
> one operation: check if the cache needs to be reconfigured and update
> if so.

How about "dcache_update_config"?

> 
>> >@@ -54,7 +62,14 @@ target_dcache_get (void)
>> >  DCACHE *
>> >  target_dcache_get_or_init (void)
>> >  {
>> >-  if (!target_dcache_init_p ())
>> >+  if (target_dcache_init_p ())
>> >+    {
>> >+      if (dcache_invalidate_p (target_dcache))
>> >+       dcache_invalidate (target_dcache);
>> >+
>> >+      dcache_shrink (target_dcache);
>> >+    }
>> >+  else
>> >      target_dcache = dcache_init ();
>> >
>> >    return target_dcache;
> I think it would be better to remove the duplication and have:
> 
>    if (target_dcache_init_p ())
>      return target_dcache_get ();

Fixed.

-- 
Yao (齐尧)

gdb:

2013-11-18  Yao Qi  <yao@codesourcery.com>

	* dcache.c (dcache_evict): New function.
	(dcache_shrink): New function.
	(dcache_update_config): New function.
	(dcache_alloc): Call dcache_evict.
	(set_dcache_size): Remove call target_dcache_invalidate.
	(set_dcache_line_size): Likewise.
	* dcache.h (dcache_update_config): Declare.
	* target-dcache.c (target_dcache_get): Invalidate and shrink
	cache if necessary.
	(target_dcache_get_or_init): Likewise.

gdb/doc:

2013-11-18  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (Caching Target Data): Update document of
	commands 'set dcache size' and 'set dcache line-size'.
---
 gdb/dcache.c        |   53 ++++++++++++++++++++++++++++++++++++++++++--------
 gdb/dcache.h        |    2 +
 gdb/doc/gdb.texinfo |    7 ++++-
 gdb/target-dcache.c |    7 +++++-
 4 files changed, 57 insertions(+), 12 deletions(-)

diff --git a/gdb/dcache.c b/gdb/dcache.c
index 5b32629..4474be4 100644
--- a/gdb/dcache.c
+++ b/gdb/dcache.c
@@ -243,6 +243,49 @@ invalidate_block (struct dcache_block *block, void *param)
   append_block (&dcache->freelist, block);
 }
 
+/* Evict the cache line decided by the eviction algorithm.  Return the
+   evicted cache line.  */
+
+static struct dcache_block *
+dcache_evict (DCACHE *dcache)
+{
+  /* Evict the least recently allocated line.  */
+  struct dcache_block *db = dcache->oldest;
+
+  remove_block (&dcache->oldest, db);
+  splay_tree_remove (dcache->tree, (splay_tree_key) db->addr);
+
+  return db;
+}
+
+/* Shrink DCACHE if it is over-sized.  */
+
+static void
+dcache_shrink (DCACHE *dcache)
+{
+  while (dcache->size > dcache_size)
+    {
+      struct dcache_block *db = dcache_evict (dcache);
+
+      free_block (db, NULL);
+      dcache->size--;
+    }
+}
+
+/* Update the configuration, such as line-size, of DCACHE if
+   necessary.  */
+
+void
+dcache_update_config (DCACHE *dcache)
+{
+  /* DCACHE must be invalidated if its line-size is different from the
+     global setting.  */
+  if (dcache->line_size != dcache_line_size)
+    dcache_invalidate (dcache);
+
+  dcache_shrink (dcache);
+}
+
 /* Free all the data cache blocks, thus discarding all cached data.  */
 
 void
@@ -359,13 +402,7 @@ dcache_alloc (DCACHE *dcache, CORE_ADDR addr)
   struct dcache_block *db;
 
   if (dcache->size >= dcache_size)
-    {
-      /* Evict the least recently allocated line.  */
-      db = dcache->oldest;
-      remove_block (&dcache->oldest, db);
-
-      splay_tree_remove (dcache->tree, (splay_tree_key) db->addr);
-    }
+    db = dcache_evict (dcache);
   else
     {
       db = dcache->freelist;
@@ -669,7 +706,6 @@ set_dcache_size (char *args, int from_tty,
       dcache_size = DCACHE_DEFAULT_SIZE;
       error (_("Dcache size must be greater than 0."));
     }
-  target_dcache_invalidate ();
 }
 
 static void
@@ -683,7 +719,6 @@ set_dcache_line_size (char *args, int from_tty,
       dcache_line_size = DCACHE_DEFAULT_LINE_SIZE;
       error (_("Invalid dcache line size: %u (must be power of 2)."), d);
     }
-  target_dcache_invalidate ();
 }
 
 static void
diff --git a/gdb/dcache.h b/gdb/dcache.h
index 720a887..d2fcd6a 100644
--- a/gdb/dcache.h
+++ b/gdb/dcache.h
@@ -40,4 +40,6 @@ int dcache_xfer_memory (struct target_ops *ops, DCACHE *cache, CORE_ADDR mem,
 void dcache_update (DCACHE *dcache, CORE_ADDR memaddr, gdb_byte *myaddr,
 		    int len);
 
+void dcache_update_config (DCACHE *dcache);
+
 #endif /* DCACHE_H */
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 8377ca8..16ae0c8 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -10878,13 +10878,16 @@ printed in hex.
 @item set dcache size @var{size}
 @cindex dcache size
 @kindex set dcache size
-Set maximum number of entries in dcache (dcache depth above).
+Set maximum number of entries in dcache (dcache depth above).  When
+the dcache is being used, if its size is greater than this setting, it
+is shrunk.
 
 @item set dcache line-size @var{line-size}
 @cindex dcache line-size
 @kindex set dcache line-size
 Set number of bytes each dcache entry caches (dcache width above).
-Must be a power of 2.
+Must be a power of 2.  When the dcache is being used, if its width is
+different from this setting, it is invalidated.
 
 @item show dcache size
 @kindex show dcache size
diff --git a/gdb/target-dcache.c b/gdb/target-dcache.c
index bf04679..6bd576d 100644
--- a/gdb/target-dcache.c
+++ b/gdb/target-dcache.c
@@ -45,6 +45,9 @@ target_dcache_invalidate (void)
 DCACHE *
 target_dcache_get (void)
 {
+  if (target_dcache_init_p ())
+    dcache_update_config (target_dcache);
+
   return target_dcache;
 }
 
@@ -54,7 +57,9 @@ target_dcache_get (void)
 DCACHE *
 target_dcache_get_or_init (void)
 {
-  if (!target_dcache_init_p ())
+  if (target_dcache_init_p ())
+    return target_dcache_get ();
+  else
     target_dcache = dcache_init ();
 
   return target_dcache;
-- 
1.7.7.6

Re: [PATCH 05/10] Invalidate or shrink dcache when setting is changed.

[Pedro Alves]

On 11/03/2013 05:54 AM, Yao Qi wrote:
> Nowadays, when cache size or line size is changed by command,
> 'target_dcache' is invalidated.  It is too conservative.  We can
> optimize in the following ways,
> 
>  - Don't have to invalidate dcache immediately after cache size or
>    line size is changed.  We can postpone the invalidation to the moment
>    using 'target_dcache'.
>  - Don't have to invalidate dcache if the cache size is changed.  If
>    cache size is changed to the value which is still greater than
>    dcache's size, nothing should be done.  If change to the value
>    which is less than dcache's size, just evict cache lines.
> 
> This is what this patch does.

Actually, my "My thoughts exactly." comment in the other patch was
originally directed at this patch.  Do we really need this extra
complication?  What's the use case that needs this?

-- 
Pedro Alves

Re: [PATCH 05/10] Invalidate or shrink dcache when setting is changed.

[Yao Qi]

On 11/18/2013 11:56 PM, Pedro Alves wrote:
> On 11/03/2013 05:54 AM, Yao Qi wrote:
>> Nowadays, when cache size or line size is changed by command,
>> 'target_dcache' is invalidated.  It is too conservative.  We can
>> optimize in the following ways,
>>
>>   - Don't have to invalidate dcache immediately after cache size or
>>     line size is changed.  We can postpone the invalidation to the moment
>>     using 'target_dcache'.
>>   - Don't have to invalidate dcache if the cache size is changed.  If
>>     cache size is changed to the value which is still greater than
>>     dcache's size, nothing should be done.  If change to the value
>>     which is less than dcache's size, just evict cache lines.
>>
>> This is what this patch does.
>
> Actually, my "My thoughts exactly." comment in the other patch was
> originally directed at this patch.  Do we really need this extra
> complication?  What's the use case that needs this?
>

Here are three use cases,

  1) target dcache has 16 cache lines, and dcache size is 4096.  User 
types command "set dcache size 32" or "set dcache size 4160".
  2) target dcache has 16 cache lines.  User types command "set dcache 
size 8".
  3) line size of target dcache is 32.  User types commands

     "set dcache line-size 16"  // change line-size to 16.
     // change it back.
     "set dcache line-size 32"

This "extra complication" looks natural or reasonable to a software 
cache.  We are using cache, and we'd like to do as few invalidations as 
possible.

-- 
Yao (齐尧)

Re: [PATCH 05/10] Invalidate or shrink dcache when setting is changed.

[Pedro Alves]

On 11/19/2013 06:00 AM, Yao Qi wrote:
> On 11/18/2013 11:56 PM, Pedro Alves wrote:
>> On 11/03/2013 05:54 AM, Yao Qi wrote:
>>> Nowadays, when cache size or line size is changed by command,
>>> 'target_dcache' is invalidated.  It is too conservative.  We can
>>> optimize in the following ways,
>>>
>>>   - Don't have to invalidate dcache immediately after cache size or
>>>     line size is changed.  We can postpone the invalidation to the moment
>>>     using 'target_dcache'.
>>>   - Don't have to invalidate dcache if the cache size is changed.  If
>>>     cache size is changed to the value which is still greater than
>>>     dcache's size, nothing should be done.  If change to the value
>>>     which is less than dcache's size, just evict cache lines.
>>>
>>> This is what this patch does.
>>
>> Actually, my "My thoughts exactly." comment in the other patch was
>> originally directed at this patch.  Do we really need this extra
>> complication?  What's the use case that needs this?
>>
> 
> Here are three use cases,
> 
>   1) target dcache has 16 cache lines, and dcache size is 4096.  User 
> types command "set dcache size 32" or "set dcache size 4160".
>   2) target dcache has 16 cache lines.  User types command "set dcache 
> size 8".
>   3) line size of target dcache is 32.  User types commands
> 
>      "set dcache line-size 16"  // change line-size to 16.
>      // change it back.
>      "set dcache line-size 32"

Sure, OK, I guess I did the wrong question then.  I really mean,
why should we care about preserving the cache when the user
changes cache size?  What makes this a use case users actually
care about?  I don't think users going to be doing this
sort of thing in a loop?  At most a couple times to check which
size might be better for then, and then stick it in .gdbinit,
forever after forgotten.

Actually, if I was probing for the ideal size, I think I'd be
annoyed that GDB didn't flush the cache, as I'd try
disassembling/backtracing with different cache sizes, and
always want to start from a cold cache.

> This "extra complication" looks natural or reasonable to a software 
> cache.  

It's always a complication/necessity/usefulness balance.
It doesn't look so reasonable if the complication it adds
isn't useful in practice, because more code usually means more
chances of getting things wrong.  The question is then what is
the real use case that makes this necessary, as opposed to keeping
it simple.

> We are using cache, and we'd like to do as few invalidations as 
> possible.

-- 
Pedro Alves

Re: [PATCH 05/10] Invalidate or shrink dcache when setting is changed.

[Yao Qi]

On 11/19/2013 07:38 PM, Pedro Alves wrote:
> Sure, OK, I guess I did the wrong question then.  I really mean,
> why should we care about preserving the cache when the user
> changes cache size?  What makes this a use case users actually
> care about?  I don't think users going to be doing this

To question #1, because it is expensive to fetch contents from target
memory, and it is unnecessary to flush cache when user changes cache size.

To question #2, nothing.  Cache invalidation is invisible to user.

> sort of thing in a loop?  At most a couple times to check which
> size might be better for then, and then stick it in .gdbinit,
> forever after forgotten.
>
> Actually, if I was probing for the ideal size, I think I'd be
> annoyed that GDB didn't flush the cache, as I'd try
> disassembling/backtracing with different cache sizes, and
> always want to start from a cold cache.

I agree that use case #3 is not very useful.

>
>> >This "extra complication" looks natural or reasonable to a software
>> >cache.
> It's always a complication/necessity/usefulness balance.
> It doesn't look so reasonable if the complication it adds
> isn't useful in practice, because more code usually means more
> chances of getting things wrong.  The question is then what is
> the real use case that makes this necessary, as opposed to keeping
> it simple.
>

If use case #1 and #2 are not real use cases, I don't have a real use 
case to show the cache invalidation is avoided due this patch.

-- 
Yao (齐尧)

[PATCH 01/10] Remove last_cache

[Yao Qi]

This patch removes global variable 'last_cache', and initialize
'target_dcache' lazily, so that 'target_dcache' can replace
'last_cache'.  No functionalities should be changed after this patch.

gdb:

2013-11-02  Yao Qi  <yao@codesourcery.com>

	* dcache.c (last_cache): Remove.
	(dcache_free, dcache_init): Update.
	(dcache_update):
	(dcache_print_line): Add parameter 'dcache'.  Replace
	'target_dcache' with 'dcache'.
	(dcache_info): Move code to dcache_info_1. Call
	'dcache_info_1'.
	(dcache_info_1): New function.
	(set_dcache_size): Call target_dcache_invalidate.
	(set_dcache_line_size): Call target_dcache_invalidate.
	* target.c (target_dcache_init_p): New function.
	(target_dcache_invalidate): Check target_dcache_init_p first.
	(target_dcache_get, target_dcache_get_or_init): New function.
	(memory_xfer_partial_1): Adjust.
	(initialize_target): Don't initialize 'target_dcache'.
	* target.h (struct dcache_struct): Declare.
	(target_dcache_get): Declare.
---
 gdb/dcache.c |   48 +++++++++++++++++++++++++-----------------------
 gdb/target.c |   47 ++++++++++++++++++++++++++++++++++++++---------
 gdb/target.h |    3 +++
 3 files changed, 66 insertions(+), 32 deletions(-)

diff --git a/gdb/dcache.c b/gdb/dcache.c
index acb9de4..f7a9c63 100644
--- a/gdb/dcache.c
+++ b/gdb/dcache.c
@@ -139,8 +139,6 @@ show_dcache_enabled_p (struct ui_file *file, int from_tty,
   fprintf_filtered (file, _("Deprecated remotecache flag is %s.\n"), value);
 }
 
-static DCACHE *last_cache; /* Used by info dcache.  */
-
 /* Add BLOCK to circular block list BLIST, behind the block at *BLIST.
    *BLIST is not updated (unless it was previously NULL of course).
    This is for the least-recently-allocated list's sake:
@@ -225,9 +223,6 @@ free_block (struct dcache_block *block, void *param)
 void
 dcache_free (DCACHE *dcache)
 {
-  if (last_cache == dcache)
-    last_cache = NULL;
-
   splay_tree_delete (dcache->tree);
   for_each_block (&dcache->oldest, free_block, NULL);
   for_each_block (&dcache->freelist, free_block, NULL);
@@ -468,7 +463,6 @@ dcache_init (void)
   dcache->size = 0;
   dcache->line_size = dcache_line_size;
   dcache->ptid = null_ptid;
-  last_cache = dcache;
 
   return dcache;
 }
@@ -557,26 +551,28 @@ dcache_update (DCACHE *dcache, CORE_ADDR memaddr, gdb_byte *myaddr, int len)
     dcache_poke_byte (dcache, memaddr + i, myaddr + i);
 }
 
+/* Print DCACHE line INDEX.  */
+
 static void
-dcache_print_line (int index)
+dcache_print_line (DCACHE *dcache, int index)
 {
   splay_tree_node n;
   struct dcache_block *db;
   int i, j;
 
-  if (!last_cache)
+  if (dcache == NULL)
     {
       printf_filtered (_("No data cache available.\n"));
       return;
     }
 
-  n = splay_tree_min (last_cache->tree);
+  n = splay_tree_min (dcache->tree);
 
   for (i = index; i > 0; --i)
     {
       if (!n)
 	break;
-      n = splay_tree_successor (last_cache->tree, n->key);
+      n = splay_tree_successor (dcache->tree, n->key);
     }
 
   if (!n)
@@ -590,19 +586,21 @@ dcache_print_line (int index)
   printf_filtered (_("Line %d: address %s [%d hits]\n"),
 		   index, paddress (target_gdbarch (), db->addr), db->refs);
 
-  for (j = 0; j < last_cache->line_size; j++)
+  for (j = 0; j < dcache->line_size; j++)
     {
       printf_filtered ("%02x ", db->data[j]);
 
       /* Print a newline every 16 bytes (48 characters).  */
-      if ((j % 16 == 15) && (j != last_cache->line_size - 1))
+      if ((j % 16 == 15) && (j != dcache->line_size - 1))
 	printf_filtered ("\n");
     }
   printf_filtered ("\n");
 }
 
+/* Parse EXP and show the info about DCACHE.  */
+
 static void
-dcache_info (char *exp, int tty)
+dcache_info_1 (DCACHE *dcache, char *exp)
 {
   splay_tree_node n;
   int i, refcount;
@@ -618,27 +616,27 @@ dcache_info (char *exp, int tty)
           return;
 	}
 
-      dcache_print_line (i);
+      dcache_print_line (dcache, i);
       return;
     }
 
   printf_filtered (_("Dcache %u lines of %u bytes each.\n"),
 		   dcache_size,
-		   last_cache ? (unsigned) last_cache->line_size
+		   dcache ? (unsigned) dcache->line_size
 		   : dcache_line_size);
 
-  if (!last_cache || ptid_equal (last_cache->ptid, null_ptid))
+  if (dcache == NULL || ptid_equal (dcache->ptid, null_ptid))
     {
       printf_filtered (_("No data cache available.\n"));
       return;
     }
 
   printf_filtered (_("Contains data for %s\n"),
-		   target_pid_to_str (last_cache->ptid));
+		   target_pid_to_str (dcache->ptid));
 
   refcount = 0;
 
-  n = splay_tree_min (last_cache->tree);
+  n = splay_tree_min (dcache->tree);
   i = 0;
 
   while (n)
@@ -650,13 +648,19 @@ dcache_info (char *exp, int tty)
       i++;
       refcount += db->refs;
 
-      n = splay_tree_successor (last_cache->tree, n->key);
+      n = splay_tree_successor (dcache->tree, n->key);
     }
 
   printf_filtered (_("Cache state: %d active lines, %d hits\n"), i, refcount);
 }
 
 static void
+dcache_info (char *exp, int tty)
+{
+  dcache_info_1 (target_dcache_get (), exp);
+}
+
+static void
 set_dcache_size (char *args, int from_tty,
 		 struct cmd_list_element *c)
 {
@@ -665,8 +669,7 @@ set_dcache_size (char *args, int from_tty,
       dcache_size = DCACHE_DEFAULT_SIZE;
       error (_("Dcache size must be greater than 0."));
     }
-  if (last_cache)
-    dcache_invalidate (last_cache);
+  target_dcache_invalidate ();
 }
 
 static void
@@ -680,8 +683,7 @@ set_dcache_line_size (char *args, int from_tty,
       dcache_line_size = DCACHE_DEFAULT_LINE_SIZE;
       error (_("Invalid dcache line size: %u (must be power of 2)."), d);
     }
-  if (last_cache)
-    dcache_invalidate (last_cache);
+  target_dcache_invalidate ();
 }
 
 static void
diff --git a/gdb/target.c b/gdb/target.c
index 22d7fb6..6403e86 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -238,12 +238,42 @@ show_stack_cache_enabled_p (struct ui_file *file, int from_tty,
 /* Cache of memory operations, to speed up remote access.  */
 static DCACHE *target_dcache;
 
+/* Target dcache is initialized or not.  */
+
+static int
+target_dcache_init_p (void)
+{
+  return (target_dcache != NULL);
+}
+
 /* Invalidate the target dcache.  */
 
 void
 target_dcache_invalidate (void)
 {
-  dcache_invalidate (target_dcache);
+  if (target_dcache_init_p ())
+    dcache_invalidate (target_dcache);
+}
+
+/* Return the target dcache.  Return NULL if target dcache is not
+   initialized yet.  */
+
+DCACHE *
+target_dcache_get (void)
+{
+  return target_dcache;
+}
+
+/* Return the target dcache.  If it is not initialized yet, initialize
+   it.  */
+
+static DCACHE *
+target_dcache_get_or_init (void)
+{
+  if (!target_dcache_init_p ())
+    target_dcache = dcache_init ();
+
+  return target_dcache;
 }
 
 /* The user just typed 'target' without the name of a target.  */
@@ -1588,15 +1618,15 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
       && (region->attrib.cache
 	  || (stack_cache_enabled_p && object == TARGET_OBJECT_STACK_MEMORY)))
     {
+      DCACHE *dcache = target_dcache_get_or_init ();
+
       if (readbuf != NULL)
-	res = dcache_xfer_memory (ops, target_dcache, memaddr, readbuf,
-				  reg_len, 0);
+	res = dcache_xfer_memory (ops, dcache, memaddr, readbuf, reg_len, 0);
       else
 	/* FIXME drow/2006-08-09: If we're going to preserve const
 	   correctness dcache_xfer_memory should take readbuf and
 	   writebuf.  */
-	res = dcache_xfer_memory (ops, target_dcache, memaddr,
-				  (void *) writebuf,
+	res = dcache_xfer_memory (ops, dcache, memaddr, (void *) writebuf,
 				  reg_len, 1);
       if (res <= 0)
 	return -1;
@@ -1641,7 +1671,9 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
       && stack_cache_enabled_p
       && object != TARGET_OBJECT_STACK_MEMORY)
     {
-      dcache_update (target_dcache, memaddr, (void *) writebuf, res);
+      DCACHE *dcache = target_dcache_get_or_init ();
+
+      dcache_update (dcache, memaddr, (void *) writebuf, res);
     }
 
   /* If we still haven't got anything, return the last error.  We
@@ -5186,7 +5218,4 @@ When this permission is on, GDB may interrupt/stop the target's execution.\n\
 Otherwise, any attempt to interrupt or stop will be ignored."),
 			   set_target_permissions, NULL,
 			   &setlist, &showlist);
-
-
-  target_dcache = dcache_init ();
 }
diff --git a/gdb/target.h b/gdb/target.h
index 56ca40c..4cff2aa 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -37,6 +37,7 @@ struct uploaded_tp;
 struct static_tracepoint_marker;
 struct traceframe_info;
 struct expression;
+struct dcache_struct;
 
 /* This include file defines the interface between the main part
    of the debugger, and the part which is target-specific, or
@@ -1045,6 +1046,8 @@ int target_supports_disable_randomization (void);
 /* Invalidate all target dcaches.  */
 extern void target_dcache_invalidate (void);
 
+extern struct dcache_struct *target_dcache_get (void);
+
 extern int target_read_string (CORE_ADDR, char **, int, int *);
 
 extern int target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr,
-- 
1.7.7.6

Re: [PATCH 01/10] Remove last_cache

[Doug Evans]

On Sat, Nov 2, 2013 at 10:54 PM, Yao Qi <yao@codesourcery.com> wrote:
> This patch removes global variable 'last_cache', and initialize
> 'target_dcache' lazily, so that 'target_dcache' can replace
> 'last_cache'.  No functionalities should be changed after this patch.
>
> gdb:
>
> 2013-11-02  Yao Qi  <yao@codesourcery.com>
>
>         * dcache.c (last_cache): Remove.
>         (dcache_free, dcache_init): Update.
>         (dcache_update):
>         (dcache_print_line): Add parameter 'dcache'.  Replace
>         'target_dcache' with 'dcache'.
>         (dcache_info): Move code to dcache_info_1. Call
>         'dcache_info_1'.
>         (dcache_info_1): New function.
>         (set_dcache_size): Call target_dcache_invalidate.
>         (set_dcache_line_size): Call target_dcache_invalidate.
>         * target.c (target_dcache_init_p): New function.
>         (target_dcache_invalidate): Check target_dcache_init_p first.
>         (target_dcache_get, target_dcache_get_or_init): New function.
>         (memory_xfer_partial_1): Adjust.
>         (initialize_target): Don't initialize 'target_dcache'.
>         * target.h (struct dcache_struct): Declare.
>         (target_dcache_get): Declare.

I haven't reviewed the rest of the patches in the series yet,
but as a cleanup to remove last_cache,
this patch is fine with me.

[PATCH 07/10] Associate target_dcache to address_space.

[Yao Qi]

Hi,
Nowadays, 'target_dcache' is a global variable in GDB, which is not
necessary.  It can be a per-address-space variable.  In this patch, we
associate target_dcache to address_space.

gdb/doc:

2013-11-02  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (Caching Target Data): Update doc for
	per-address-space dcache.

gdb:

2013-11-02  Yao Qi  <yao@codesourcery.com>

	* progspace.h (struct address_space_data): Declare.
	* target-dcache.c: Include "progspace.h".
	(target_dache): Remove.
	(target_dcache_aspace_key): New.
	(target_dcache_cleanup): New function.
	(target_dcache_init_p): Get data through
	target_dcache_aspace_key.
	(target_dcache_invalidate): Likewise.
	(target_dcache_get): Likewise.
	(target_dcache_get_or_init): Likewise.
	(_initialize_target_dcache): Initialize
	target_dcache_aspace_key.
---
 gdb/doc/gdb.texinfo |   12 ++++++----
 gdb/progspace.h     |    1 +
 gdb/target-dcache.c |   60 +++++++++++++++++++++++++++++++++++++-------------
 3 files changed, 52 insertions(+), 21 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 561243b..a64bb0b 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -10813,6 +10813,8 @@ character.
 @cindex caching data of targets
 
 @value{GDBN} caches data exchanged between the debugger and a target.
+Each cache is associated with the address space of the inferior.  See
+@ref{Inferiors and Programs} about inferior and address space.
 Such caching generally improves performance in @ref{Remote Debugging},
 because it reduces the overhead of the remote protocol by
 bundling memory reads and writes into large chunks.  Unfortunately, simply
@@ -10851,11 +10853,11 @@ Show the current state of data caching for memory accesses.
 
 @kindex info dcache
 @item info dcache @r{[}line@r{]}
-Print the information about the data cache performance.  The
-information displayed includes the dcache width and depth, and for
-each cache line, its number, address, and how many times it was
-referenced.  This command is useful for debugging the data cache
-operation.
+Print the information about the performance of data cache of the
+current inferior's address space.  The information displayed
+includes the dcache width and depth, and for each cache line, its
+number, address, and how many times it was referenced.  This
+command is useful for debugging the data cache operation.
 
 If a line number is specified, the contents of that line will be
 printed in hex.
diff --git a/gdb/progspace.h b/gdb/progspace.h
index 3735202..bbe81a4 100644
--- a/gdb/progspace.h
+++ b/gdb/progspace.h
@@ -32,6 +32,7 @@ struct objfile;
 struct inferior;
 struct exec;
 struct program_space_data;
+struct address_space_data;
 
 typedef struct so_list *so_list_ptr;
 DEF_VEC_P (so_list_ptr);
diff --git a/gdb/target-dcache.c b/gdb/target-dcache.c
index 28f1aa6..a840c40 100644
--- a/gdb/target-dcache.c
+++ b/gdb/target-dcache.c
@@ -18,16 +18,31 @@
 #include "defs.h"
 #include "target-dcache.h"
 #include "gdbcmd.h"
+#include "progspace.h"
 
-/* Cache of memory operations, to speed up remote access.  */
-static DCACHE *target_dcache;
+/* The target dcache is kept per-address-space.  This key lets us
+   associate the cache with the address space.  */
+
+static const struct address_space_data *target_dcache_aspace_key;
+
+/* Clean up dcache, represented by ARG, which is associated with
+   ASPACE.  */
+
+static void
+target_dcache_cleanup (struct address_space *aspace, void *arg)
+{
+  dcache_free (arg);
+}
 
 /* Target dcache is initialized or not.  */
 
 int
 target_dcache_init_p (void)
 {
-  return (target_dcache != NULL);
+  DCACHE *dcache = address_space_data (current_program_space->aspace,
+				       target_dcache_aspace_key);
+
+ return (dcache != NULL);
 }
 
 /* Invalidate the target dcache.  */
@@ -35,8 +50,11 @@ target_dcache_init_p (void)
 void
 target_dcache_invalidate (void)
 {
-  if (target_dcache_init_p ())
-    dcache_invalidate (target_dcache);
+  DCACHE *dcache = address_space_data (current_program_space->aspace,
+				       target_dcache_aspace_key);
+
+  if (dcache != NULL)
+    dcache_invalidate (dcache);
 }
 
 /* Return the target dcache.  Return NULL if target dcache is not
@@ -45,15 +63,18 @@ target_dcache_invalidate (void)
 DCACHE *
 target_dcache_get (void)
 {
-  if (target_dcache_init_p ())
+  DCACHE *dcache = address_space_data (current_program_space->aspace,
+				       target_dcache_aspace_key);
+
+  if (dcache != NULL)
     {
-      if (dcache_invalidate_p (target_dcache))
-	dcache_invalidate (target_dcache);
+      if (dcache_invalidate_p (dcache))
+	dcache_invalidate (dcache);
 
-      dcache_shrink (target_dcache);
+      dcache_shrink (dcache);
     }
 
-  return target_dcache;
+  return dcache;
 }
 
 /* Return the target dcache.  If it is not initialized yet, initialize
@@ -62,17 +83,20 @@ target_dcache_get (void)
 DCACHE *
 target_dcache_get_or_init (void)
 {
-  if (target_dcache_init_p ())
+  DCACHE *dcache = address_space_data (current_program_space->aspace,
+				       target_dcache_aspace_key);
+
+  if (dcache != NULL)
     {
-      if (dcache_invalidate_p (target_dcache))
-	dcache_invalidate (target_dcache);
+      if (dcache_invalidate_p (dcache))
+	dcache_invalidate (dcache);
 
-      dcache_shrink (target_dcache);
+      dcache_shrink (dcache);
     }
   else
-    target_dcache = dcache_init ();
+    dcache = dcache_init ();
 
-  return target_dcache;
+  return dcache;
 }
 
 /* The option sets this.  */
@@ -128,4 +152,8 @@ By default, caching for stack access is on."),
 			   set_stack_cache_enabled_p,
 			   show_stack_cache_enabled_p,
 			   &setlist, &showlist);
+
+  target_dcache_aspace_key
+    = register_address_space_data_with_cleanup (NULL,
+						target_dcache_cleanup);
 }
-- 
1.7.7.6

Re: [PATCH 07/10] Associate target_dcache to address_space.

[Eli Zaretskii]

> From: Yao Qi <yao@codesourcery.com>
> Date: Sun, 3 Nov 2013 13:54:07 +0800
> 
> 2013-11-02  Yao Qi  <yao@codesourcery.com>
> 
> 	* gdb.texinfo (Caching Target Data): Update doc for
> 	per-address-space dcache.

This part is approved, provided you fix the gotcha pointed out below.

>  @value{GDBN} caches data exchanged between the debugger and a target.
> +Each cache is associated with the address space of the inferior.  See
> +@ref{Inferiors and Programs} about inferior and address space.

Please use "@xref" instead of "See @ref" at the beginning of a sentence.

Thanks.

Re: [PATCH 07/10] Associate target_dcache to address_space.

[Doug Evans]

On Sat, Nov 2, 2013 at 10:54 PM, Yao Qi <yao@codesourcery.com> wrote:
> Hi,
> Nowadays, 'target_dcache' is a global variable in GDB, which is not
> necessary.  It can be a per-address-space variable.  In this patch, we
> associate target_dcache to address_space.
>
> gdb/doc:
>
> 2013-11-02  Yao Qi  <yao@codesourcery.com>
>
>         * gdb.texinfo (Caching Target Data): Update doc for
>         per-address-space dcache.
>
> gdb:
>
> 2013-11-02  Yao Qi  <yao@codesourcery.com>
>
>         * progspace.h (struct address_space_data): Declare.
>         * target-dcache.c: Include "progspace.h".
>         (target_dache): Remove.
>         (target_dcache_aspace_key): New.
>         (target_dcache_cleanup): New function.
>         (target_dcache_init_p): Get data through
>         target_dcache_aspace_key.
>         (target_dcache_invalidate): Likewise.
>         (target_dcache_get): Likewise.
>         (target_dcache_get_or_init): Likewise.
>         (_initialize_target_dcache): Initialize
>         target_dcache_aspace_key.

Ok.

[I realize I've asked for changes that will affect this patch, but
beyond that it's ok.]

Re: [PATCH 07/10] Associate target_dcache to address_space.

[Yao Qi]

On 11/18/2013 05:09 AM, Doug Evans wrote:
> Ok.
> 
> [I realize I've asked for changes that will affect this patch, but
> beyond that it's ok.]

Update the patch, and fix the doc pointed out by Eli.
Patch below is pushed.

-- 
Yao (齐尧)

gdb/doc:

2013-11-20  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (Caching Target Data): Update doc for
	per-address-space dcache.

gdb:

2013-11-20  Yao Qi  <yao@codesourcery.com>

	* progspace.h (struct address_space_data): Declare.
	* target-dcache.c: Include "progspace.h".
	(target_dache): Remove.
	(target_dcache_aspace_key): New.
	(target_dcache_cleanup): New function.
	(target_dcache_init_p): Get data through
	target_dcache_aspace_key.
	(target_dcache_invalidate): Likewise.
	(target_dcache_get): Likewise.
	(target_dcache_get_or_init): Likewise.
	(_initialize_target_dcache): Initialize
	target_dcache_aspace_key.
---
 gdb/ChangeLog       |   15 +++++++++++++++
 gdb/doc/ChangeLog   |    5 +++++
 gdb/doc/gdb.texinfo |   12 +++++++-----
 gdb/progspace.h     |    1 +
 gdb/target-dcache.c |   46 +++++++++++++++++++++++++++++++++++++---------
 5 files changed, 65 insertions(+), 14 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e8df902..dadea24 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,20 @@
 2013-11-20  Yao Qi  <yao@codesourcery.com>
 
+	* progspace.h (struct address_space_data): Declare.
+	* target-dcache.c: Include "progspace.h".
+	(target_dache): Remove.
+	(target_dcache_aspace_key): New.
+	(target_dcache_cleanup): New function.
+	(target_dcache_init_p): Get data through
+	target_dcache_aspace_key.
+	(target_dcache_invalidate): Likewise.
+	(target_dcache_get): Likewise.
+	(target_dcache_get_or_init): Likewise.
+	(_initialize_target_dcache): Initialize
+	target_dcache_aspace_key.
+
+2013-11-20  Yao Qi  <yao@codesourcery.com>
+
 	* progspace.c (struct address_space): Update comments.
 	<REGISTRY_FIELDS>: New fields.
 	DEFINE_REGISTRY for address_space.
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 9b641e6..207b845 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,5 +1,10 @@
 2013-11-20  Yao Qi  <yao@codesourcery.com>
 
+	* gdb.texinfo (Caching Target Data): Update doc for
+	per-address-space dcache.
+
+2013-11-20  Yao Qi  <yao@codesourcery.com>
+
 	* gdb.texinfo (Data): Rename menu item.
 	(Caching Remote Data): Rename to ...
 	(Caching Target Data): ... it.  Update.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index acad48d..d1fa6c8 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -10828,6 +10828,8 @@ character.
 @cindex caching data of targets
 
 @value{GDBN} caches data exchanged between the debugger and a target.
+Each cache is associated with the address space of the inferior.
+@xref{Inferiors and Programs}, about inferior and address space.
 Such caching generally improves performance in remote debugging
 (@pxref{Remote Debugging}), because it reduces the overhead of the
 remote protocol by bundling memory reads and writes into large chunks.
@@ -10867,11 +10869,11 @@ Show the current state of data caching for memory accesses.
 
 @kindex info dcache
 @item info dcache @r{[}line@r{]}
-Print the information about the data cache performance.  The
-information displayed includes the dcache width and depth, and for
-each cache line, its number, address, and how many times it was
-referenced.  This command is useful for debugging the data cache
-operation.
+Print the information about the performance of data cache of the
+current inferior's address space.  The information displayed
+includes the dcache width and depth, and for each cache line, its
+number, address, and how many times it was referenced.  This
+command is useful for debugging the data cache operation.
 
 If a line number is specified, the contents of that line will be
 printed in hex.
diff --git a/gdb/progspace.h b/gdb/progspace.h
index 04ac374..e2786ec 100644
--- a/gdb/progspace.h
+++ b/gdb/progspace.h
@@ -33,6 +33,7 @@ struct inferior;
 struct exec;
 struct address_space;
 struct program_space_data;
+struct address_space_data;
 
 typedef struct so_list *so_list_ptr;
 DEF_VEC_P (so_list_ptr);
diff --git a/gdb/target-dcache.c b/gdb/target-dcache.c
index bf04679..eb7d478 100644
--- a/gdb/target-dcache.c
+++ b/gdb/target-dcache.c
@@ -18,16 +18,31 @@
 #include "defs.h"
 #include "target-dcache.h"
 #include "gdbcmd.h"
+#include "progspace.h"
 
-/* Cache of memory operations, to speed up remote access.  */
-static DCACHE *target_dcache;
+/* The target dcache is kept per-address-space.  This key lets us
+   associate the cache with the address space.  */
+
+static const struct address_space_data *target_dcache_aspace_key;
+
+/* Clean up dcache, represented by ARG, which is associated with
+   ASPACE.  */
+
+static void
+target_dcache_cleanup (struct address_space *aspace, void *arg)
+{
+  dcache_free (arg);
+}
 
 /* Target dcache is initialized or not.  */
 
 int
 target_dcache_init_p (void)
 {
-  return (target_dcache != NULL);
+  DCACHE *dcache = address_space_data (current_program_space->aspace,
+				       target_dcache_aspace_key);
+
+  return (dcache != NULL);
 }
 
 /* Invalidate the target dcache.  */
@@ -35,8 +50,11 @@ target_dcache_init_p (void)
 void
 target_dcache_invalidate (void)
 {
-  if (target_dcache_init_p ())
-    dcache_invalidate (target_dcache);
+  DCACHE *dcache = address_space_data (current_program_space->aspace,
+				       target_dcache_aspace_key);
+
+  if (dcache != NULL)
+    dcache_invalidate (dcache);
 }
 
 /* Return the target dcache.  Return NULL if target dcache is not
@@ -45,7 +63,10 @@ target_dcache_invalidate (void)
 DCACHE *
 target_dcache_get (void)
 {
-  return target_dcache;
+  DCACHE *dcache = address_space_data (current_program_space->aspace,
+				       target_dcache_aspace_key);
+
+  return dcache;
 }
 
 /* Return the target dcache.  If it is not initialized yet, initialize
@@ -54,10 +75,13 @@ target_dcache_get (void)
 DCACHE *
 target_dcache_get_or_init (void)
 {
-  if (!target_dcache_init_p ())
-    target_dcache = dcache_init ();
+  DCACHE *dcache = address_space_data (current_program_space->aspace,
+				       target_dcache_aspace_key);
 
-  return target_dcache;
+  if (dcache == NULL)
+    dcache = dcache_init ();
+
+  return dcache;
 }
 
 /* The option sets this.  */
@@ -113,4 +137,8 @@ By default, caching for stack access is on."),
 			   set_stack_cache_enabled_p,
 			   show_stack_cache_enabled_p,
 			   &setlist, &showlist);
+
+  target_dcache_aspace_key
+    = register_address_space_data_with_cleanup (NULL,
+						target_dcache_cleanup);
 }
-- 
1.7.7.6

Re: [PATCH 07/10] Associate target_dcache to address_space.

[Yao Qi]

On 11/20/2013 12:44 PM, Yao Qi wrote:
>   target_dcache_get_or_init (void)
>   {
> -  if (!target_dcache_init_p ())
> -    target_dcache = dcache_init ();
> +  DCACHE *dcache = address_space_data (current_program_space->aspace,
> +				       target_dcache_aspace_key);
>   
> -  return target_dcache;
> +  if (dcache == NULL)
> +    dcache = dcache_init ();
> +
> +  return dcache;
>   }

Sigh,
set_address_space_data is missed after conflict resolution.  This patch
fixed it.  Committed.

-- 
Yao (齐尧)

gdb:

2013-11-20  Yao Qi  <yao@codesourcery.com>

	* target-dcache.c (target_dcache_get_or_init): Call
	set_address_space_data if 'dcache' is NULL.
---
 gdb/target-dcache.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/gdb/target-dcache.c b/gdb/target-dcache.c
index e1b0a56..58be6cd 100644
--- a/gdb/target-dcache.c
+++ b/gdb/target-dcache.c
@@ -79,7 +79,11 @@ target_dcache_get_or_init (void)
 				       target_dcache_aspace_key);
 
   if (dcache == NULL)
-    dcache = dcache_init ();
+    {
+      dcache = dcache_init ();
+      set_address_space_data (current_program_space->aspace,
+			      target_dcache_aspace_key, dcache);
+    }
 
   return dcache;
 }
-- 
1.7.7.6

[PATCH 08/10] Don't invalidate dcache when option stack-cache is changed

[Yao Qi]

Nowadays, option "stack-cache" on->off and off->on transitions trigger
cache invalidation.  It is not very necessary and not friendly to
using target_dcache for other purpose, such as code caching.

Option "stack-cache" can be regarded as a switch, to decide whether to
read through cache when reading data from target.  When this switch is
off, read from target.  When the switch is on, read from cache.  We
need to keep cache in sync, so when GDB writes data, cache should be
updated unconditionally.

gdb:

2013-11-02  Yao Qi  <yao@codesourcery.com>

	* target-dcache.c (stack_cache_enabled_p_1): Remove.
	(set_stack_cache_enabled_p): Remove.
	(_initialize_target_dcache): Update command register.
	* target.c (memory_xfer_partial_1): Don't check
	stack_cache_enabled.
---
 gdb/target-dcache.c |   24 ++++--------------------
 gdb/target.c        |    4 ++--
 2 files changed, 6 insertions(+), 22 deletions(-)

diff --git a/gdb/target-dcache.c b/gdb/target-dcache.c
index a840c40..ce36bbb 100644
--- a/gdb/target-dcache.c
+++ b/gdb/target-dcache.c
@@ -100,26 +100,10 @@ target_dcache_get_or_init (void)
 }
 
 /* The option sets this.  */
-static int stack_cache_enabled_p_1 = 1;
-/* And set_stack_cache_enabled_p updates this.
-   The reason for the separation is so that we don't flush the cache for
-   on->on transitions.  */
-static int stack_cache_enabled_p = 1;
-
-/* This is called *after* the stack-cache has been set.
-   Flush the cache for off->on and on->off transitions.
-   There's no real need to flush the cache for on->off transitions,
-   except cleanliness.  */
 
-static void
-set_stack_cache_enabled_p (char *args, int from_tty,
-			   struct cmd_list_element *c)
-{
-  if (stack_cache_enabled_p != stack_cache_enabled_p_1)
-    target_dcache_invalidate ();
+static int stack_cache_enabled_p = 1;
 
-  stack_cache_enabled_p = stack_cache_enabled_p_1;
-}
+/* Show option "stack-cache".  */
 
 static void
 show_stack_cache_enabled_p (struct ui_file *file, int from_tty,
@@ -143,13 +127,13 @@ void
 _initialize_target_dcache (void)
 {
   add_setshow_boolean_cmd ("stack-cache", class_support,
-			   &stack_cache_enabled_p_1, _("\
+			   &stack_cache_enabled_p, _("\
 Set cache use for stack access."), _("\
 Show cache use for stack access."), _("\
 When on, use the data cache for all stack access, regardless of any\n\
 configured memory regions.  This improves remote performance significantly.\n\
 By default, caching for stack access is on."),
-			   set_stack_cache_enabled_p,
+			   NULL,
 			   show_stack_cache_enabled_p,
 			   &setlist, &showlist);
 
diff --git a/gdb/target.c b/gdb/target.c
index 1212347..55c5d78 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1593,14 +1593,14 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
 
   /* Make sure the cache gets updated no matter what - if we are writing
      to the stack.  Even if this write is not tagged as such, we still need
-     to update the cache.  */
+     to update the cache.  Update the cache to keep it in sync if it
+     has been initialized.  */
 
   if (res > 0
       && inf != NULL
       && writebuf != NULL
       && target_dcache_init_p ()
       && !region->attrib.cache
-      && stack_cache_enabled ()
       && object != TARGET_OBJECT_STACK_MEMORY)
     {
       DCACHE *dcache = target_dcache_get ();
-- 
1.7.7.6

Re: [PATCH 08/10] Don't invalidate dcache when option stack-cache is changed

[Doug Evans]

On Sat, Nov 2, 2013 at 10:54 PM, Yao Qi <yao@codesourcery.com> wrote:
> Nowadays, option "stack-cache" on->off and off->on transitions trigger
> cache invalidation.  It is not very necessary and not friendly to
> using target_dcache for other purpose, such as code caching.
>
> Option "stack-cache" can be regarded as a switch, to decide whether to
> read through cache when reading data from target.  When this switch is
> off, read from target.  When the switch is on, read from cache.  We
> need to keep cache in sync, so when GDB writes data, cache should be
> updated unconditionally.
>
> gdb:
>
> 2013-11-02  Yao Qi  <yao@codesourcery.com>
>
>         * target-dcache.c (stack_cache_enabled_p_1): Remove.
>         (set_stack_cache_enabled_p): Remove.
>         (_initialize_target_dcache): Update command register.
>         * target.c (memory_xfer_partial_1): Don't check
>         stack_cache_enabled.
> ---
>  gdb/target-dcache.c |   24 ++++--------------------
>  gdb/target.c        |    4 ++--
>  2 files changed, 6 insertions(+), 22 deletions(-)
>
> diff --git a/gdb/target-dcache.c b/gdb/target-dcache.c
> index a840c40..ce36bbb 100644
> --- a/gdb/target-dcache.c
> +++ b/gdb/target-dcache.c
> @@ -100,26 +100,10 @@ target_dcache_get_or_init (void)
>  }
>
>  /* The option sets this.  */
> -static int stack_cache_enabled_p_1 = 1;
> -/* And set_stack_cache_enabled_p updates this.
> -   The reason for the separation is so that we don't flush the cache for
> -   on->on transitions.  */
> -static int stack_cache_enabled_p = 1;
> -
> -/* This is called *after* the stack-cache has been set.
> -   Flush the cache for off->on and on->off transitions.
> -   There's no real need to flush the cache for on->off transitions,
> -   except cleanliness.  */
>
> -static void
> -set_stack_cache_enabled_p (char *args, int from_tty,
> -                          struct cmd_list_element *c)
> -{
> -  if (stack_cache_enabled_p != stack_cache_enabled_p_1)
> -    target_dcache_invalidate ();
> +static int stack_cache_enabled_p = 1;
>
> -  stack_cache_enabled_p = stack_cache_enabled_p_1;
> -}
> +/* Show option "stack-cache".  */
>
>  static void
>  show_stack_cache_enabled_p (struct ui_file *file, int from_tty,
> @@ -143,13 +127,13 @@ void
>  _initialize_target_dcache (void)
>  {
>    add_setshow_boolean_cmd ("stack-cache", class_support,
> -                          &stack_cache_enabled_p_1, _("\
> +                          &stack_cache_enabled_p, _("\
>  Set cache use for stack access."), _("\
>  Show cache use for stack access."), _("\
>  When on, use the data cache for all stack access, regardless of any\n\
>  configured memory regions.  This improves remote performance significantly.\n\
>  By default, caching for stack access is on."),
> -                          set_stack_cache_enabled_p,
> +                          NULL,
>                            show_stack_cache_enabled_p,
>                            &setlist, &showlist);
>

I'm still not comfortable with this.
This is optimizing a rare occurrence.  Users aren't expected to be
turning this on and off during a session.

If one wanted to remove the cache invalidation for the off->on
transition that seems reasonable, but if I do a backtrace, turn the
caching optimization off, and then do another backtrace, I'd want the
second one to not use the cache.  YMMV.

OTOH, if there was a command I could use to flush the cache after
turning off the stack cache optimization, then I could see leaving the
cache alone after the on->off transition.  [I still think it'd be
preferable to invalidate the cache, but I can live with asking users
to live with having to manually flush the cache after turning off the
optimization.]

> diff --git a/gdb/target.c b/gdb/target.c
> index 1212347..55c5d78 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -1593,14 +1593,14 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
>
>    /* Make sure the cache gets updated no matter what - if we are writing
>       to the stack.  Even if this write is not tagged as such, we still need
> -     to update the cache.  */
> +     to update the cache.  Update the cache to keep it in sync if it
> +     has been initialized.  */
>
>    if (res > 0
>        && inf != NULL
>        && writebuf != NULL
>        && target_dcache_init_p ()
>        && !region->attrib.cache
> -      && stack_cache_enabled ()
>        && object != TARGET_OBJECT_STACK_MEMORY)
>      {
>        DCACHE *dcache = target_dcache_get ();

This part of the patch seems ok though.

Re: [PATCH 08/10] Don't invalidate dcache when option stack-cache is changed

[Yao Qi]

On 11/18/2013 05:44 AM, Doug Evans wrote:
> If one wanted to remove the cache invalidation for the off->on
> transition that seems reasonable, but if I do a backtrace, turn the
> caching optimization off, and then do another backtrace, I'd want the
> second one to not use the cache.  YMMV.

This behaviour is what we proposed in this patch.

>
> OTOH, if there was a command I could use to flush the cache after
> turning off the stack cache optimization, then I could see leaving the
> cache alone after the on->off transition.  [I still think it'd be
> preferable to invalidate the cache, but I can live with asking users
> to live with having to manually flush the cache after turning off the
> optimization.]

IMO, cache to target memory is quite transparent to users.  I can't
find a case that user has to flush or invalidate cache.

With this patch, we are lack of a way to flush the cache.  I prefer to 
add a new command to explicitly invalidate cache like "cache invalidate" 
or "maint cache invalidate", if we think users really need this command.

-- 
Yao (齐尧)

Re: [PATCH 08/10] Don't invalidate dcache when option stack-cache is changed

[Pedro Alves]

On 11/17/2013 09:44 PM, Doug Evans wrote:

> I'm still not comfortable with this.
> This is optimizing a rare occurrence.  Users aren't expected to be
> turning this on and off during a session.

My thoughts exactly.  This seems to be adding extra code, therefore
a wider surface for bugs, for no apparent real use case?

-- 
Pedro Alves

Re: [PATCH 08/10] Don't invalidate dcache when option stack-cache is changed

[Yao Qi]

On 11/18/2013 11:43 PM, Pedro Alves wrote:
> On 11/17/2013 09:44 PM, Doug Evans wrote:
>
>> I'm still not comfortable with this.
>> This is optimizing a rare occurrence.  Users aren't expected to be
>> turning this on and off during a session.
>
> My thoughts exactly.  This seems to be adding extra code, therefore
> a wider surface for bugs, for no apparent real use case?
>

The motivation of this patch is not about performance improvement, but
about using target_dcache for code caching, as I described in the mail
below...

> Nowadays, option "stack-cache" on->off and off->on transitions trigger
> cache invalidation.  It is not very necessary and not friendly to
> using target_dcache for other purpose, such as code caching.

... here.

We've decided to use a single global target_dcache for both stack
caching and code caching.  However, it is odd to me that option
"stack-cache" transitions invalidate cache for code.  If the behaviour 
like this is acceptable to you, I am OK to keep the existing behaviour 
unchanged, and add option "code-cache".  Finally, either option 
"stack-cache" or option "code-cache" transitions will invalidate the 
global cache.  What do you think?

-- 
Yao (齐尧)

Re: [PATCH 08/10] Don't invalidate dcache when option stack-cache is changed

[Pedro Alves]

I realize I didn't reply to this part, when I had meant to.

On 11/17/2013 09:44 PM, Doug Evans wrote:
> If one wanted to remove the cache invalidation for the off->on
> transition that seems reasonable, but if I do a backtrace, turn the
> caching optimization off, and then do another backtrace, I'd want the
> second one to not use the cache.  YMMV.

I definitely agree.

Actually, doesn't the patch actually introduce a bug, like:

#1 - enable stack cache
#2 - print stack variable
#3 - disable cache
#4 - write to variable.  As the cache is off, and

 @@ -1593,14 +1593,14 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,

   /* Make sure the cache gets updated no matter what - if we are writing
      to the stack.  Even if this write is not tagged as such, we still need
 -     to update the cache.  */
 +     to update the cache.  Update the cache to keep it in sync if it
 +     has been initialized.  */

   if (res > 0
       && inf != NULL
       && writebuf != NULL
       && target_dcache_init_p ()
       && !region->attrib.cache
 -      && stack_cache_enabled ()
       && object != TARGET_OBJECT_STACK_MEMORY)
     {

... we don't update the cache.

#5 - enable cache
#6 - print variable again.  The stale cached value is printed.

?

-- 
Pedro Alves

Re: [PATCH 08/10] Don't invalidate dcache when option stack-cache is changed

[Yao Qi]

On 11/19/2013 07:52 PM, Pedro Alves wrote:
>   @@ -1593,14 +1593,14 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
>
>     /* Make sure the cache gets updated no matter what - if we are writing
>        to the stack.  Even if this write is not tagged as such, we still need
>   -     to update the cache.  */
>   +     to update the cache.  Update the cache to keep it in sync if it
>   +     has been initialized.  */
>
>     if (res > 0
>         && inf != NULL
>         && writebuf != NULL
>         && target_dcache_init_p ()
>         && !region->attrib.cache
>   -      && stack_cache_enabled ()
>         && object != TARGET_OBJECT_STACK_MEMORY)
>       {
>
> ... we don't update the cache.

We do update the cache.  When we do "set variable foo = 1" (foo is a 
local variable), OBJECT is TARGET_OBJECT_MEMORY, so the condition above 
is true and cache is updated then.

-- 
Yao (齐尧)

[PATCH 09/10] set/show code-cache

[Yao Qi]

Similar to stack cache, in this patch, we add
TARGET_OBJECT_CODE_MEMORY to read code from target and add a new
option "set code-cache on|off" to control use code cache or not.

gdb:

2013-11-02  Yao Qi  <yao@codesourcery.com>

	* NEWS: Add note on new "set code-cache" option.
	* target-dcache.c (code_cache_enabled_p): New.
	(show_code_cache_enabled_p): New function.
	(code_cache_enabled): New function.
	(_initialize_target_dcache): Register command.
	* target-dcache.h (code_cache_enabled): Declare.
	* target.c (memory_xfer_partial_1):Handle
	TARGET_OBJECT_CODE_MEMORY and code_cache_enabled.
	(target_read_code): New function.
	* target.h (enum target_object) <TARGET_OBJECT_CODE_MEMORY>:
	New.
	(target_read_code): Declare.

gdb/doc:

2013-11-02  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (Caching Remote Data): Document new
	`set/show stack-cache' option.
---
 gdb/NEWS            |    6 ++++++
 gdb/doc/gdb.texinfo |   15 ++++++++++++++-
 gdb/target-dcache.c |   32 ++++++++++++++++++++++++++++++++
 gdb/target-dcache.h |    2 ++
 gdb/target.c        |   22 +++++++++++++++++++---
 gdb/target.h        |    5 +++++
 6 files changed, 78 insertions(+), 4 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index efeda68..dad167c 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -109,6 +109,12 @@ show startup-with-shell
   Specifies whether Unix child processes are started via a shell or
   directly.
 
+set code-cache
+show code-cache
+  Use more aggressive caching for accesses to the code segment.  This
+  improves performance of remote debugging (particularly disassembly)
+  without affecting correctness.
+
 * You can now use a literal value 'unlimited' for options that
   interpret 0 or -1 as meaning "unlimited".  E.g., "set
   trace-buffer-size unlimited" is now an alias for "set
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a64bb0b..405e4b5 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -10826,7 +10826,8 @@ Therefore, by default, @value{GDBN} only caches data
 known to be on the stack@footnote{In non-stop mode, it is moderately
 rare for a running thread to modify the stack of a stopped thread
 in a way that would interfere with a backtrace, and caching of
-stack reads provides a significant speed up of remote backtraces.}.
+stack reads provides a significant speed up of remote backtraces.} or
+in the code segment.
 Other regions of memory can be explicitly marked as
 cacheable; see @pxref{Memory Region Attributes}.
 
@@ -10851,6 +10852,18 @@ caching.  By default, this option is @code{ON}.
 @item show stack-cache
 Show the current state of data caching for memory accesses.
 
+@kindex set code-cache
+@item set code-cache on
+@itemx set code-cache off
+Enable or disable caching of code segment accesses.  When @code{ON},
+use caching.  By default, this option is @code{ON}.  This improves
+performance of disassembly in remote debugging without affecting
+correctness.
+
+@kindex show code-cache
+@item show code-cache
+Show the current state of data caching for code segment accesses.
+
 @kindex info dcache
 @item info dcache @r{[}line@r{]}
 Print the information about the performance of data cache of the
diff --git a/gdb/target-dcache.c b/gdb/target-dcache.c
index ce36bbb..e6d11b9 100644
--- a/gdb/target-dcache.c
+++ b/gdb/target-dcache.c
@@ -120,6 +120,27 @@ stack_cache_enabled (void)
   return stack_cache_enabled_p;
 }
 
+/* The option sets this.  */
+
+static int code_cache_enabled_p = 1;
+
+/* Show option "code-cache".  */
+
+static void
+show_code_cache_enabled_p (struct ui_file *file, int from_tty,
+			    struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("Cache use for code accesses is %s.\n"), value);
+}
+
+/* Return true if "stack cache" is enabled, otherwise, return false.  */
+
+int
+code_cache_enabled (void)
+{
+  return code_cache_enabled_p;
+}
+
 /* -Wmissing-prototypes */
 extern initialize_file_ftype _initialize_target_dcache;
 
@@ -137,6 +158,17 @@ By default, caching for stack access is on."),
 			   show_stack_cache_enabled_p,
 			   &setlist, &showlist);
 
+  add_setshow_boolean_cmd ("code-cache", class_support,
+			   &code_cache_enabled_p, _("\
+Set cache use for code segment access."), _("\
+Show cache use for code segment access."), _("\
+When on, use the data cache for all code segment access, regardless\n\
+of any configured memory regions.  This improves remote performance\n\
+significantly.  By default, caching for code segment access is on."),
+			   NULL,
+			   show_code_cache_enabled_p,
+			   &setlist, &showlist);
+
   target_dcache_aspace_key
     = register_address_space_data_with_cleanup (NULL,
 						target_dcache_cleanup);
diff --git a/gdb/target-dcache.h b/gdb/target-dcache.h
index 06a938f..17c61a1 100644
--- a/gdb/target-dcache.h
+++ b/gdb/target-dcache.h
@@ -26,3 +26,5 @@ extern DCACHE *target_dcache_get_or_init (void);
 extern int target_dcache_init_p (void);
 
 extern int stack_cache_enabled (void);
+
+extern int code_cache_enabled (void);
diff --git a/gdb/target.c b/gdb/target.c
index 55c5d78..6bdc671 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1547,7 +1547,8 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
 	 the collected memory range fails.  */
       && get_traceframe_number () == -1
       && (region->attrib.cache
-	  || (stack_cache_enabled () && object == TARGET_OBJECT_STACK_MEMORY)))
+	  || (stack_cache_enabled () && object == TARGET_OBJECT_STACK_MEMORY)
+	  || (code_cache_enabled () && object == TARGET_OBJECT_CODE_MEMORY)))
     {
       DCACHE *dcache = target_dcache_get_or_init ();
 
@@ -1601,7 +1602,8 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
       && writebuf != NULL
       && target_dcache_init_p ()
       && !region->attrib.cache
-      && object != TARGET_OBJECT_STACK_MEMORY)
+      && object != TARGET_OBJECT_STACK_MEMORY
+      && object != TARGET_OBJECT_CODE_MEMORY)
     {
       DCACHE *dcache = target_dcache_get ();
 
@@ -1690,7 +1692,8 @@ target_xfer_partial (struct target_ops *ops,
   /* If this is a memory transfer, let the memory-specific code
      have a look at it instead.  Memory transfers are more
      complicated.  */
-  if (object == TARGET_OBJECT_MEMORY || object == TARGET_OBJECT_STACK_MEMORY)
+  if (object == TARGET_OBJECT_MEMORY || object == TARGET_OBJECT_STACK_MEMORY
+      || object == TARGET_OBJECT_CODE_MEMORY)
     retval = memory_xfer_partial (ops, object, readbuf,
 				  writebuf, offset, len);
   else
@@ -1792,6 +1795,19 @@ target_read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
     return TARGET_XFER_E_IO;
 }
 
+/* Like target_read_memory, but specify explicitly that this is a read from
+   the target's code.  This may trigger different cache behavior.  */
+
+int
+target_read_code (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
+{
+  if (target_read (current_target.beneath, TARGET_OBJECT_CODE_MEMORY, NULL,
+		   myaddr, memaddr, len) == len)
+    return 0;
+  else
+    return TARGET_XFER_E_IO;
+}
+
 /* Write LEN bytes from MYADDR to target memory at address MEMADDR.
    Returns either 0 for success or a target_xfer_error value if any
    error occurs.  If an error occurs, no guarantee is made about how
diff --git a/gdb/target.h b/gdb/target.h
index 7a93646..7f43b4e 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -145,6 +145,9 @@ enum target_object
      if it is not in a region marked as such, since it is known to be
      "normal" RAM.  */
   TARGET_OBJECT_STACK_MEMORY,
+  /* Memory known to be part of the target code.   This is cached even
+     if it is not in a region marked as such.  */
+  TARGET_OBJECT_CODE_MEMORY,
   /* Kernel Unwind Table.  See "ia64-tdep.c".  */
   TARGET_OBJECT_UNWIND_TABLE,
   /* Transfer auxilliary vector.  */
@@ -1050,6 +1053,8 @@ extern int target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr,
 
 extern int target_read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len);
 
+extern int target_read_code (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len);
+
 extern int target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr,
 				ssize_t len);
 
-- 
1.7.7.6

Re: [PATCH 09/10] set/show code-cache

[Eli Zaretskii]

> From: Yao Qi <yao@codesourcery.com>
> Date: Sun, 3 Nov 2013 13:54:09 +0800
> 
> Similar to stack cache, in this patch, we add
> TARGET_OBJECT_CODE_MEMORY to read code from target and add a new
> option "set code-cache on|off" to control use code cache or not.
> 
> gdb:
> 
> 2013-11-02  Yao Qi  <yao@codesourcery.com>
> 
> 	* NEWS: Add note on new "set code-cache" option.
> 	* target-dcache.c (code_cache_enabled_p): New.
> 	(show_code_cache_enabled_p): New function.
> 	(code_cache_enabled): New function.
> 	(_initialize_target_dcache): Register command.
> 	* target-dcache.h (code_cache_enabled): Declare.
> 	* target.c (memory_xfer_partial_1):Handle
> 	TARGET_OBJECT_CODE_MEMORY and code_cache_enabled.
> 	(target_read_code): New function.
> 	* target.h (enum target_object) <TARGET_OBJECT_CODE_MEMORY>:
> 	New.
> 	(target_read_code): Declare.
> 
> gdb/doc:
> 
> 2013-11-02  Yao Qi  <yao@codesourcery.com>
> 
> 	* gdb.texinfo (Caching Remote Data): Document new
> 	`set/show stack-cache' option.

The documentation parts are OK, thanks.

Re: [PATCH 09/10] set/show code-cache

[Doug Evans]

On Sat, Nov 2, 2013 at 10:54 PM, Yao Qi <yao@codesourcery.com> wrote:
> Similar to stack cache, in this patch, we add
> TARGET_OBJECT_CODE_MEMORY to read code from target and add a new
> option "set code-cache on|off" to control use code cache or not.

To help avoid future confusion, I think it would be useful to rephrase this as:

"[...] and add a new option "set code-cache on|off" to optimize code
accesses by using the target memory cache."

I'd be happy with a better name for code-cache too if that helped.
"code-cache" is good enough though.

>
> gdb:
>
> 2013-11-02  Yao Qi  <yao@codesourcery.com>
>
>         * NEWS: Add note on new "set code-cache" option.
>         * target-dcache.c (code_cache_enabled_p): New.
>         (show_code_cache_enabled_p): New function.
>         (code_cache_enabled): New function.
>         (_initialize_target_dcache): Register command.
>         * target-dcache.h (code_cache_enabled): Declare.
>         * target.c (memory_xfer_partial_1):Handle
>         TARGET_OBJECT_CODE_MEMORY and code_cache_enabled.
>         (target_read_code): New function.
>         * target.h (enum target_object) <TARGET_OBJECT_CODE_MEMORY>:
>         New.
>         (target_read_code): Declare.
>
> gdb/doc:
>
> 2013-11-02  Yao Qi  <yao@codesourcery.com>
>
>         * gdb.texinfo (Caching Remote Data): Document new
>         `set/show stack-cache' option.
> ---
>  gdb/NEWS            |    6 ++++++
>  gdb/doc/gdb.texinfo |   15 ++++++++++++++-
>  gdb/target-dcache.c |   32 ++++++++++++++++++++++++++++++++
>  gdb/target-dcache.h |    2 ++
>  gdb/target.c        |   22 +++++++++++++++++++---
>  gdb/target.h        |    5 +++++
>  6 files changed, 78 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index efeda68..dad167c 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -109,6 +109,12 @@ show startup-with-shell
>    Specifies whether Unix child processes are started via a shell or
>    directly.
>
> +set code-cache
> +show code-cache
> +  Use more aggressive caching for accesses to the code segment.  This
> +  improves performance of remote debugging (particularly disassembly)
> +  without affecting correctness.

I would rephrase this as:

Use the target memory cache for accesses to the code segment.
This improves the performance of remote debugging (particularly disassembly)
without affecting correctness.

> +
>  * You can now use a literal value 'unlimited' for options that
>    interpret 0 or -1 as meaning "unlimited".  E.g., "set
>    trace-buffer-size unlimited" is now an alias for "set
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index a64bb0b..405e4b5 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -10826,7 +10826,8 @@ Therefore, by default, @value{GDBN} only caches data
>  known to be on the stack@footnote{In non-stop mode, it is moderately
>  rare for a running thread to modify the stack of a stopped thread
>  in a way that would interfere with a backtrace, and caching of
> -stack reads provides a significant speed up of remote backtraces.}.
> +stack reads provides a significant speed up of remote backtraces.} or
> +in the code segment.
>  Other regions of memory can be explicitly marked as
>  cacheable; see @pxref{Memory Region Attributes}.
>
> @@ -10851,6 +10852,18 @@ caching.  By default, this option is @code{ON}.
>  @item show stack-cache
>  Show the current state of data caching for memory accesses.
>
> +@kindex set code-cache
> +@item set code-cache on
> +@itemx set code-cache off
> +Enable or disable caching of code segment accesses.  When @code{ON},
> +use caching.  By default, this option is @code{ON}.  This improves
> +performance of disassembly in remote debugging without affecting
> +correctness.
> +
> +@kindex show code-cache
> +@item show code-cache
> +Show the current state of data caching for code segment accesses.
> +
>  @kindex info dcache
>  @item info dcache @r{[}line@r{]}
>  Print the information about the performance of data cache of the
> diff --git a/gdb/target-dcache.c b/gdb/target-dcache.c
> index ce36bbb..e6d11b9 100644
> --- a/gdb/target-dcache.c
> +++ b/gdb/target-dcache.c
> @@ -120,6 +120,27 @@ stack_cache_enabled (void)
>    return stack_cache_enabled_p;
>  }
>
> +/* The option sets this.  */
> +
> +static int code_cache_enabled_p = 1;
> +
> +/* Show option "code-cache".  */
> +
> +static void
> +show_code_cache_enabled_p (struct ui_file *file, int from_tty,
> +                           struct cmd_list_element *c, const char *value)
> +{
> +  fprintf_filtered (file, _("Cache use for code accesses is %s.\n"), value);
> +}
> +
> +/* Return true if "stack cache" is enabled, otherwise, return false.  */
> +
> +int
> +code_cache_enabled (void)
> +{
> +  return code_cache_enabled_p;
> +}

IWBN to name the function code_cache_enabled_p, but it could be left for later.

> +
>  /* -Wmissing-prototypes */
>  extern initialize_file_ftype _initialize_target_dcache;
>
> @@ -137,6 +158,17 @@ By default, caching for stack access is on."),
>                            show_stack_cache_enabled_p,
>                            &setlist, &showlist);
>
> +  add_setshow_boolean_cmd ("code-cache", class_support,
> +                          &code_cache_enabled_p, _("\
> +Set cache use for code segment access."), _("\
> +Show cache use for code segment access."), _("\
> +When on, use the data cache for all code segment access, regardless\n\
> +of any configured memory regions.  This improves remote performance\n\
> +significantly.  By default, caching for code segment access is on."),
> +                          NULL,
> +                          show_code_cache_enabled_p,
> +                          &setlist, &showlist);

This would be clearer if "data cache" was replaced with "target memory cache".
OTOH, we use "dcache" in the various option names.  Blech.
[Ok to leave the wording as is, just wanted to make this comment.]

> +
>    target_dcache_aspace_key
>      = register_address_space_data_with_cleanup (NULL,
>                                                 target_dcache_cleanup);
> diff --git a/gdb/target-dcache.h b/gdb/target-dcache.h
> index 06a938f..17c61a1 100644
> --- a/gdb/target-dcache.h
> +++ b/gdb/target-dcache.h
> @@ -26,3 +26,5 @@ extern DCACHE *target_dcache_get_or_init (void);
>  extern int target_dcache_init_p (void);
>
>  extern int stack_cache_enabled (void);
> +
> +extern int code_cache_enabled (void);
> diff --git a/gdb/target.c b/gdb/target.c
> index 55c5d78..6bdc671 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -1547,7 +1547,8 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
>          the collected memory range fails.  */
>        && get_traceframe_number () == -1
>        && (region->attrib.cache
> -         || (stack_cache_enabled () && object == TARGET_OBJECT_STACK_MEMORY)))
> +         || (stack_cache_enabled () && object == TARGET_OBJECT_STACK_MEMORY)
> +         || (code_cache_enabled () && object == TARGET_OBJECT_CODE_MEMORY)))
>      {
>        DCACHE *dcache = target_dcache_get_or_init ();
>
> @@ -1601,7 +1602,8 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
>        && writebuf != NULL
>        && target_dcache_init_p ()
>        && !region->attrib.cache
> -      && object != TARGET_OBJECT_STACK_MEMORY)
> +      && object != TARGET_OBJECT_STACK_MEMORY
> +      && object != TARGET_OBJECT_CODE_MEMORY)
>      {
>        DCACHE *dcache = target_dcache_get ();
>
> @@ -1690,7 +1692,8 @@ target_xfer_partial (struct target_ops *ops,
>    /* If this is a memory transfer, let the memory-specific code
>       have a look at it instead.  Memory transfers are more
>       complicated.  */
> -  if (object == TARGET_OBJECT_MEMORY || object == TARGET_OBJECT_STACK_MEMORY)
> +  if (object == TARGET_OBJECT_MEMORY || object == TARGET_OBJECT_STACK_MEMORY
> +      || object == TARGET_OBJECT_CODE_MEMORY)
>      retval = memory_xfer_partial (ops, object, readbuf,
>                                   writebuf, offset, len);
>    else
> @@ -1792,6 +1795,19 @@ target_read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
>      return TARGET_XFER_E_IO;
>  }
>
> +/* Like target_read_memory, but specify explicitly that this is a read from
> +   the target's code.  This may trigger different cache behavior.  */
> +
> +int
> +target_read_code (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
> +{
> +  if (target_read (current_target.beneath, TARGET_OBJECT_CODE_MEMORY, NULL,
> +                  myaddr, memaddr, len) == len)
> +    return 0;
> +  else
> +    return TARGET_XFER_E_IO;
> +}
> +
>  /* Write LEN bytes from MYADDR to target memory at address MEMADDR.
>     Returns either 0 for success or a target_xfer_error value if any
>     error occurs.  If an error occurs, no guarantee is made about how
> diff --git a/gdb/target.h b/gdb/target.h
> index 7a93646..7f43b4e 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -145,6 +145,9 @@ enum target_object
>       if it is not in a region marked as such, since it is known to be
>       "normal" RAM.  */
>    TARGET_OBJECT_STACK_MEMORY,
> +  /* Memory known to be part of the target code.   This is cached even
> +     if it is not in a region marked as such.  */
> +  TARGET_OBJECT_CODE_MEMORY,
>    /* Kernel Unwind Table.  See "ia64-tdep.c".  */
>    TARGET_OBJECT_UNWIND_TABLE,
>    /* Transfer auxilliary vector.  */
> @@ -1050,6 +1053,8 @@ extern int target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr,
>
>  extern int target_read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len);
>
> +extern int target_read_code (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len);
> +
>  extern int target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr,
>                                 ssize_t len);
>

I don't see target_read_code called from anywhere but I'm guessing
that's in 10/10.

Patch is ok to me (with the suggested wording change to the NEWS entry).

[PATCH 04/10] Don't stress 'remote' in "Data Caching" in doc

[Yao Qi]

When I try to describe the cache and its related commands (in a
cache-per-address-space world), I find hard to add, because
existing doc is focused on remote debugging, while data cache is used
regardless of the target.  More precisely, GDB caches target data,
instead of remote data.

gdb/doc:

2013-11-02  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (Data): Rename menu item.
	(Caching Remote Data): Rename to ...
	(Caching Target Data): ... it.  Update.
---
 gdb/doc/gdb.texinfo |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 28e6ff9..4d72983 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -8064,7 +8064,7 @@ being passed the type of @var{arg} as the argument.
 * Core File Generation::        Cause a program dump its core
 * Character Sets::              Debugging programs that use a different
                                 character set than GDB does
-* Caching Remote Data::         Data caching for remote targets
+* Caching Target Data::         Data caching for targets
 * Searching Memory::            Searching memory for a sequence of bytes
 @end menu
 
@@ -10808,13 +10808,13 @@ $10 = 78 '+'
 The @sc{ibm1047} character set uses the number 78 to encode the @samp{+}
 character.
 
-@node Caching Remote Data
-@section Caching Data of Remote Targets
-@cindex caching data of remote targets
+@node Caching Target Data
+@section Caching Data of Targets
+@cindex caching data of targets
 
-@value{GDBN} caches data exchanged between the debugger and a
-remote target (@pxref{Remote Debugging}).  Such caching generally improves
-performance, because it reduces the overhead of the remote protocol by
+@value{GDBN} caches data exchanged between the debugger and a target.
+Such caching generally improves performance in @ref{Remote Debugging},
+because it reduces the overhead of the remote protocol by
 bundling memory reads and writes into large chunks.  Unfortunately, simply
 caching everything would lead to incorrect results, since @value{GDBN} 
 does not necessarily know anything about volatile values, memory-mapped I/O
@@ -10873,11 +10873,11 @@ Must be a power of 2.
 
 @item show dcache size
 @kindex show dcache size
-Show maximum number of dcache entries.  See also @ref{Caching Remote Data, info dcache}.
+Show maximum number of dcache entries.  See also @ref{Caching Target Data, info dcache}.
 
 @item show dcache line-size
 @kindex show dcache line-size
-Show default size of dcache lines.  See also @ref{Caching Remote Data, info dcache}.
+Show default size of dcache lines.  See also @ref{Caching Target Data, info dcache}.
 
 @end table
 
-- 
1.7.7.6

Re: [PATCH 04/10] Don't stress 'remote' in "Data Caching" in doc

[Eli Zaretskii]

> From: Yao Qi <yao@codesourcery.com>
> Date: Sun, 3 Nov 2013 13:54:04 +0800
> 
> When I try to describe the cache and its related commands (in a
> cache-per-address-space world), I find hard to add, because
> existing doc is focused on remote debugging, while data cache is used
> regardless of the target.  More precisely, GDB caches target data,
> instead of remote data.

Thanks.  But may I ask in the future not to split the patches to
documentation that are related to the same series?  When you split
them, it makes the review harder, as I see the documentation changes
piecemeal, rather than together.

> +@value{GDBN} caches data exchanged between the debugger and a target.
> +Such caching generally improves performance in @ref{Remote Debugging},

Please don't use @ref as if it were an HTML hyperlink.  The results
look awkward in certain formats (this one will look awkward in Info).
Instead, please say something like

  Such caching generally improves performance in remote debugging
  (@pxref{Remote Debugging}).

> -Show maximum number of dcache entries.  See also @ref{Caching Remote Data, info dcache}.
> +Show maximum number of dcache entries.  See also @ref{Caching Target Data, info dcache}.

Likewise.  Here, I would simply use @xref:

  @xref{Caching Target Data, info dcache}.

>  @item show dcache line-size
>  @kindex show dcache line-size
> -Show default size of dcache lines.  See also @ref{Caching Remote Data, info dcache}.
> +Show default size of dcache lines.  See also @ref{Caching Target Data, info dcache}.

This cross-reference is simply redundant (you just have it in the
previous sentence), and should be removed.

OK with those changes.

Re: [PATCH 04/10] Don't stress 'remote' in "Data Caching" in doc

[Yao Qi]

On 11/04/2013 12:55 AM, Eli Zaretskii wrote:
> Thanks.  But may I ask in the future not to split the patches to
> documentation that are related to the same series?  When you split
> them, it makes the review harder, as I see the documentation changes
> piecemeal, rather than together.

What I want is to keep doc in sync with the code in every change/patch. 
  I'll put all doc stuff in a single patch from now on.  Sorry for the 
inconvenience.

-- 
Yao (齐尧)

Re: [PATCH 04/10] Don't stress 'remote' in "Data Caching" in doc

[Eli Zaretskii]

> Date: Wed, 6 Nov 2013 15:06:38 +0800
> From: Yao Qi <yao@codesourcery.com>
> CC: <gdb-patches@sourceware.org>
> 
> I'll put all doc stuff in a single patch from now on.

Thank you.

> Sorry for the inconvenience.

It's only a minor one, so no need to feel sorry.

Re: [PATCH 04/10] Don't stress 'remote' in "Data Caching" in doc

[Doug Evans]

On Sun, Nov 3, 2013 at 8:55 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Yao Qi <yao@codesourcery.com>
>> Date: Sun, 3 Nov 2013 13:54:04 +0800
>>
>> When I try to describe the cache and its related commands (in a
>> cache-per-address-space world), I find hard to add, because
>> existing doc is focused on remote debugging, while data cache is used
>> regardless of the target.  More precisely, GDB caches target data,
>> instead of remote data.
>
> Thanks.  But may I ask in the future not to split the patches to
> documentation that are related to the same series?  When you split
> them, it makes the review harder, as I see the documentation changes
> piecemeal, rather than together.

That may be hard to apply in general.

For code we ask people to split such things out.
I can well imagine people applying the same logic to documentation.
I don't know that it necessarily applies here, but it could.

Re: [PATCH 04/10] Don't stress 'remote' in "Data Caching" in doc

[Eli Zaretskii]

> Date: Sun, 17 Nov 2013 12:15:10 -0800
> From: Doug Evans <dje@google.com>
> Cc: Yao Qi <yao@codesourcery.com>, gdb-patches <gdb-patches@sourceware.org>
> 
> > Thanks.  But may I ask in the future not to split the patches to
> > documentation that are related to the same series?  When you split
> > them, it makes the review harder, as I see the documentation changes
> > piecemeal, rather than together.
> 
> That may be hard to apply in general.

I don't see why it would be.  Can you elaborate?

> For code we ask people to split such things out.
> I can well imagine people applying the same logic to documentation.
> I don't know that it necessarily applies here, but it could.

Sorry, I don't understand: what logic?

What I'm asking is not request me to review a 15-line change to
documentation in 5 3-line pieces.

Re: [PATCH 04/10] Don't stress 'remote' in "Data Caching" in doc

[Doug Evans]

On Sun, Nov 17, 2013 at 1:21 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Sun, 17 Nov 2013 12:15:10 -0800
>> From: Doug Evans <dje@google.com>
>> Cc: Yao Qi <yao@codesourcery.com>, gdb-patches <gdb-patches@sourceware.org>
>>
>> > Thanks.  But may I ask in the future not to split the patches to
>> > documentation that are related to the same series?  When you split
>> > them, it makes the review harder, as I see the documentation changes
>> > piecemeal, rather than together.
>>
>> That may be hard to apply in general.
>
> I don't see why it would be.  Can you elaborate?

We actively ask people to do the opposite for code.
So we would have one rule for code and the opposite rule for docs.
Sometimes a patch series will have several doc additions, that while
collectively may appear as one doc patch, the submitter chose to break
them up to keep them with their respective code parts.
I think it should be ok if someone did that ... we have a lot of rules
to what is an acceptable patch already.

>> For code we ask people to split such things out.
>> I can well imagine people applying the same logic to documentation.
>> I don't know that it necessarily applies here, but it could.
>
> Sorry, I don't understand: what logic?
>
> What I'm asking is not request me to review a 15-line change to
> documentation in 5 3-line pieces.

See my point above.

Can I suggest that we allow any GM to approve doc changes.
We need all the review bandwidth we can get.
And *even* if they make mistakes sometimes, *that's ok*'.  Even better.
Mistakes are great teachers (if handled appropriately). :-)

Re: [PATCH 04/10] Don't stress 'remote' in "Data Caching" in doc

[Eli Zaretskii]

> Date: Tue, 19 Nov 2013 18:16:46 -0800
> From: Doug Evans <dje@google.com>
> Cc: Yao Qi <yao@codesourcery.com>, gdb-patches <gdb-patches@sourceware.org>
> 
> >> > Thanks.  But may I ask in the future not to split the patches to
> >> > documentation that are related to the same series?  When you split
> >> > them, it makes the review harder, as I see the documentation changes
> >> > piecemeal, rather than together.
> >>
> >> That may be hard to apply in general.
> >
> > I don't see why it would be.  Can you elaborate?
> 
> We actively ask people to do the opposite for code.

I don't understand why, but I won't argue about that part.

> So we would have one rule for code and the opposite rule for docs.

Yes, but I see no problem here: the translation of code rules to docs
is problematic anyway.

> Sometimes a patch series will have several doc additions, that while
> collectively may appear as one doc patch, the submitter chose to break
> them up to keep them with their respective code parts.

I'm asking that all documentation changes for a series appear as one
patch.

> I think it should be ok if someone did that ... we have a lot of rules
> to what is an acceptable patch already.

I didn't suggest to add a new rule, I was just asking several
individuals to humor me.  They can elect to ignore my request, if they
don't want to.

> Can I suggest that we allow any GM to approve doc changes.
> We need all the review bandwidth we can get.

If you think I'm slow in reviewing, let's talk about that.

Re: [PATCH 04/10] Don't stress 'remote' in "Data Caching" in doc

[Doug Evans]

On Tue, Nov 19, 2013 at 7:58 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Tue, 19 Nov 2013 18:16:46 -0800
>> From: Doug Evans <dje@google.com>
>> Cc: Yao Qi <yao@codesourcery.com>, gdb-patches <gdb-patches@sourceware.org>
>>
>> >> > Thanks.  But may I ask in the future not to split the patches to
>> >> > documentation that are related to the same series?  When you split
>> >> > them, it makes the review harder, as I see the documentation changes
>> >> > piecemeal, rather than together.
>> >>
>> >> That may be hard to apply in general.
>> >
>> > I don't see why it would be.  Can you elaborate?
>>
>> We actively ask people to do the opposite for code.
>
> I don't understand why, but I won't argue about that part.
>
>> So we would have one rule for code and the opposite rule for docs.
>
> Yes, but I see no problem here: the translation of code rules to docs
> is problematic anyway.

I think it would depend on the patch.
In general I'd just apply something intuitive based on how I'm
submitting the code changes.

>> Sometimes a patch series will have several doc additions, that while
>> collectively may appear as one doc patch, the submitter chose to break
>> them up to keep them with their respective code parts.
>
> I'm asking that all documentation changes for a series appear as one
> patch.

I can add something to the patch submission guidelines that says that.

>> I think it should be ok if someone did that ... we have a lot of rules
>> to what is an acceptable patch already.
>
> I didn't suggest to add a new rule, I was just asking several
> individuals to humor me.  They can elect to ignore my request, if they
> don't want to.
>
>> Can I suggest that we allow any GM to approve doc changes.
>> We need all the review bandwidth we can get.
>
> If you think I'm slow in reviewing, let's talk about that.

Rather, I didn't want to suggest an increased workload without also
suggesting a way to share it.

Re: [PATCH 04/10] Don't stress 'remote' in "Data Caching" in doc

[Yao Qi]

On 11/04/2013 12:55 AM, Eli Zaretskii wrote:
> Please don't use @ref as if it were an HTML hyperlink.  The results
> look awkward in certain formats (this one will look awkward in Info).
> Instead, please say something like
> 
>    Such caching generally improves performance in remote debugging
>    (@pxref{Remote Debugging}).
> 

Fixed.

>> >-Show maximum number of dcache entries.  See also @ref{Caching Remote Data, info dcache}.
>> >+Show maximum number of dcache entries.  See also @ref{Caching Target Data, info dcache}.
> Likewise.  Here, I would simply use @xref:
> 
>    @xref{Caching Target Data, info dcache}.
> 

Fixed.

>> >  @item show dcache line-size
>> >  @kindex show dcache line-size
>> >-Show default size of dcache lines.  See also @ref{Caching Remote Data, info dcache}.
>> >+Show default size of dcache lines.  See also @ref{Caching Target Data, info dcache}.
> This cross-reference is simply redundant (you just have it in the
> previous sentence), and should be removed.
> 

OK.

> OK with those changes.

Patch below is what I committed.  Thanks for the review.

-- 
Yao (齐尧)

gdb/doc:

2013-11-20  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (Data): Rename menu item.
	(Caching Remote Data): Rename to ...
	(Caching Target Data): ... it.  Update.
---
 gdb/doc/ChangeLog   |    6 ++++++
 gdb/doc/gdb.texinfo |   31 ++++++++++++++++---------------
 2 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 53bb973..9b641e6 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,9 @@
+2013-11-20  Yao Qi  <yao@codesourcery.com>
+
+	* gdb.texinfo (Data): Rename menu item.
+	(Caching Remote Data): Rename to ...
+	(Caching Target Data): ... it.  Update.
+
 2013-11-18  Joel Brobecker  <brobecker@adacore.com>
 
 	* gdb.texinfo (GDB/MI Miscellaneous Commands): Document the new
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 19e9aa5..7ec16dd 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -8066,7 +8066,7 @@ being passed the type of @var{arg} as the argument.
 * Core File Generation::        Cause a program dump its core
 * Character Sets::              Debugging programs that use a different
                                 character set than GDB does
-* Caching Remote Data::         Data caching for remote targets
+* Caching Target Data::         Data caching for targets
 * Searching Memory::            Searching memory for a sequence of bytes
 @end menu
 
@@ -10823,18 +10823,19 @@ $10 = 78 '+'
 The @sc{ibm1047} character set uses the number 78 to encode the @samp{+}
 character.
 
-@node Caching Remote Data
-@section Caching Data of Remote Targets
-@cindex caching data of remote targets
-
-@value{GDBN} caches data exchanged between the debugger and a
-remote target (@pxref{Remote Debugging}).  Such caching generally improves
-performance, because it reduces the overhead of the remote protocol by
-bundling memory reads and writes into large chunks.  Unfortunately, simply
-caching everything would lead to incorrect results, since @value{GDBN} 
-does not necessarily know anything about volatile values, memory-mapped I/O
-addresses, etc.  Furthermore, in non-stop mode (@pxref{Non-Stop Mode})
-memory can be changed @emph{while} a gdb command is executing.
+@node Caching Target Data
+@section Caching Data of Targets
+@cindex caching data of targets
+
+@value{GDBN} caches data exchanged between the debugger and a target.
+Such caching generally improves performance in remote debugging
+(@pxref{Remote Debugging}), because it reduces the overhead of the
+remote protocol by bundling memory reads and writes into large chunks.
+Unfortunately, simply caching everything would lead to incorrect results,
+since @value{GDBN} does not necessarily know anything about volatile
+values, memory-mapped I/O addresses, etc.  Furthermore, in non-stop mode
+(@pxref{Non-Stop Mode}) memory can be changed @emph{while} a gdb command
+is executing.
 Therefore, by default, @value{GDBN} only caches data
 known to be on the stack@footnote{In non-stop mode, it is moderately
 rare for a running thread to modify the stack of a stopped thread
@@ -10888,11 +10889,11 @@ Must be a power of 2.
 
 @item show dcache size
 @kindex show dcache size
-Show maximum number of dcache entries.  See also @ref{Caching Remote Data, info dcache}.
+Show maximum number of dcache entries.  @xref{Caching Target Data, info dcache}.
 
 @item show dcache line-size
 @kindex show dcache line-size
-Show default size of dcache lines.  See also @ref{Caching Remote Data, info dcache}.
+Show default size of dcache lines.
 
 @end table
 
-- 
1.7.7.6

[PATCH 06/10] Add REGISTRY for struct address_space.

[Yao Qi]

This patch adds REGISTRY for struct address_space.

gdb:

2013-11-02  Yao Qi  <yao@codesourcery.com>

	* progspace.c: DEFINE_REGISTRY for address_space.
	(new_address_space): Call address_space_alloc_data.
	(free_address_space): Call address_space_free_data.
	(struct address_space): Move it to ...
	* progspace.h: ... here.
	(struct address_space) <REGISTRY_FIELDS>: New.
	Use DECLARE_REGISTRY.
---
 gdb/progspace.c |   14 ++++++--------
 gdb/progspace.h |   18 +++++++++++++++++-
 2 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/gdb/progspace.c b/gdb/progspace.c
index 6e72211..b9b32dd 100644
--- a/gdb/progspace.c
+++ b/gdb/progspace.c
@@ -44,16 +44,12 @@ static int highest_address_space_num;
 
 DEFINE_REGISTRY (program_space, REGISTRY_ACCESS_FIELD)
 
-\f
+/* Keep a registry of per-address_space data-pointers required by other GDB
+   modules.  */
 
-/* An address space.  Currently this is not used for much other than
-   for comparing if pspaces/inferior/threads see the same address
-   space.  */
+DEFINE_REGISTRY (address_space, REGISTRY_ACCESS_FIELD)
 
-struct address_space
-{
-  int num;
-};
+\f
 
 /* Create a new address space object, and add it to the list.  */
 
@@ -64,6 +60,7 @@ new_address_space (void)
 
   aspace = XZALLOC (struct address_space);
   aspace->num = ++highest_address_space_num;
+  address_space_alloc_data (aspace);
 
   return aspace;
 }
@@ -89,6 +86,7 @@ maybe_new_address_space (void)
 static void
 free_address_space (struct address_space *aspace)
 {
+  address_space_free_data (aspace);
   xfree (aspace);
 }
 
diff --git a/gdb/progspace.h b/gdb/progspace.h
index f24a569..3735202 100644
--- a/gdb/progspace.h
+++ b/gdb/progspace.h
@@ -31,7 +31,6 @@ struct bfd;
 struct objfile;
 struct inferior;
 struct exec;
-struct address_space;
 struct program_space_data;
 
 typedef struct so_list *so_list_ptr;
@@ -131,6 +130,18 @@ DEF_VEC_P (so_list_ptr);
    (traditional unix/uClinux), or, in the DICOS case, the address
    space bound to the program space is mostly ignored.  */
 
+/* An address space.  It is used for comparing if pspaces/inferior/threads
+   see the same address space and for associating caches to each address
+   space.  */
+
+struct address_space
+{
+  int num;
+
+  /* Per aspace data-pointers required by other GDB modules.  */
+  REGISTRY_FIELDS;
+};
+
 /* The program space structure.  */
 
 struct program_space
@@ -304,4 +315,9 @@ extern void clear_program_space_solib_cache (struct program_space *);
 
 DECLARE_REGISTRY (program_space);
 
+/* Keep a registry of per-aspace data-pointers required by other GDB
+   modules.  */
+
+DECLARE_REGISTRY (address_space);
+
 #endif
-- 
1.7.7.6

Re: [PATCH 06/10] Add REGISTRY for struct address_space.

[Doug Evans]

On Sat, Nov 2, 2013 at 10:54 PM, Yao Qi <yao@codesourcery.com> wrote:
> This patch adds REGISTRY for struct address_space.
>
> gdb:
>
> 2013-11-02  Yao Qi  <yao@codesourcery.com>
>
>         * progspace.c: DEFINE_REGISTRY for address_space.
>         (new_address_space): Call address_space_alloc_data.
>         (free_address_space): Call address_space_free_data.
>         (struct address_space): Move it to ...
>         * progspace.h: ... here.
>         (struct address_space) <REGISTRY_FIELDS>: New.
>         Use DECLARE_REGISTRY.

Ok.

[Maybe struct address_space could be kept in progspace.c, dunno.
Perhaps a later patch requires moving it to a header.]

Re: [PATCH 06/10] Add REGISTRY for struct address_space.

[Yao Qi]

On 11/18/2013 04:55 AM, Doug Evans wrote:
> Ok.
> 
> [Maybe struct address_space could be kept in progspace.c, dunno.
> Perhaps a later patch requires moving it to a header.]

Right, 'struct address_space' could be still kept in progspace.c.
Update the patch and push it.

-- 
Yao (齐尧)

gdb:

2013-11-20  Yao Qi  <yao@codesourcery.com>

	* progspace.c (struct address_space): Update comments.
	<REGISTRY_FIELDS>: New fields.
	DEFINE_REGISTRY for address_space.
	(new_address_space): Call address_space_alloc_data.
	(free_address_space): Call address_space_free_data.
	* progspace.h: Use DECLARE_REGISTRY.
---
 gdb/ChangeLog   |    9 +++++++++
 gdb/progspace.c |   18 ++++++++++++++----
 gdb/progspace.h |    5 +++++
 3 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1362295..e8df902 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,14 @@
 2013-11-20  Yao Qi  <yao@codesourcery.com>
 
+	* progspace.c (struct address_space): Update comments.
+	<REGISTRY_FIELDS>: New fields.
+	DEFINE_REGISTRY for address_space.
+	(new_address_space): Call address_space_alloc_data.
+	(free_address_space): Call address_space_free_data.
+	* progspace.h: Use DECLARE_REGISTRY.
+
+2013-11-20  Yao Qi  <yao@codesourcery.com>
+
 	* Makefile.in (SFILES):Add target-dcache.c.
 	(HFILES_NO_SRCDIR): Add target-dcache.h.
 	(COMMON_OBS): Add target-dcache.o.
diff --git a/gdb/progspace.c b/gdb/progspace.c
index 6e72211..303e88e 100644
--- a/gdb/progspace.c
+++ b/gdb/progspace.c
@@ -44,17 +44,25 @@ static int highest_address_space_num;
 
 DEFINE_REGISTRY (program_space, REGISTRY_ACCESS_FIELD)
 
-\f
-
-/* An address space.  Currently this is not used for much other than
-   for comparing if pspaces/inferior/threads see the same address
+/* An address space.  It is used for comparing if pspaces/inferior/threads
+   see the same address space and for associating caches to each address
    space.  */
 
 struct address_space
 {
   int num;
+
+  /* Per aspace data-pointers required by other GDB modules.  */
+  REGISTRY_FIELDS;
 };
 
+/* Keep a registry of per-address_space data-pointers required by other GDB
+   modules.  */
+
+DEFINE_REGISTRY (address_space, REGISTRY_ACCESS_FIELD)
+
+\f
+
 /* Create a new address space object, and add it to the list.  */
 
 struct address_space *
@@ -64,6 +72,7 @@ new_address_space (void)
 
   aspace = XZALLOC (struct address_space);
   aspace->num = ++highest_address_space_num;
+  address_space_alloc_data (aspace);
 
   return aspace;
 }
@@ -89,6 +98,7 @@ maybe_new_address_space (void)
 static void
 free_address_space (struct address_space *aspace)
 {
+  address_space_free_data (aspace);
   xfree (aspace);
 }
 
diff --git a/gdb/progspace.h b/gdb/progspace.h
index f24a569..04ac374 100644
--- a/gdb/progspace.h
+++ b/gdb/progspace.h
@@ -304,4 +304,9 @@ extern void clear_program_space_solib_cache (struct program_space *);
 
 DECLARE_REGISTRY (program_space);
 
+/* Keep a registry of per-aspace data-pointers required by other GDB
+   modules.  */
+
+DECLARE_REGISTRY (address_space);
+
 #endif
-- 
1.7.7.6

Re: [PATCH 00/10 V2] Cache code access for disassemble

[Yao Qi]

On 11/03/2013 01:54 PM, Yao Qi wrote:
> This is the V2 of this patch series.  In the discussion of V1, the
> major comments are:
>
>   1. dcache.c and target_dcache needs some cleanups and factor.
>   2. commands "set dcache size" and "set dcahce line-size" needs a
>      description on semantics when dcache is per-address_space.
>   3. Use a single global 'target_dcache' for both stack caching and
>      code caching.
>   4. Replace "executable code" with "code segment" in doc and NEWS.
>
> V2 addresses these comments as follows:
>
> Patch #1 ~ #4 are about cleanups and factors to dcache.c and
> target_dcache.  Patch #5 is a minor optimization to target_dcache.
> Patch #6 and #7 change target_dcache to per-address_space mode, and
> update doc on dcache-related commands.
>
> We decide to use single 'target_dcache' for caching different types
> of access, so cache invalidation caused by option "stack-cache"
> changes makes troubles.  Think a little, I find we don't have to
> invalidate target_dcache when option "stack-cache" is changed.  This
> is what patch #8 does, and looks change in patch #8 doesn't affect
> the semantics of option "set stack-cache".
>
> Patch #9 adds command "set code-cache", similar to "set stack-cache", and
> patch #10 is to use TARGET_OBJECT_CODE_MEMORY to read for disassembly.
>
> Patch 4, 5, 7, and 9 need doc review.
>
> Each patch is regression tested on x86_64-linux.

Ping.  https://sourceware.org/ml/gdb-patches/2013-11/msg00040.html

-- 
Yao (齐尧)

Re: [PATCH 00/10 V2] Cache code access for disassemble

[Yao Qi]

On 11/11/2013 04:29 PM, Yao Qi wrote:
> On 11/03/2013 01:54 PM, Yao Qi wrote:
>> This is the V2 of this patch series.  In the discussion of V1, the
>> major comments are:
>>
>>    1. dcache.c and target_dcache needs some cleanups and factor.
>>    2. commands "set dcache size" and "set dcahce line-size" needs a
>>       description on semantics when dcache is per-address_space.
>>    3. Use a single global 'target_dcache' for both stack caching and
>>       code caching.
>>    4. Replace "executable code" with "code segment" in doc and NEWS.
>>
>> V2 addresses these comments as follows:
>>
>> Patch #1 ~ #4 are about cleanups and factors to dcache.c and
>> target_dcache.  Patch #5 is a minor optimization to target_dcache.
>> Patch #6 and #7 change target_dcache to per-address_space mode, and
>> update doc on dcache-related commands.
>>
>> We decide to use single 'target_dcache' for caching different types
>> of access, so cache invalidation caused by option "stack-cache"
>> changes makes troubles.  Think a little, I find we don't have to
>> invalidate target_dcache when option "stack-cache" is changed.  This
>> is what patch #8 does, and looks change in patch #8 doesn't affect
>> the semantics of option "set stack-cache".
>>
>> Patch #9 adds command "set code-cache", similar to "set stack-cache", and
>> patch #10 is to use TARGET_OBJECT_CODE_MEMORY to read for disassembly.
>>
>> Patch 4, 5, 7, and 9 need doc review.
>>
>> Each patch is regression tested on x86_64-linux.
>
> Ping.  https://sourceware.org/ml/gdb-patches/2013-11/msg00040.html
>

Ping^2.

-- 
Yao (齐尧)

[PATCH 10/10] Use target_read_code in disassemble.

[Yao Qi]

[Patch 10/10 is not sent out by git send-email.  Send it now.]

This patch teaches "disassembly" use code cache mechanism to read
target code.

gdb:

2013-11-18  Yao Qi  <yao@codesourcery.com>

	* disasm.c (dis_asm_read_memory): Call target_read_code
	instead of target_read_memory.
---
 gdb/disasm.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gdb/disasm.c b/gdb/disasm.c
index e643c2d..522e25f 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -47,7 +47,7 @@ static int
 dis_asm_read_memory (bfd_vma memaddr, gdb_byte *myaddr, unsigned int len,
 		     struct disassemble_info *info)
 {
-  return target_read_memory (memaddr, myaddr, len);
+  return target_read_code (memaddr, myaddr, len);
 }
 
 /* Like memory_error with slightly different parameters.  */
-- 
1.7.7.6

Re: [PATCH 10/10] Use target_read_code in disassemble.

[Doug Evans]

On Mon, Nov 18, 2013 at 12:21 AM, Yao Qi <yao@codesourcery.com> wrote:
> [Patch 10/10 is not sent out by git send-email.  Send it now.]
>
> This patch teaches "disassembly" use code cache mechanism to read
> target code.
>
> gdb:
>
> 2013-11-18  Yao Qi  <yao@codesourcery.com>
>
>         * disasm.c (dis_asm_read_memory): Call target_read_code
>         instead of target_read_memory.
> ---
>  gdb/disasm.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/gdb/disasm.c b/gdb/disasm.c
> index e643c2d..522e25f 100644
> --- a/gdb/disasm.c
> +++ b/gdb/disasm.c
> @@ -47,7 +47,7 @@ static int
>  dis_asm_read_memory (bfd_vma memaddr, gdb_byte *myaddr, unsigned int len,
>                      struct disassemble_info *info)
>  {
> -  return target_read_memory (memaddr, myaddr, len);
> +  return target_read_code (memaddr, myaddr, len);
>  }
>
>  /* Like memory_error with slightly different parameters.  */

Looks good to me.

I realize there's a few nits remaining, but thanks for persevering!

Re: [PATCH 00/10 V2] Cache code access for disassemble

[Yao Qi]

On 11/03/2013 01:54 PM, Yao Qi wrote:
> This is the V2 of this patch series.  In the discussion of V1, the
> major comments are:
>
>   1. dcache.c and target_dcache needs some cleanups and factor.
>   2. commands "set dcache size" and "set dcahce line-size" needs a
>      description on semantics when dcache is per-address_space.
>   3. Use a single global 'target_dcache' for both stack caching and
>      code caching.
>   4. Replace "executable code" with "code segment" in doc and NEWS.
>
> V2 addresses these comments as follows:
>
> Patch #1 ~ #4 are about cleanups and factors to dcache.c and
> target_dcache.  Patch #5 is a minor optimization to target_dcache.
> Patch #6 and #7 change target_dcache to per-address_space mode, and
> update doc on dcache-related commands.
>
> We decide to use single 'target_dcache' for caching different types
> of access, so cache invalidation caused by option "stack-cache"
> changes makes troubles.  Think a little, I find we don't have to
> invalidate target_dcache when option "stack-cache" is changed.  This
> is what patch #8 does, and looks change in patch #8 doesn't affect
> the semantics of option "set stack-cache".

I've pushed patch #1 ~ #4, as they are cleanups.  I'll drop #8 from this 
series, as it is unnecessary pointed out during the review.  I'll post V3.

-- 
Yao (齐尧)