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

Yao Qi
2013-12-08T08:01:25Z

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 (é½å°§)