From d2f4d7286629da6e9f1b844beefb141a4d3ef2c3 Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Wed, 10 Dec 2008 20:39:45 +0100 Subject: PR6866: First pass at translating addresses to symbol names through vma. --- runtime/ChangeLog | 11 +++++++++++ runtime/runtime.h | 18 ++++++++++++++++++ runtime/sym.c | 32 ++++++++++++++++++++++++++------ runtime/task_finder.c | 22 ++++++++++++++++------ runtime/task_finder_vma.c | 38 ++++++++++++++++++++++++++------------ 5 files changed, 97 insertions(+), 24 deletions(-) (limited to 'runtime') diff --git a/runtime/ChangeLog b/runtime/ChangeLog index 3f88bae2..d83ac12c 100644 --- a/runtime/ChangeLog +++ b/runtime/ChangeLog @@ -1,3 +1,14 @@ +2008-12-10 Mark Wielaard + + * runtime.h: Define __stp_tf_vma_entry here. + * sym.c (_stp_mod_sec_lookup): Try vma matching first if task given. + (_stp_kallsyms_lookup): Take an optional task. + (_stp_symbol_snprint): Likewise. + task_finder.c (__stp_tf_vm_cb): Use vm_path to match and attach + module to vma entry. + task_finder_vma.c (stap_add_vma_map_info): Take an optional module. + (__stp_tf_get_vma_entry_addr): New lookup function. + 2008-12-09 Frank Ch. Eigler * time.c (_stp_gettimeofday_ns): Protect some more against freq=0. diff --git a/runtime/runtime.h b/runtime/runtime.h index 3b3e117d..055d3f27 100644 --- a/runtime/runtime.h +++ b/runtime/runtime.h @@ -86,6 +86,24 @@ static struct #include "io.c" #include "arith.c" #include "copy.c" + +/* Lifted task_finder, internal details used in sym.c - XXX */ +struct __stp_tf_vma_entry { + struct hlist_node hlist; + + pid_t pid; + unsigned long addr; + unsigned long vm_start; + unsigned long vm_end; + unsigned long vm_pgoff; + // Is that enough? Should we store a dcookie for vm_file? + + // Module that this vma entry is mapped from, if any. + struct _stp_module *module; +}; +static struct __stp_tf_vma_entry * +__stp_tf_get_vma_entry_addr(struct task_struct *, unsigned long); + #include "sym.c" #ifdef STP_PERFMON #include "perf.c" diff --git a/runtime/sym.c b/runtime/sym.c index 06ac14a5..9b295f58 100644 --- a/runtime/sym.c +++ b/runtime/sym.c @@ -77,11 +77,28 @@ unsigned long _stp_module_relocate(const char *module, const char *section, unsi if found, return NULL otherwise. XXX: needs to be address-space-specific. */ static struct _stp_module *_stp_mod_sec_lookup(unsigned long addr, + struct task_struct *task, struct _stp_section **sec) { struct _stp_module *m = NULL; unsigned midx = 0; unsigned long closest_section_offset = ~0; + + // Try vma matching first if task given. + struct __stp_tf_vma_entry *entry; + if (task) + { + entry = __stp_tf_get_vma_entry_addr(task, addr); + if (entry != NULL && entry->module != NULL) + { + m = entry->module; + *sec = & m->sections[0]; // XXX check actual section and relocate + if (strcmp(".dynamic", m->sections[0].name) == 0) + m->sections[0].addr = entry->vm_start; // cheat... + return m; + } + } + for (midx = 0; midx < _stp_num_modules; midx++) { unsigned secidx; @@ -109,7 +126,8 @@ static const char *_stp_kallsyms_lookup(unsigned long addr, unsigned long *symbo unsigned long *offset, const char **modname, /* char ** secname? */ - char *namebuf) + char *namebuf, + struct task_struct *task) { struct _stp_module *m = NULL; struct _stp_section *sec = NULL; @@ -117,7 +135,7 @@ static const char *_stp_kallsyms_lookup(unsigned long addr, unsigned long *symbo unsigned long flags; unsigned end, begin = 0; - m = _stp_mod_sec_lookup(addr, &sec); + m = _stp_mod_sec_lookup(addr, task, &sec); if (unlikely (m == NULL || sec == NULL)) return NULL; @@ -242,7 +260,7 @@ void _stp_symbol_print(unsigned long address) const char *name; unsigned long offset, size; - name = _stp_kallsyms_lookup(address, &size, &offset, &modname, NULL); + name = _stp_kallsyms_lookup(address, &size, &offset, &modname, NULL, NULL); _stp_printf("%p", (int64_t) address); @@ -267,7 +285,7 @@ int _stp_func_print(unsigned long address, int verbose, int exact) else exstr = " (inexact)"; - name = _stp_kallsyms_lookup(address, &size, &offset, &modname, NULL); + name = _stp_kallsyms_lookup(address, &size, &offset, &modname, NULL, NULL); if (name) { if (verbose) { @@ -283,13 +301,15 @@ int _stp_func_print(unsigned long address, int verbose, int exact) return 0; } -void _stp_symbol_snprint(char *str, size_t len, unsigned long address) +void _stp_symbol_snprint(char *str, size_t len, unsigned long address, + struct task_struct *task) { const char *modname; const char *name; unsigned long offset, size; - name = _stp_kallsyms_lookup(address, &size, &offset, &modname, NULL); + name = _stp_kallsyms_lookup(address, &size, &offset, &modname, NULL, + task); if (name) strlcpy(str, name, len); else diff --git a/runtime/task_finder.c b/runtime/task_finder.c index 25fad1a4..1e0a8474 100644 --- a/runtime/task_finder.c +++ b/runtime/task_finder.c @@ -55,7 +55,6 @@ typedef int (*stap_task_finder_vm_callback)(struct stap_task_finder_target *tgt, unsigned long vm_end, unsigned long vm_pgoff); -#ifdef DEBUG_TASK_FINDER_VMA int __stp_tf_vm_cb(struct stap_task_finder_target *tgt, struct task_struct *tsk, int map_p, char *vm_path, @@ -63,21 +62,32 @@ int __stp_tf_vm_cb(struct stap_task_finder_target *tgt, unsigned long vm_end, unsigned long vm_pgoff) { + int i; +#ifdef DEBUG_TASK_FINDER_VMA _stp_dbug(__FUNCTION__, __LINE__, "vm_cb: tsk %d:%d path %s, start 0x%08lx, end 0x%08lx, offset 0x%lx\n", tsk->pid, map_p, vm_path, vm_start, vm_end, vm_pgoff); +#endif if (map_p) { - // FIXME: What should we do with vm_path? We can't save - // the vm_path pointer itself, but we don't have any - // storage space allocated to save it in... - stap_add_vma_map_info(tsk, vm_start, vm_end, vm_pgoff); + struct _stp_module *module = NULL; + if (vm_path != NULL) + for (i = 0; i < _stp_num_modules; i++) + if (strcmp(vm_path, _stp_modules[i]->name) == 0) + { +#ifdef DEBUG_TASK_FINDER_VMA + _stp_dbug(__FUNCTION__, __LINE__, + "vm_cb: matched path %s to module\n", vm_path); +#endif + module = _stp_modules[i]; + break; + } + stap_add_vma_map_info(tsk, vm_start, vm_end, vm_pgoff, module); } else { stap_remove_vma_map_info(tsk, vm_start, vm_end, vm_pgoff); } return 0; } -#endif struct stap_task_finder_target { /* private: */ diff --git a/runtime/task_finder_vma.c b/runtime/task_finder_vma.c index 4dce4be8..b65d9ea4 100644 --- a/runtime/task_finder_vma.c +++ b/runtime/task_finder_vma.c @@ -16,17 +16,6 @@ static DEFINE_MUTEX(__stp_tf_vma_mutex); #define TASK_FINDER_VMA_ENTRY_ITEMS 100 #endif -struct __stp_tf_vma_entry { - struct hlist_node hlist; - - pid_t pid; - unsigned long addr; - unsigned long vm_start; - unsigned long vm_end; - unsigned long vm_pgoff; - // Is that enough? Should we store a dcookie for vm_file? -}; - static struct __stp_tf_vma_entry __stp_tf_vma_free_list_items[TASK_FINDER_VMA_ENTRY_ITEMS]; @@ -211,7 +200,8 @@ __stp_tf_get_vma_map_entry_internal(struct task_struct *tsk, // Add the vma info to the vma map hash table. static int stap_add_vma_map_info(struct task_struct *tsk, unsigned long vm_start, - unsigned long vm_end, unsigned long vm_pgoff) + unsigned long vm_end, unsigned long vm_pgoff, + struct _stp_module *module) { struct hlist_head *head; struct hlist_node *node; @@ -242,6 +232,7 @@ stap_add_vma_map_info(struct task_struct *tsk, unsigned long vm_start, entry->vm_start = vm_start; entry->vm_end = vm_end; entry->vm_pgoff = vm_pgoff; + entry->module = module; head = &__stp_tf_vma_map[__stp_tf_vma_map_hash(tsk)]; hlist_add_head(&entry->hlist, head); @@ -305,3 +296,26 @@ stap_find_vma_map_info(struct task_struct *tsk, unsigned long vm_addr, mutex_unlock(&__stp_tf_vma_mutex); return rc; } + +// Get vma_entry of the address (vm_start/vm_end) if the vma is +// present in the vma hash table containing. +// Returns NULL if not present. +static struct __stp_tf_vma_entry * +__stp_tf_get_vma_entry_addr(struct task_struct *tsk, unsigned long addr) +{ + struct hlist_head *head; + struct hlist_node *node; + struct __stp_tf_vma_entry *entry; + + mutex_lock(&__stp_tf_vma_mutex); + head = &__stp_tf_vma_map[__stp_tf_vma_map_hash(tsk)]; + hlist_for_each_entry(entry, node, head, hlist) { + if (tsk->pid == entry->pid + && addr >= entry->vm_start && addr < entry->vm_end) { + mutex_unlock(&__stp_tf_vma_mutex); + return entry; + } + } + mutex_unlock(&__stp_tf_vma_mutex); + return NULL; +} -- cgit From 204038b16193de78eeb333fde0cce6081d9d1fcd Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Mon, 15 Dec 2008 17:51:49 +0100 Subject: Always include task_finder.c and enable emit_vm_callback_probe_decl. --- runtime/ChangeLog | 7 +++++++ runtime/runtime.h | 17 +---------------- runtime/sym.c | 4 +++- runtime/task_finder_vma.c | 14 ++++++++++++++ 4 files changed, 25 insertions(+), 17 deletions(-) (limited to 'runtime') diff --git a/runtime/ChangeLog b/runtime/ChangeLog index d83ac12c..b9a2a058 100644 --- a/runtime/ChangeLog +++ b/runtime/ChangeLog @@ -1,3 +1,10 @@ +2008-12-15 Mark Wielaard + + * runtime.h: Just include task_finder.c instead of defining parts + of it here. + * sym.c (_stp_mod_sec_lookup): Add dbug_sym statements. + * task_finder_vma.c: Define __stp_tf_vma_entry here. + 2008-12-10 Mark Wielaard * runtime.h: Define __stp_tf_vma_entry here. diff --git a/runtime/runtime.h b/runtime/runtime.h index 055d3f27..3ca43dc4 100644 --- a/runtime/runtime.h +++ b/runtime/runtime.h @@ -87,22 +87,7 @@ static struct #include "arith.c" #include "copy.c" -/* Lifted task_finder, internal details used in sym.c - XXX */ -struct __stp_tf_vma_entry { - struct hlist_node hlist; - - pid_t pid; - unsigned long addr; - unsigned long vm_start; - unsigned long vm_end; - unsigned long vm_pgoff; - // Is that enough? Should we store a dcookie for vm_file? - - // Module that this vma entry is mapped from, if any. - struct _stp_module *module; -}; -static struct __stp_tf_vma_entry * -__stp_tf_get_vma_entry_addr(struct task_struct *, unsigned long); +#include "task_finder.c" #include "sym.c" #ifdef STP_PERFMON diff --git a/runtime/sym.c b/runtime/sym.c index 9b295f58..82eef17d 100644 --- a/runtime/sym.c +++ b/runtime/sym.c @@ -92,7 +92,9 @@ static struct _stp_module *_stp_mod_sec_lookup(unsigned long addr, if (entry != NULL && entry->module != NULL) { m = entry->module; - *sec = & m->sections[0]; // XXX check actual section and relocate + *sec = &m->sections[0]; // XXX check actual section and relocate + dbug_sym(1, "found section %s in module %s at 0x%lx\n", + m->sections[0].name, m->name, entry->vm_start); if (strcmp(".dynamic", m->sections[0].name) == 0) m->sections[0].addr = entry->vm_start; // cheat... return m; diff --git a/runtime/task_finder_vma.c b/runtime/task_finder_vma.c index b65d9ea4..87a32fe5 100644 --- a/runtime/task_finder_vma.c +++ b/runtime/task_finder_vma.c @@ -16,6 +16,20 @@ static DEFINE_MUTEX(__stp_tf_vma_mutex); #define TASK_FINDER_VMA_ENTRY_ITEMS 100 #endif +struct __stp_tf_vma_entry { + struct hlist_node hlist; + + pid_t pid; + unsigned long addr; + unsigned long vm_start; + unsigned long vm_end; + unsigned long vm_pgoff; + // Is that enough? Should we store a dcookie for vm_file? + + // Module that this vma entry is mapped from, if any. + struct _stp_module *module; +}; + static struct __stp_tf_vma_entry __stp_tf_vma_free_list_items[TASK_FINDER_VMA_ENTRY_ITEMS]; -- cgit From cb1468beb7b34ad988280fea9fb7c4558b47341a Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Wed, 18 Mar 2009 12:27:13 +0100 Subject: Replace mutex in task_finder_vma with rwlock to be interrupt context safe. * runtime/task_finder_vma.c: Replace all mutex calls with appropriate read or write (un)lock calls. --- runtime/task_finder_vma.c | 82 +++++++++++++++++++++++++++++------------------ 1 file changed, 51 insertions(+), 31 deletions(-) (limited to 'runtime') diff --git a/runtime/task_finder_vma.c b/runtime/task_finder_vma.c index 83b206e5..a210d92e 100644 --- a/runtime/task_finder_vma.c +++ b/runtime/task_finder_vma.c @@ -1,13 +1,19 @@ #include #include -#include +#include // When handling memcpy() syscall tracing to notice memory map // changes, we need to cache memcpy() entry parameter values for // processing at memcpy() exit. -// __stp_tf_vma_mutex protects the hash table. -static DEFINE_MUTEX(__stp_tf_vma_mutex); +// __stp_tf_vma_lock protects the hash table. +// Documentation/spinlocks.txt suggest we can be a bit more clever +// if we guarantee that in interrupt context we only read, not write +// the datastructures. We should never change the hash table or the +// contents in interrupt context (which should only ever call +// __stp_tf_get_vma_entry_addr for symbol lookup). So we might want +// to look into that if this seems a bottleneck. +static DEFINE_RWLOCK(__stp_tf_vma_lock); #define __STP_TF_HASH_BITS 4 #define __STP_TF_TABLE_SIZE (1 << __STP_TF_HASH_BITS) @@ -40,23 +46,24 @@ static struct hlist_head __stp_tf_vma_table[__STP_TF_TABLE_SIZE]; static struct hlist_head __stp_tf_vma_map[__STP_TF_TABLE_SIZE]; // __stp_tf_vma_initialize(): Initialize the free list. Grabs the -// mutex. +// spinlock. static void __stp_tf_vma_initialize(void) { int i; struct hlist_head *head = &__stp_tf_vma_free_list[0]; - mutex_lock(&__stp_tf_vma_mutex); + unsigned long flags; + write_lock_irqsave(&__stp_tf_vma_lock, flags); for (i = 0; i < TASK_FINDER_VMA_ENTRY_ITEMS; i++) { hlist_add_head(&__stp_tf_vma_free_list_items[i].hlist, head); } - mutex_unlock(&__stp_tf_vma_mutex); + write_unlock_irqrestore(&__stp_tf_vma_lock, flags); } // __stp_tf_vma_get_free_entry(): Returns an entry from the free list -// or NULL. The __stp_tf_vma_mutex must be locked before calling this +// or NULL. The __stp_tf_vma_lock must be write locked before calling this // function. static struct __stp_tf_vma_entry * __stp_tf_vma_get_free_entry(void) @@ -77,7 +84,7 @@ __stp_tf_vma_get_free_entry(void) // __stp_tf_vma_put_free_entry(): Puts an entry back on the free -// list. The __stp_tf_vma_mutex must be locked before calling this +// list. The __stp_tf_vma_lock must be write locked before calling this // function. static void __stp_tf_vma_put_free_entry(struct __stp_tf_vma_entry *entry) @@ -101,7 +108,7 @@ __stp_tf_vma_hash(struct task_struct *tsk, unsigned long addr) // Get vma_entry if the vma is present in the vma hash table. -// Returns NULL if not present. +// Returns NULL if not present. Takes a read lock on __stp_tf_vma_lock. static struct __stp_tf_vma_entry * __stp_tf_get_vma_entry(struct task_struct *tsk, unsigned long addr) { @@ -109,20 +116,22 @@ __stp_tf_get_vma_entry(struct task_struct *tsk, unsigned long addr) struct hlist_node *node; struct __stp_tf_vma_entry *entry; - mutex_lock(&__stp_tf_vma_mutex); + unsigned long flags; + read_lock_irqsave(&__stp_tf_vma_lock, flags); head = &__stp_tf_vma_table[__stp_tf_vma_hash(tsk, addr)]; hlist_for_each_entry(entry, node, head, hlist) { if (tsk->pid == entry->pid && addr == entry->addr) { - mutex_unlock(&__stp_tf_vma_mutex); + read_unlock_irqrestore(&__stp_tf_vma_lock, flags); return entry; } } - mutex_unlock(&__stp_tf_vma_mutex); + read_unlock_irqrestore(&__stp_tf_vma_lock, flags); return NULL; } // Add the vma info to the vma hash table. +// Takes a write lock on __stp_tf_vma_lock. static int __stp_tf_add_vma(struct task_struct *tsk, unsigned long addr, struct vm_area_struct *vma) @@ -131,7 +140,8 @@ __stp_tf_add_vma(struct task_struct *tsk, unsigned long addr, struct hlist_node *node; struct __stp_tf_vma_entry *entry; - mutex_lock(&__stp_tf_vma_mutex); + unsigned long flags; + write_lock_irqsave(&__stp_tf_vma_lock, flags); head = &__stp_tf_vma_table[__stp_tf_vma_hash(tsk, addr)]; hlist_for_each_entry(entry, node, head, hlist) { if (tsk->pid == entry->pid @@ -141,7 +151,7 @@ __stp_tf_add_vma(struct task_struct *tsk, unsigned long addr, "vma (pid: %d, vm_start: 0x%lx) present?\n", tsk->pid, vma->vm_start); #endif - mutex_unlock(&__stp_tf_vma_mutex); + write_unlock_irqrestore(&__stp_tf_vma_lock, flags); return -EBUSY; /* Already there */ } } @@ -149,7 +159,7 @@ __stp_tf_add_vma(struct task_struct *tsk, unsigned long addr, // Get an element from the free list. entry = __stp_tf_vma_get_free_entry(); if (!entry) { - mutex_unlock(&__stp_tf_vma_mutex); + write_unlock_irqrestore(&__stp_tf_vma_lock, flags); return -ENOMEM; } entry->pid = tsk->pid; @@ -158,11 +168,12 @@ __stp_tf_add_vma(struct task_struct *tsk, unsigned long addr, entry->vm_end = vma->vm_end; entry->vm_pgoff = vma->vm_pgoff; hlist_add_head(&entry->hlist, head); - mutex_unlock(&__stp_tf_vma_mutex); + write_unlock_irqrestore(&__stp_tf_vma_lock, flags); return 0; } // Remove the vma entry from the vma hash table. +// Takes a write lock on __stp_tf_vma_lock. static int __stp_tf_remove_vma_entry(struct __stp_tf_vma_entry *entry) { @@ -171,10 +182,11 @@ __stp_tf_remove_vma_entry(struct __stp_tf_vma_entry *entry) int found = 0; if (entry != NULL) { - mutex_lock(&__stp_tf_vma_mutex); + unsigned long flags; + write_lock_irqsave(&__stp_tf_vma_lock, flags); hlist_del(&entry->hlist); __stp_tf_vma_put_free_entry(entry); - mutex_unlock(&__stp_tf_vma_mutex); + write_unlock_irqrestore(&__stp_tf_vma_lock, flags); } return 0; } @@ -189,7 +201,7 @@ __stp_tf_vma_map_hash(struct task_struct *tsk) } // Get vma_entry if the vma is present in the vma map hash table. -// Returns NULL if not present. The __stp_tf_vma_mutex must be locked +// Returns NULL if not present. The __stp_tf_vma_lock must be read locked // before calling this function. static struct __stp_tf_vma_entry * __stp_tf_get_vma_map_entry_internal(struct task_struct *tsk, @@ -220,7 +232,10 @@ stap_add_vma_map_info(struct task_struct *tsk, unsigned long vm_start, struct hlist_node *node; struct __stp_tf_vma_entry *entry; - mutex_lock(&__stp_tf_vma_mutex); + unsigned long flags; + // Take a write lock, since we are most likely going to write + // after reading. + write_lock_irqsave(&__stp_tf_vma_lock, flags); entry = __stp_tf_get_vma_map_entry_internal(tsk, vm_start); if (entry != NULL) { #if 0 @@ -228,14 +243,14 @@ stap_add_vma_map_info(struct task_struct *tsk, unsigned long vm_start, "vma (pid: %d, vm_start: 0x%lx) present?\n", tsk->pid, entry->vm_start); #endif - mutex_unlock(&__stp_tf_vma_mutex); + write_unlock_irqrestore(&__stp_tf_vma_lock, flags); return -EBUSY; /* Already there */ } // Get an element from the free list. entry = __stp_tf_vma_get_free_entry(); if (!entry) { - mutex_unlock(&__stp_tf_vma_mutex); + write_unlock_irqrestore(&__stp_tf_vma_lock, flags); return -ENOMEM; } @@ -249,7 +264,7 @@ stap_add_vma_map_info(struct task_struct *tsk, unsigned long vm_start, head = &__stp_tf_vma_map[__stp_tf_vma_map_hash(tsk)]; hlist_add_head(&entry->hlist, head); - mutex_unlock(&__stp_tf_vma_mutex); + write_unlock_irqrestore(&__stp_tf_vma_lock, flags); return 0; } @@ -263,18 +278,21 @@ stap_remove_vma_map_info(struct task_struct *tsk, unsigned long vm_start, struct hlist_node *node; struct __stp_tf_vma_entry *entry; - mutex_lock(&__stp_tf_vma_mutex); + // Take a write lock since we are most likely going to delete + // after reading. + unsigned long flags; + write_lock_irqsave(&__stp_tf_vma_lock, flags); entry = __stp_tf_get_vma_map_entry_internal(tsk, vm_start); if (entry != NULL) { hlist_del(&entry->hlist); __stp_tf_vma_put_free_entry(entry); } - mutex_unlock(&__stp_tf_vma_mutex); + write_unlock_irqrestore(&__stp_tf_vma_lock, flags); return 0; } // Finds vma info if the vma is present in the vma map hash table. -// Returns ESRCH if not present. The __stp_tf_vma_mutex must *not* be +// Returns ESRCH if not present. The __stp_tf_vma_lock must *not* be // locked before calling this function. static int stap_find_vma_map_info(struct task_struct *tsk, unsigned long vm_addr, @@ -287,7 +305,8 @@ stap_find_vma_map_info(struct task_struct *tsk, unsigned long vm_addr, struct __stp_tf_vma_entry *found_entry = NULL; int rc = ESRCH; - mutex_lock(&__stp_tf_vma_mutex); + unsigned long flags; + read_lock_irqsave(&__stp_tf_vma_lock, flags); head = &__stp_tf_vma_map[__stp_tf_vma_map_hash(tsk)]; hlist_for_each_entry(entry, node, head, hlist) { if (tsk->pid == entry->pid @@ -306,7 +325,7 @@ stap_find_vma_map_info(struct task_struct *tsk, unsigned long vm_addr, *vm_pgoff = found_entry->vm_pgoff; rc = 0; } - mutex_unlock(&__stp_tf_vma_mutex); + read_unlock_irqrestore(&__stp_tf_vma_lock, flags); return rc; } @@ -320,15 +339,16 @@ __stp_tf_get_vma_entry_addr(struct task_struct *tsk, unsigned long addr) struct hlist_node *node; struct __stp_tf_vma_entry *entry; - mutex_lock(&__stp_tf_vma_mutex); + unsigned long flags; + read_lock_irqsave(&__stp_tf_vma_lock, flags); head = &__stp_tf_vma_map[__stp_tf_vma_map_hash(tsk)]; hlist_for_each_entry(entry, node, head, hlist) { if (tsk->pid == entry->pid && addr >= entry->vm_start && addr < entry->vm_end) { - mutex_unlock(&__stp_tf_vma_mutex); + read_unlock_irqrestore(&__stp_tf_vma_lock, flags); return entry; } } - mutex_unlock(&__stp_tf_vma_mutex); + read_unlock_irqrestore(&__stp_tf_vma_lock, flags); return NULL; } -- cgit From 1882152af71f8339c91751ee17c97f1c4d18660a Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Thu, 19 Mar 2009 14:28:14 +0100 Subject: Get rid of __stp_tf_get_vma_entry_addr function. * runtime/task_finder_vma.c (__stp_tf_get_vma_entry_addr): Deleted. (struct __stp_tf_vma_entry): Replace module by user void* field. (stap_add_vma_map_info): Take a void *user arg. (stap_find_vma_map_info): Take a void **user arg. * runtime/sym.c (_stp_mod_sec_lookup): Use stap_find_vma_map_info. --- runtime/sym.c | 28 ++++++++++++++++------------ runtime/task_finder_vma.c | 40 +++++++++------------------------------- 2 files changed, 25 insertions(+), 43 deletions(-) (limited to 'runtime') diff --git a/runtime/sym.c b/runtime/sym.c index 3788544e..1fe3f818 100644 --- a/runtime/sym.c +++ b/runtime/sym.c @@ -79,25 +79,29 @@ static struct _stp_module *_stp_mod_sec_lookup(unsigned long addr, struct task_struct *task, struct _stp_section **sec) { + void *user; struct _stp_module *m = NULL; unsigned midx = 0; unsigned long closest_section_offset = ~0; // Try vma matching first if task given. - struct __stp_tf_vma_entry *entry; + unsigned long vm_start; if (task) { - entry = __stp_tf_get_vma_entry_addr(task, addr); - if (entry != NULL && entry->module != NULL) - { - m = entry->module; - *sec = &m->sections[0]; // XXX check actual section and relocate - dbug_sym(1, "found section %s in module %s at 0x%lx\n", - m->sections[0].name, m->name, entry->vm_start); - if (strcmp(".dynamic", m->sections[0].name) == 0) - m->sections[0].addr = entry->vm_start; // cheat... - return m; - } + + if (stap_find_vma_map_info(task, addr, + &vm_start, NULL, + NULL, &user) == 0) + if (user != NULL) + { + m = (struct _stp_module *)user; + *sec = &m->sections[0]; // XXX check actual section and relocate + dbug_sym(1, "found section %s in module %s at 0x%lx\n", + m->sections[0].name, m->name, vm_start); + if (strcmp(".dynamic", m->sections[0].name) == 0) + m->sections[0].addr = vm_start; // cheat... + return m; + } } for (midx = 0; midx < _stp_num_modules; midx++) diff --git a/runtime/task_finder_vma.c b/runtime/task_finder_vma.c index a210d92e..ed9c6f4f 100644 --- a/runtime/task_finder_vma.c +++ b/runtime/task_finder_vma.c @@ -11,8 +11,8 @@ // if we guarantee that in interrupt context we only read, not write // the datastructures. We should never change the hash table or the // contents in interrupt context (which should only ever call -// __stp_tf_get_vma_entry_addr for symbol lookup). So we might want -// to look into that if this seems a bottleneck. +// stap_find_vma_map_info for getting stored vma info). So we might +// want to look into that if this seems a bottleneck. static DEFINE_RWLOCK(__stp_tf_vma_lock); #define __STP_TF_HASH_BITS 4 @@ -32,8 +32,8 @@ struct __stp_tf_vma_entry { unsigned long vm_pgoff; // Is that enough? Should we store a dcookie for vm_file? - // Module that this vma entry is mapped from, if any. - struct _stp_module *module; + // User data (possibly stp_module) + void *user; }; static struct __stp_tf_vma_entry @@ -226,7 +226,7 @@ __stp_tf_get_vma_map_entry_internal(struct task_struct *tsk, static int stap_add_vma_map_info(struct task_struct *tsk, unsigned long vm_start, unsigned long vm_end, unsigned long vm_pgoff, - struct _stp_module *module) + void *user) { struct hlist_head *head; struct hlist_node *node; @@ -260,7 +260,7 @@ stap_add_vma_map_info(struct task_struct *tsk, unsigned long vm_start, entry->vm_start = vm_start; entry->vm_end = vm_end; entry->vm_pgoff = vm_pgoff; - entry->module = module; + entry->user = user; head = &__stp_tf_vma_map[__stp_tf_vma_map_hash(tsk)]; hlist_add_head(&entry->hlist, head); @@ -297,7 +297,7 @@ stap_remove_vma_map_info(struct task_struct *tsk, unsigned long vm_start, static int stap_find_vma_map_info(struct task_struct *tsk, unsigned long vm_addr, unsigned long *vm_start, unsigned long *vm_end, - unsigned long *vm_pgoff) + unsigned long *vm_pgoff, void **user) { struct hlist_head *head; struct hlist_node *node; @@ -323,32 +323,10 @@ stap_find_vma_map_info(struct task_struct *tsk, unsigned long vm_addr, *vm_end = found_entry->vm_end; if (vm_pgoff != NULL) *vm_pgoff = found_entry->vm_pgoff; + if (user != NULL) + *user = found_entry->user; rc = 0; } read_unlock_irqrestore(&__stp_tf_vma_lock, flags); return rc; } - -// Get vma_entry of the address (vm_start/vm_end) if the vma is -// present in the vma hash table containing. -// Returns NULL if not present. -static struct __stp_tf_vma_entry * -__stp_tf_get_vma_entry_addr(struct task_struct *tsk, unsigned long addr) -{ - struct hlist_head *head; - struct hlist_node *node; - struct __stp_tf_vma_entry *entry; - - unsigned long flags; - read_lock_irqsave(&__stp_tf_vma_lock, flags); - head = &__stp_tf_vma_map[__stp_tf_vma_map_hash(tsk)]; - hlist_for_each_entry(entry, node, head, hlist) { - if (tsk->pid == entry->pid - && addr >= entry->vm_start && addr < entry->vm_end) { - read_unlock_irqrestore(&__stp_tf_vma_lock, flags); - return entry; - } - } - read_unlock_irqrestore(&__stp_tf_vma_lock, flags); - return NULL; -} -- cgit From bfd5b4a50e6797da88a11eb6a52c185d2826a0d1 Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Thu, 19 Mar 2009 14:51:33 +0100 Subject: Initialize user and vm_start before use. * runtime/sym.c (_stp_mod_sec_lookup): Initialize user and vm_start. --- runtime/sym.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'runtime') diff --git a/runtime/sym.c b/runtime/sym.c index 1fe3f818..ed108f08 100644 --- a/runtime/sym.c +++ b/runtime/sym.c @@ -79,16 +79,15 @@ static struct _stp_module *_stp_mod_sec_lookup(unsigned long addr, struct task_struct *task, struct _stp_section **sec) { - void *user; + void *user = NULL; struct _stp_module *m = NULL; unsigned midx = 0; unsigned long closest_section_offset = ~0; // Try vma matching first if task given. - unsigned long vm_start; if (task) { - + unsigned long vm_start = 0; if (stap_find_vma_map_info(task, addr, &vm_start, NULL, NULL, &user) == 0) -- cgit From 5e94ef56760c087784e485c35521a6e438cfc3e5 Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Thu, 19 Mar 2009 16:51:02 +0100 Subject: Remove __stp_tf_vm_cb from task_finder interface. * runtime/task_finder.c (__stp_tf_vm_cb): Removed. * tapsets.cxx (utrace_derived_probe_group::emit_module_decls): Output task finder vma callback and hook it. --- runtime/task_finder.c | 34 ---------------------------------- 1 file changed, 34 deletions(-) (limited to 'runtime') diff --git a/runtime/task_finder.c b/runtime/task_finder.c index 38f9145d..1db2bc2a 100644 --- a/runtime/task_finder.c +++ b/runtime/task_finder.c @@ -55,40 +55,6 @@ typedef int (*stap_task_finder_vm_callback)(struct stap_task_finder_target *tgt, unsigned long vm_end, unsigned long vm_pgoff); -static int __stp_tf_vm_cb(struct stap_task_finder_target *tgt, - struct task_struct *tsk, - int map_p, char *vm_path, - unsigned long vm_start, - unsigned long vm_end, - unsigned long vm_pgoff) -{ - int i; -#ifdef DEBUG_TASK_FINDER_VMA - _stp_dbug(__FUNCTION__, __LINE__, - "vm_cb: tsk %d:%d path %s, start 0x%08lx, end 0x%08lx, offset 0x%lx\n", - tsk->pid, map_p, vm_path, vm_start, vm_end, vm_pgoff); -#endif - if (map_p) { - struct _stp_module *module = NULL; - if (vm_path != NULL) - for (i = 0; i < _stp_num_modules; i++) - if (strcmp(vm_path, _stp_modules[i]->path) == 0) - { -#ifdef DEBUG_TASK_FINDER_VMA - _stp_dbug(__FUNCTION__, __LINE__, - "vm_cb: matched path %s to module\n", vm_path); -#endif - module = _stp_modules[i]; - break; - } - stap_add_vma_map_info(tsk, vm_start, vm_end, vm_pgoff, module); - } - else { - stap_remove_vma_map_info(tsk, vm_start, vm_end, vm_pgoff); - } - return 0; -} - struct stap_task_finder_target { /* private: */ struct list_head list; /* __stp_task_finder_list linkage */ -- cgit From f735e6054d182b589750db65a906c1851faa9d75 Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Fri, 20 Mar 2009 13:30:58 +0100 Subject: Move _stp_tf_vm_cb to sym.c. * tapsets.cxx (utrace_derived_probe_group::emit_module_decls): Remove output task finder vma callback _stp_tf_vm_cb. * runtime/sym.c (_stp_tf_vm_cb): And add it here. --- runtime/sym.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) (limited to 'runtime') diff --git a/runtime/sym.c b/runtime/sym.c index ed108f08..d0c5d9fd 100644 --- a/runtime/sym.c +++ b/runtime/sym.c @@ -20,6 +20,40 @@ * @{ */ +/* Callback that needs to be registered (in tapsets.cxx for + emit_module_init) for every user task path or pid for which we + might need symbols or unwind info. */ +static int _stp_tf_vm_cb(struct stap_task_finder_target *tgt, + struct task_struct *tsk, + int map_p, char *vm_path, + unsigned long vm_start, unsigned long vm_end, + unsigned long vm_pgoff) +{ + int i; +#ifdef DEBUG_TASK_FINDER_VMA + _stp_dbug(__FUNCTION__, __LINE__, "vm_cb: tsk %d:%d path %s, start 0x%08lx, end 0x%08lx, offset 0x%lx\n", tsk->pid, map_p, vm_path, vm_start, vm_end, vm_pgoff); +#endif + if (map_p) + { + struct _stp_module *module = NULL; + if (vm_path != NULL) + for (i = 0; i < _stp_num_modules; i++) + if (strcmp(vm_path, _stp_modules[i]->path) == 0) + { +#ifdef DEBUG_TASK_FINDER_VMA + _stp_dbug(__FUNCTION__, __LINE__, "vm_cb: matched path %s to module\n", vm_path); +#endif + module = _stp_modules[i]; + break; + } + stap_add_vma_map_info(tsk, vm_start, vm_end, vm_pgoff, module); + } + else + stap_remove_vma_map_info(tsk, vm_start, vm_end, vm_pgoff); + + return 0; +} + /* XXX: this needs to be address-space-specific. */ static unsigned long _stp_module_relocate(const char *module, const char *section, unsigned long offset) { -- cgit From 489afa702639fd10e9756795bd516d939766247d Mon Sep 17 00:00:00 2001 From: Maynard Johnson Date: Wed, 1 Apr 2009 09:51:57 -0500 Subject: Fix runtime/itrace.c to call arch_has_*_step() prior to calling utrace_control As Roland pointed out in his Mar 31 reply to a posting of mine (subject: Re: Testing insn.block probe point uncovers possible utrace bug), utrace_control() documents that the caller must ensure that arch_has_single_step and arch_has_block_step are defined before trying to use those step modes. The attached patch addresses that issue. I've tested this patch on x86_64 arch, and a block step test runs successfully, since block step is supported on that arch. Testing on ppc64 arch, the test fails as expected (since block step is not supported on ppc64 yet) with: "ERROR: callback for failed: 1" which is sent to stdderr and "usr_itrace_init: arch does not support block step mode" which is sent to the system log. This isn't the most user-friendly way of surfacing an error. Perhaps the stap runtime could have a set of defined return codes that would be mapped to strings so the user can get an idea of what the error is without looking in the system log. But that's a side issue, of course. Regards, -Maynard --- runtime/itrace.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'runtime') diff --git a/runtime/itrace.c b/runtime/itrace.c index 97ba427e..69f3bf74 100644 --- a/runtime/itrace.c +++ b/runtime/itrace.c @@ -318,6 +318,19 @@ static int usr_itrace_init(int single_step, pid_t tid, struct stap_itrace_probe struct task_struct *tsk; spin_lock_init(&itrace_lock); + + /* 'arch_has_single_step' needs to be defined for either single step mode + * or branch mode. + */ + if (!arch_has_single_step()) { + printk(KERN_ERR "usr_itrace_init: arch does not support step mode\n"); + return 1; + } + if (!single_step && !arch_has_block_step()) { + printk(KERN_ERR "usr_itrace_init: arch does not support block step mode\n"); + return 1; + } + rcu_read_lock(); #ifdef STAPCONF_FIND_TASK_PID tsk = find_task_by_pid(tid); -- cgit From f3921e808cc95592308727d011d7718f053008e2 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Wed, 1 Apr 2009 16:39:12 -0700 Subject: Revert "Fix runtime/itrace.c to call arch_has_*_step() prior to calling utrace_control" This reverts commit 489afa702639fd10e9756795bd516d939766247d. --- runtime/itrace.c | 13 ------------- 1 file changed, 13 deletions(-) (limited to 'runtime') diff --git a/runtime/itrace.c b/runtime/itrace.c index 69f3bf74..97ba427e 100644 --- a/runtime/itrace.c +++ b/runtime/itrace.c @@ -318,19 +318,6 @@ static int usr_itrace_init(int single_step, pid_t tid, struct stap_itrace_probe struct task_struct *tsk; spin_lock_init(&itrace_lock); - - /* 'arch_has_single_step' needs to be defined for either single step mode - * or branch mode. - */ - if (!arch_has_single_step()) { - printk(KERN_ERR "usr_itrace_init: arch does not support step mode\n"); - return 1; - } - if (!single_step && !arch_has_block_step()) { - printk(KERN_ERR "usr_itrace_init: arch does not support block step mode\n"); - return 1; - } - rcu_read_lock(); #ifdef STAPCONF_FIND_TASK_PID tsk = find_task_by_pid(tid); -- cgit From dcfd7fed7088871f46d9da7183e485877fb2d81f Mon Sep 17 00:00:00 2001 From: "Frank Ch. Eigler" Date: Wed, 1 Apr 2009 22:50:47 -0400 Subject: PR10019: --skip-badvars to suppress run-time memory errors too * NEWS: Note this change. * hash.cxx (find_script_hash): Add s.skip_badvars into hash. * translate.cxx (translate_pass): Emit STP_SKIP_BADVARS. * runtime/loc2c-runtime.h (DEREF_FAULT, STORE_DEREF_FAULT): Provide dummy implementation if STP_SKIP_BADVARS. --- runtime/loc2c-runtime.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'runtime') diff --git a/runtime/loc2c-runtime.h b/runtime/loc2c-runtime.h index 16ddb950..eaf47cad 100644 --- a/runtime/loc2c-runtime.h +++ b/runtime/loc2c-runtime.h @@ -62,6 +62,10 @@ must work right for kernel addresses, and can use whatever existing machine-specific kernel macros are convenient. */ +#if STP_SKIP_BADVARS +#define DEREF_FAULT(addr) ({0; }) +#define STORE_DEREF_FAULT(addr) ({0; }) +#else #define DEREF_FAULT(addr) ({ \ snprintf(c->error_buffer, sizeof(c->error_buffer), \ "kernel read fault at 0x%p (%s)", (void *)(intptr_t)(addr), #addr); \ @@ -75,7 +79,7 @@ c->last_error = c->error_buffer; \ goto deref_fault; \ }) - +#endif #if defined (STAPCONF_X86_UNIREGS) && defined (__i386__) -- cgit From 5ff8be611e2460ad5e0a15e1cd0936c38e518c7f Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Thu, 2 Apr 2009 13:49:12 +0200 Subject: Make task_finder.c compiler if ! defined (CONFIG_UTRACE). * runtime/task-finder.c: If ! defined (CONFIG_UTRACE) define dummy noop API for sym.c consisting of struct stap_task_finder_target, stap_add_vma_map_info, stap_remove_vma_map_info and stap_find_vma_map_info. --- runtime/task_finder.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) (limited to 'runtime') diff --git a/runtime/task_finder.c b/runtime/task_finder.c index 44dca296..3507c669 100644 --- a/runtime/task_finder.c +++ b/runtime/task_finder.c @@ -2,8 +2,32 @@ #define TASK_FINDER_C #if ! defined(CONFIG_UTRACE) -#error "Need CONFIG_UTRACE!" -#endif +/* Dummy definitions for use in sym.c */ +struct stap_task_finder_target { }; + +static int +stap_add_vma_map_info(struct task_struct *tsk, unsigned long vm_start, + unsigned long vm_end, unsigned long vm_pgoff, + void *user) +{ + return 0; +} + +static int +stap_remove_vma_map_info(struct task_struct *tsk, unsigned long vm_start, + unsigned long vm_end, unsigned long vm_pgoff) +{ + return 0; +} + +static int +stap_find_vma_map_info(struct task_struct *tsk, unsigned long vm_addr, + unsigned long *vm_start, unsigned long *vm_end, + unsigned long *vm_pgoff, void **user) +{ + return ESRCH; +} +#else #include @@ -1358,4 +1382,6 @@ stap_stop_task_finder(void) } +#endif /* defined(CONFIG_UTRACE) */ + #endif /* TASK_FINDER_C */ -- cgit