Re: [PATCH 2/5] Associate target_dcache to address_space.

Yao Qi
2013-10-24T08:33:03Z

On 10/24/2013 12:37 AM, Tom Tromey wrote:
> I like this patch quite a bit. For one thing, it fixes the target
> dcache for the coming multi-target work:-)
>
> Yao> +/* The current added space. */
> Yao> +#define current_address_space current_program_space->aspace
>
> On the whole I would prefer we not add new object-style macros like
> this. (Actually I'd really like it if we got rid of the existing ones
> too...) Writing out the expansion in the few places it is used seems
> better to me.
>

This macro is removed in the updated patch.

> Yao> +static void
> Yao> +target_dcache_cleanup (struct address_space *aspace, void *arg)
> Yao> +{
> Yao> + struct target_dcache *dcache
> Yao> + = address_space_data (aspace, target_dcache_aspace_key);
> Yao> +
> Yao> + target_dcache_xfree (dcache);
>
> Here you don't need to call address_space_data, since ARG is the dcache
> that was passed in.

You are right. Fixed. b.t.w, this sort of problems exist somewhere
else in the tree, I'll post a patch to fix them separately.

--
Yao (é½å°§)

gdb:

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

* progspace.h (struct address_space): Declare.
* target.c (target_dcache_aspace_key): New.
(target_dcache): Delete.
(struct target_dcache): New.
(target_dcache_alloc): New function.
(target_dcache_xfree): New function.
(target_dcache_get): New function.
(target_dcache_cleanup)): New function.
(target_dcache_invalidate): Update.
(memory_xfer_partial_1): Update.
(initialize_targets): Initialize target_dcache_aspace_key.
---
gdb/progspace.h | 1 +
gdb/target.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++-------
2 files changed, 77 insertions(+), 11 deletions(-)

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.c b/gdb/target.c
index 22d7fb6..073a602 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -206,6 +206,70 @@ show_targetdebug (struct ui_file *file, int from_tty,

static void setup_target_debug (void);

+/* 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;
+
+/* dcache for target memory. */
+
+struct target_dcache
+{
+ /* Cache the memory on stack and areas specified by memory
+ attributes. */
+ DCACHE *general;
+};
+
+/* Allocate an instance of struct target_dcache. */
+
+static struct target_dcache *
+target_dcache_alloc (void)
+{
+ struct target_dcache *dcache = xmalloc (sizeof (*dcache));
+
+ dcache->general = dcache_init ();
+
+ return dcache;
+}
+
+/* Free DCACHE and its fields. */
+
+static void
+target_dcache_xfree (struct target_dcache *dcache)
+{
+ if (dcache != NULL)
+ {
+ dcache_free (dcache->general);
+ xfree (dcache);
+ }
+}
+
+/* Get an instance of struct target_dcache. */
+
+static struct target_dcache *
+target_dcache_get (void)
+{
+ struct target_dcache *dcache
+ = address_space_data (current_program_space->aspace,
+ target_dcache_aspace_key);
+
+ if (dcache == NULL)
+ {
+ dcache = target_dcache_alloc ();
+
+ set_address_space_data (current_program_space->aspace,
+ target_dcache_aspace_key, dcache);
+ }
+
+ return dcache;
+}
+
+static void
+target_dcache_cleanup (struct address_space *aspace, void *arg)
+{
+ target_dcache_xfree (arg);
+}
+
/* The option sets this. */
static int stack_cache_enabled_p_1 = 1;
/* And set_stack_cache_enabled_p updates this.
@@ -235,15 +299,14 @@ show_stack_cache_enabled_p (struct ui_file *file, int from_tty,
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;
-
/* Invalidate the target dcache. */

void
target_dcache_invalidate (void)
{
- dcache_invalidate (target_dcache);
+ struct target_dcache *td = target_dcache_get ();
+
+ dcache_invalidate (td->general);
}

/* The user just typed 'target' without the name of a target. */
@@ -1589,14 +1652,14 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
|| (stack_cache_enabled_p && object == TARGET_OBJECT_STACK_MEMORY)))
{
if (readbuf != NULL)
- res = dcache_xfer_memory (ops, target_dcache, memaddr, readbuf,
- reg_len, 0);
+ res = dcache_xfer_memory (ops, target_dcache_get ()->general,
+ 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, target_dcache_get ()->general,
+ memaddr, (void *) writebuf,
reg_len, 1);
if (res <= 0)
return -1;
@@ -1641,7 +1704,8 @@ 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_update (target_dcache_get ()->general, memaddr,
+ (void *) writebuf, res);
}

/* If we still haven't got anything, return the last error. We
@@ -5187,6 +5251,7 @@ Otherwise, any attempt to interrupt or stop will be ignored."),
set_target_permissions, NULL,
&setlist, &showlist);

-
- target_dcache = dcache_init ();
+ target_dcache_aspace_key
+ = register_address_space_data_with_cleanup (NULL,
+ target_dcache_cleanup);
}
--
1.7.7.6