[PATCH 5/5] unwind-libdw: use the mmap_notify subsystem

Masatake YAMATO yamato at redhat.com
Sun Apr 29 21:45:40 UTC 2018


The unwind subsystem uses the mmap_cache subsystem even it uses
unwind-libdw as backend. unwind-libdw doesn't need the full set of the
mmap_cache subsystem; libdw has a feature for caching a memory
mapping.

This commit does three things.

(1) Make the unwind subsystem not use the mmap_cache subsystem.
The unwind subsystem never concern the memory mapping of the target.
It becomes a thin layer.

(2) Make unwind-libunwind use the mmap_cache subsystem directly.

(3) Make unwind-libdw use the mmap_notify subsystem to know when it
should call dwfl_linux_proc_report/dwfl_report_end for updating the
cache.

Here is a subsystem structure that this patch
introduces:

	+-------------------------------------+
	|            unwind subsys            |
	+------------------+------------------+
	| unwind-libunwind |   unwind-libdw   |
	+------------------+------------------+
	|    mmap_cache    |                  |
	+------------------+                  |
	|               mmap_notify           |
	+-------------------------------------+
	|                syscall              |
	+-------------------------------------+
               mmap/munmap/mprotect/brk...

* unwind.c: Don't include mmap_cache.h.
(unwind_init): Don't call mmap_cache_enable.
(unwind_tcb_print, unwind_tcb_capture): Just call unwinder.tcb_walk.
Don't call mmap_cache related functions.

* unwind.h (struct unwind_unwinder_t::tcb_flush_cache): Remove the field.

* unwind-libdw.c: Include mmap_notify.h instead of mmap_cache.h
(struct ctx::last_proc_updating): New field to record the generation
of a memory mapping that is cached by dwfl_linux_proc_report
and dwfl_report_end.
(mapping_generation): A variable counting how many times the memory
mapping of targets is changed.
(updating_mapping_generation): New utility function for updating
mapping_generation.
(init): New function for registering updating_mapping_generation to
the mmap_notify subsystem as a callback function.
(tcb_init): Initialize ctx::last_proc_updating.
(flush_cache_maybe): Rename tcb_flush_cache. Rebuild the cache data
only if the data is stale.
(tcb_walk): Call flush_cache_maybe for avoiding referring staled cache data.
(unwinder.init): Set init function.
(unwinder.tcb_flush_cache): Remove the field.

* unwind-libunwind.c (init): Enable the mmap_cache subsystem.
(tcb_walk): Call mmap_cache_rebuild_if_invalid and unw_flush_cache for
updating the cache of the memory mapping before walking the stack.
(walk): new function renamed from tcb_walk.
(unwinder.tcb_flush_cache): Remove the field.

Signed-off-by: Masatake YAMATO <yamato at redhat.com>
---
 unwind-libdw.c     | 69 ++++++++++++++++++++++++++++++++++++------------------
 unwind-libunwind.c | 33 ++++++++++++++++++++------
 unwind.c           | 34 +++++----------------------
 unwind.h           |  6 -----
 4 files changed, 78 insertions(+), 64 deletions(-)

diff --git a/unwind-libdw.c b/unwind-libdw.c
index b20ba204..76f9b5fb 100644
--- a/unwind-libdw.c
+++ b/unwind-libdw.c
@@ -36,13 +36,28 @@
 
 #include "defs.h"
 #include "unwind.h"
-#include "mmap_cache.h"
+#include "mmap_notify.h"
 #include <elfutils/libdwfl.h>
 
 struct ctx {
 	Dwfl *dwfl;
+	unsigned int last_proc_updating;
 };
 
+static unsigned int mapping_generation;
+
+static void
+update_mapping_generation(struct tcb *tcp, void *nouse)
+{
+	mapping_generation++;
+}
+
+static void
+init(void)
+{
+	mmap_notify_register_client(update_mapping_generation, NULL);
+}
+
 static void *
 tcb_init(struct tcb *tcp)
 {
@@ -74,6 +89,7 @@ tcb_init(struct tcb *tcp)
 
 	struct ctx *ctx = xmalloc(sizeof(*ctx));
 	ctx->dwfl = dwfl;
+	ctx->last_proc_updating = 0;
 	return ctx;
 }
 
@@ -87,6 +103,31 @@ tcb_fin(struct tcb *tcp)
 	}
 }
 
+static void
+flush_cache_maybe(struct tcb *tcp)
+{
+	struct ctx *ctx = tcp->unwind_ctx;
+	if (!ctx)
+		return;
+
+	if (ctx->last_proc_updating == mapping_generation)
+		return;
+
+	int r = dwfl_linux_proc_report(ctx->dwfl, tcp->pid);
+
+	if (r < 0)
+		error_msg("dwfl_linux_proc_report returned an error"
+			  " for pid %d: %s", tcp->pid, dwfl_errmsg(-1));
+	else if (r > 0)
+		error_msg("dwfl_linux_proc_report returned an error"
+			  " for pid %d", tcp->pid);
+	else if (dwfl_report_end(ctx->dwfl, NULL, NULL) != 0)
+		error_msg("dwfl_report_end returned an error"
+			  " for pid %d: %s", tcp->pid, dwfl_errmsg(-1));
+
+	ctx->last_proc_updating = mapping_generation;
+}
+
 struct frame_user_data {
 	unwind_call_action_fn call_action;
 	unwind_error_action_fn error_action;
@@ -150,6 +191,9 @@ tcb_walk(struct tcb *tcp,
 		.data = data,
 		.stack_depth = 256,
 	};
+
+	flush_cache_maybe(tcp);
+
 	int r = dwfl_getthread_frames(ctx->dwfl, tcp->pid, frame_callback,
 				      &user_data);
 	if (r)
@@ -158,31 +202,10 @@ tcb_walk(struct tcb *tcp,
 			     0);
 }
 
-static void
-tcb_flush_cache(struct tcb *tcp)
-{
-	struct ctx *ctx = tcp->unwind_ctx;
-	if (!ctx)
-		return;
-
-	int r = dwfl_linux_proc_report(ctx->dwfl, tcp->pid);
-
-	if (r < 0)
-		error_msg("dwfl_linux_proc_report returned an error"
-			  " for pid %d: %s", tcp->pid, dwfl_errmsg(-1));
-	else if (r > 0)
-		error_msg("dwfl_linux_proc_report returned an error"
-			  " for pid %d", tcp->pid);
-	else if (dwfl_report_end(ctx->dwfl, NULL, NULL) != 0)
-		error_msg("dwfl_report_end returned an error"
-			  " for pid %d: %s", tcp->pid, dwfl_errmsg(-1));
-}
-
 const struct unwind_unwinder_t unwinder = {
 	.name = "libdw",
-	.init = NULL,
+	.init = init,
 	.tcb_init = tcb_init,
 	.tcb_fin = tcb_fin,
 	.tcb_walk = tcb_walk,
-	.tcb_flush_cache = tcb_flush_cache,
 };
diff --git a/unwind-libunwind.c b/unwind-libunwind.c
index 38b3e2c5..c64e92cb 100644
--- a/unwind-libunwind.c
+++ b/unwind-libunwind.c
@@ -36,6 +36,8 @@ static unw_addr_space_t libunwind_as;
 static void
 init(void)
 {
+	mmap_cache_enable();
+
 	libunwind_as = unw_create_addr_space(&_UPT_accessors, 0);
 	if (!libunwind_as)
 		error_msg_and_die("failed to create address space"
@@ -125,10 +127,10 @@ print_stack_frame(struct tcb *tcp,
 }
 
 static void
-tcb_walk(struct tcb *tcp,
-	 unwind_call_action_fn call_action,
-	 unwind_error_action_fn error_action,
-	 void *data)
+walk(struct tcb *tcp,
+     unwind_call_action_fn call_action,
+     unwind_error_action_fn error_action,
+     void *data)
 {
 	char *symbol_name;
 	size_t symbol_name_size = 40;
@@ -159,9 +161,27 @@ tcb_walk(struct tcb *tcp,
 }
 
 static void
-tcb_flush_cache(struct tcb *tcp)
+tcb_walk(struct tcb *tcp,
+	 unwind_call_action_fn call_action,
+	 unwind_error_action_fn error_action,
+	 void *data)
 {
-	unw_flush_cache(libunwind_as, 0, 0);
+	switch (mmap_cache_rebuild_if_invalid(tcp, __func__)) {
+		case MMAP_CACHE_REBUILD_RENEWED:
+			/*
+			 * Rebuild the unwinder internal cache.
+			 * Called when mmap cache subsystem detects a
+			 * change of tracee memory mapping.
+			 */
+			unw_flush_cache(libunwind_as, 0, 0);
+			ATTRIBUTE_FALLTHROUGH;
+		case MMAP_CACHE_REBUILD_READY:
+			walk(tcp, call_action, error_action, data);
+			break;
+		default:
+			/* Do nothing */
+			;
+	}
 }
 
 const struct unwind_unwinder_t unwinder = {
@@ -170,5 +190,4 @@ const struct unwind_unwinder_t unwinder = {
 	.tcb_init = tcb_init,
 	.tcb_fin = tcb_fin,
 	.tcb_walk = tcb_walk,
-	.tcb_flush_cache = tcb_flush_cache,
 };
diff --git a/unwind.c b/unwind.c
index 586f7450..e4c6623c 100644
--- a/unwind.c
+++ b/unwind.c
@@ -26,7 +26,6 @@
  */
 
 #include "defs.h"
-#include "mmap_cache.h"
 #include "unwind.h"
 
 #ifdef USE_DEMANGLE
@@ -59,7 +58,6 @@ unwind_init(void)
 {
 	if (unwinder.init)
 		unwinder.init();
-	mmap_cache_enable();
 }
 
 void
@@ -296,19 +294,8 @@ unwind_tcb_print(struct tcb *tcp)
 		debug_func_msg("head: tcp=%p, queue=%p",
 			       tcp, tcp->unwind_queue->head);
 		queue_print(tcp->unwind_queue);
-	} else switch (mmap_cache_rebuild_if_invalid(tcp, __func__)) {
-		case MMAP_CACHE_REBUILD_RENEWED:
-			unwinder.tcb_flush_cache(tcp);
-			ATTRIBUTE_FALLTHROUGH;
-		case MMAP_CACHE_REBUILD_READY:
-			debug_func_msg("walk: tcp=%p, queue=%p",
-				       tcp, tcp->unwind_queue->head);
-			unwinder.tcb_walk(tcp, print_call_cb, print_error_cb, NULL);
-			break;
-		default:
-			/* Do nothing */
-			;
-	}
+	} else
+		unwinder.tcb_walk(tcp, print_call_cb, print_error_cb, NULL);
 }
 
 /*
@@ -325,19 +312,10 @@ unwind_tcb_capture(struct tcb *tcp)
 #endif
 	if (tcp->unwind_queue->head)
 		error_msg_and_die("bug: unprinted entries in queue");
-
-	switch (mmap_cache_rebuild_if_invalid(tcp, __func__)) {
-	case MMAP_CACHE_REBUILD_RENEWED:
-		unwinder.tcb_flush_cache(tcp);
-		ATTRIBUTE_FALLTHROUGH;
-	case MMAP_CACHE_REBUILD_READY:
-		unwinder.tcb_walk(tcp, queue_put_call, queue_put_error,
-				   tcp->unwind_queue);
-		debug_func_msg("tcp=%p, queue=%p",
+	else {
+		debug_func_msg("walk: tcp=%p, queue=%p",
 			       tcp, tcp->unwind_queue->head);
-		break;
-	default:
-		/* Do nothing */
-		;
+		unwinder.tcb_walk(tcp, queue_put_call, queue_put_error,
+				  tcp->unwind_queue);
 	}
 }
diff --git a/unwind.h b/unwind.h
index 3a6065b4..678c561f 100644
--- a/unwind.h
+++ b/unwind.h
@@ -63,12 +63,6 @@ struct unwind_unwinder_t {
 			   unwind_call_action_fn,
 			   unwind_error_action_fn,
 			   void *);
-
-	/*
-	 * Rebuild the unwinder internal cache.  Called when mmap cache
-	 * subsystem detects a change of tracee memory mapping.
-	 */
-	void   (*tcb_flush_cache)(struct tcb *);
 };
 
 extern const struct unwind_unwinder_t unwinder;
-- 
2.14.3



More information about the Strace-devel mailing list