Re: [PATCH 2/3] skip_prolgoue (amd64)

[Yao Qi <yao at codesourcery dot com>]

On 12/04/2013 08:07 PM, Pedro Alves wrote:
>> >+  if (non_stop)
>> >+    {
>> >+      /* In non-stop mode, one thread stops and caches the contents of
>> >+	 stack or code, while other running threads may change the
>> >+	 code (through JIT) or stack.  The target cache can get stale
>> >+	 without us being able to detect it.  Flush target cache
>> >+	 before handling each event.  */
>> >+      target_dcache_invalidate ();
>> >+    }
> I don't actually think this should be gated on non-stop.  It
> should be unconditional.  I mentioned before that it'd be most
> visible with non-stop, but that doesn't imply it's not
> visible with all-stop.  If we're seeing or going to wait for
> a target event, it's because the target was running,
> irrespective of all-stop/non-stop.  I really think we
> should invalidate the cache at all places we invalidate the
> overlay cache (wait_for_inferior, etc.), not just fetch_inferior_event.

After some discussions, it becomes clear to me that we should flush
target cache before handling events, in the place of the callers of
handle_inferior_event.  I am wondering why don't we flush cache inside
handle_inferior_event?  Although flushing cache is not much relevant
to handle_inferior_event, this can avoid doing cache flush in every
caller of handle_inferior_event.

> 
> For all-stop, it shouldn't really make a difference to
> performance, as we invalidate the cache on resumes anyway,
> and in all-stop, there must always be a resume prior to
> any stop...

If that is the right way to go, I'll move 'overlay_cache_invalid = 1'
to handle_inferior_event too.  WDYT?

gdb:

2013-12-08  Yao Qi  <yao@codesourcery.com>

	* infrun.c: Include "target-dcache.h".
	(fetch_inferior_event): Flush target cache.
---
 gdb/infrun.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 3b55583..0a12107 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -60,6 +60,7 @@
 #include "objfiles.h"
 #include "completer.h"
 #include "target-descriptions.h"
+#include "target-dcache.h"
 
 /* Prototypes for local functions */
 
@@ -3168,6 +3169,11 @@ handle_inferior_event (struct execution_control_state *ecs)
 {
   enum stop_kind stop_soon;
 
+  /* If we've got an event from target, it means the target was
+     running, so cache would be staled.  Flush target cache before
+     handling each event.  */
+  target_dcache_invalidate ();
+
   if (ecs->ws.kind == TARGET_WAITKIND_IGNORE)
     {
       /* We had an event in the inferior, but we are not interested in
-- 
1.7.7.6

-- 
Yao (éå)