diff --git a/Makefile.am b/Makefile.am index 4003df38..89f43417 100644 --- a/Makefile.am +++ b/Makefile.am @@ -17,8 +17,6 @@ SYSPROF_CORE = \ demangle.c \ elfparser.c \ elfparser.h \ - process.h \ - process.c \ profile.h \ profile.c \ sfile.h \ diff --git a/TODO b/TODO index 11a4ab49..8a5c6be3 100644 --- a/TODO +++ b/TODO @@ -23,6 +23,12 @@ Before 1.0.4: Before 1.2: +* Get rid of remaining gulongs (use uint64_t instead) + +* Move binfile hash table to state_t. + +* Get rid of process.c + * On 32 bit, NMI stackframes are not filtered out which leads to wrong kernel traces diff --git a/binfile.c b/binfile.c index 5040741c..510e216c 100644 --- a/binfile.c +++ b/binfile.c @@ -36,7 +36,6 @@ #include "binfile.h" #include "elfparser.h" -#include "process.h" struct BinFile { @@ -258,8 +257,158 @@ get_debug_binaries (GList *files, return files; } + static GHashTable *bin_files; +typedef struct Map Map; +struct Map +{ + char * filename; + gulong start; + gulong end; + gulong offset; + gulong inode; + + BinFile * bin_file; +}; + +static Map * +read_maps (int pid, int *n_maps) +{ + char *name = g_strdup_printf ("/proc/%d/maps", pid); + char buffer[1024]; + FILE *in; + GArray *result; + + in = fopen (name, "r"); + if (!in) + { + g_free (name); + return NULL; + } + + result = g_array_new (FALSE, FALSE, sizeof (Map)); + + while (fgets (buffer, sizeof (buffer) - 1, in)) + { + char file[256]; + int count; + gulong start; + gulong end; + gulong offset; + gulong inode; + + count = sscanf ( + buffer, "%lx-%lx %*15s %lx %*x:%*x %lu %255s", + &start, &end, &offset, &inode, file); + + if (count == 5) + { + Map map; + + map.filename = g_strdup (file); + map.start = start; + map.end = end; + + if (strcmp (map.filename, "[vdso]") == 0) + { + /* For the vdso, the kernel reports 'offset' as the + * the same as the mapping addres. This doesn't make + * any sense to me, so we just zero it here. There + * is code in binfile.c (read_inode) that returns 0 + * for [vdso]. + */ + map.offset = 0; + map.inode = 0; + } + else + { + map.offset = offset; + map.inode = inode; + } + + map.bin_file = NULL; + + g_array_append_val (result, map); + } + } + + g_free (name); + fclose (in); + + if (n_maps) + *n_maps = result->len; + + return (Map *)g_array_free (result, FALSE); +} + +static void +free_maps (int *n_maps, + Map *maps) +{ + int i; + + for (i = 0; i < *n_maps; ++i) + { + Map *map = &(maps[i]); + + if (map->filename) + g_free (map->filename); + + if (map->bin_file) + bin_file_free (map->bin_file); + } + + g_free (maps); + *n_maps = 0; +} + +const guint8 * +get_vdso_bytes (gsize *length) +{ + static gboolean has_data; + static const guint8 *bytes = NULL; + static gsize n_bytes = 0; + + if (!has_data) + { + Map *maps; + int n_maps, i; + + maps = read_maps (getpid(), &n_maps); + + for (i = 0; i < n_maps; ++i) + { + Map *map = &(maps[i]); + + if (strcmp (map->filename, "[vdso]") == 0) + { + n_bytes = map->end - map->start; + + /* Dup the memory here so that valgrind will only + * report one 1 byte invalid read instead of + * a ton when the elf parser scans the vdso + * + * The reason we get a spurious invalid read from + * valgrind is that we are getting the address directly + * from /proc/maps, and valgrind knows that its mmap() + * wrapper never returned that address. But since it + * is a legal mapping, it is legal to read it. + */ + bytes = g_memdup ((guint8 *)map->start, n_bytes); + } + } + + has_data = TRUE; + free_maps (&n_maps, maps); + } + + if (length) + *length = n_bytes; + + return bytes; +} + BinFile * bin_file_new (const char *filename) { @@ -294,7 +443,7 @@ bin_file_new (const char *filename) const guint8 *vdso_bytes; gsize length; - vdso_bytes = process_get_vdso_bytes (&length); + vdso_bytes = get_vdso_bytes (&length); if (vdso_bytes) elf = elf_parser_new_from_data (vdso_bytes, length); diff --git a/collector.c b/collector.c index e7ba6827..561211b7 100644 --- a/collector.c +++ b/collector.c @@ -165,11 +165,26 @@ in_dead_period (Collector *collector) return FALSE; } +static int +get_page_size (void) +{ + static int page_size; + static gboolean has_page_size = FALSE; + + if (!has_page_size) + { + page_size = getpagesize(); + has_page_size = TRUE; + } + + return page_size; +} + static void on_read (gpointer data) { counter_t *counter = data; - int mask = (N_PAGES * process_get_page_size() - 1); + int mask = (N_PAGES * get_page_size() - 1); gboolean skip_samples; Collector *collector; uint64_t head, tail; @@ -245,27 +260,27 @@ on_read (gpointer data) static void * map_buffer (counter_t *counter) { - int n_bytes = N_PAGES * process_get_page_size(); + int n_bytes = N_PAGES * get_page_size(); void *address, *a; /* We use the old trick of mapping the ring buffer twice * consecutively, so that we don't need special-case code * to deal with wrapping. */ - address = mmap (NULL, n_bytes * 2 + process_get_page_size(), PROT_NONE, + address = mmap (NULL, n_bytes * 2 + get_page_size(), PROT_NONE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); if (address == MAP_FAILED) fail ("mmap"); - a = mmap (address + n_bytes, n_bytes + process_get_page_size(), + a = mmap (address + n_bytes, n_bytes + get_page_size(), PROT_READ | PROT_WRITE, MAP_SHARED | MAP_FIXED, counter->fd, 0); if (a != address + n_bytes) fail ("mmap"); - a = mmap (address, n_bytes + process_get_page_size(), + a = mmap (address, n_bytes + get_page_size(), PROT_READ | PROT_WRITE, MAP_SHARED | MAP_FIXED, counter->fd, 0); @@ -320,7 +335,7 @@ counter_new (Collector *collector, return NULL; } - counter->data = (uint8_t *)counter->mmap_page + process_get_page_size (); + counter->data = (uint8_t *)counter->mmap_page + get_page_size (); counter->tail = 0; counter->cpu = cpu; counter->partial = g_string_new (NULL); @@ -340,7 +355,7 @@ counter_enable (counter_t *counter) static void counter_free (counter_t *counter) { - munmap (counter->mmap_page, (N_PAGES + 1) * process_get_page_size()); + munmap (counter->mmap_page, (N_PAGES + 1) * get_page_size()); fd_remove_watch (counter->fd); close (counter->fd); @@ -375,8 +390,6 @@ collector_reset (Collector *collector) collector->tracker = NULL; } - process_flush_caches(); - collector->n_samples = 0; g_get_current_time (&collector->latest_reset); diff --git a/process.c b/process.c index 764e756a..bf6991ae 100644 --- a/process.c +++ b/process.c @@ -35,18 +35,6 @@ static GHashTable *processes_by_pid; -typedef struct Map Map; -struct Map -{ - char * filename; - gulong start; - gulong end; - gulong offset; - gulong inode; - - BinFile * bin_file; -}; - struct Process { char * cmdline; @@ -68,142 +56,6 @@ initialize (void) processes_by_pid = g_hash_table_new (g_direct_hash, g_direct_equal); } -static Map * -read_maps (int pid, int *n_maps) -{ - char *name = g_strdup_printf ("/proc/%d/maps", pid); - char buffer[1024]; - FILE *in; - GArray *result; - - in = fopen (name, "r"); - if (!in) - { - g_free (name); - return NULL; - } - - result = g_array_new (FALSE, FALSE, sizeof (Map)); - - while (fgets (buffer, sizeof (buffer) - 1, in)) - { - char file[256]; - int count; - gulong start; - gulong end; - gulong offset; - gulong inode; - - count = sscanf ( - buffer, "%lx-%lx %*15s %lx %*x:%*x %lu %255s", - &start, &end, &offset, &inode, file); - if (count == 5) - { - Map map; - - map.filename = g_strdup (file); - map.start = start; - map.end = end; - - if (strcmp (map.filename, "[vdso]") == 0) - { - /* For the vdso, the kernel reports 'offset' as the - * the same as the mapping addres. This doesn't make - * any sense to me, so we just zero it here. There - * is code in binfile.c (read_inode) that returns 0 - * for [vdso]. - */ - map.offset = 0; - map.inode = 0; - } - else - { - map.offset = offset; - map.inode = inode; - } - - map.bin_file = NULL; - - g_array_append_val (result, map); - } - } - - g_free (name); - fclose (in); - - if (n_maps) - *n_maps = result->len; - - return (Map *)g_array_free (result, FALSE); -} - -static void -free_maps (int *n_maps, - Map *maps) -{ - int i; - - for (i = 0; i < *n_maps; ++i) - { - Map *map = &(maps[i]); - - if (map->filename) - g_free (map->filename); - - if (map->bin_file) - bin_file_free (map->bin_file); - } - - g_free (maps); - *n_maps = 0; -} - -const guint8 * -process_get_vdso_bytes (gsize *length) -{ - static gboolean has_data; - static const guint8 *bytes = NULL; - static gsize n_bytes = 0; - - if (!has_data) - { - Map *maps; - int n_maps, i; - - maps = read_maps (getpid(), &n_maps); - - for (i = 0; i < n_maps; ++i) - { - Map *map = &(maps[i]); - - if (strcmp (map->filename, "[vdso]") == 0) - { - n_bytes = map->end - map->start; - - /* Dup the memory here so that valgrind will only - * report one 1 byte invalid read instead of - * a ton when the elf parser scans the vdso - * - * The reason we get a spurious invalid read from - * valgrind is that we are getting the address directly - * from /proc/maps, and valgrind knows that its mmap() - * wrapper never returned that address. But since it - * is a legal mapping, it is legal to read it. - */ - bytes = g_memdup ((guint8 *)map->start, n_bytes); - } - } - - has_data = TRUE; - free_maps (&n_maps, maps); - } - - if (length) - *length = n_bytes; - - return bytes; -} - static Process * create_process (const char *cmdline, int pid) { @@ -296,21 +148,6 @@ process_has_page (Process *process, gulong addr) return FALSE; } -int -process_get_page_size (void) -{ - static int page_size; - static gboolean has_page_size = FALSE; - - if (!has_page_size) - { - page_size = getpagesize(); - has_page_size = TRUE; - } - - return page_size; -} - void process_ensure_map (Process *process, int pid, gulong addr) { diff --git a/tracker.c b/tracker.c index 05fde6c6..639774c1 100644 --- a/tracker.c +++ b/tracker.c @@ -91,6 +91,7 @@ fake_new_process (tracker_t *tracker, pid_t pid) { tracker_add_process ( tracker, pid, g_strstrip (strchr (lines[i], ':') + 1)); + break; } } @@ -314,7 +315,6 @@ struct process_t pid_t pid; char * comm; - char * undefined; GPtrArray * maps; }; @@ -389,7 +389,6 @@ destroy_process (process_t *process) int i; g_free (process->comm); - g_free (process->undefined); for (i = 0; i < process->maps->len; ++i) { @@ -409,7 +408,6 @@ create_process (state_t *state, new_process_t *new_process) process->pid = new_process->pid; process->comm = g_strdup (new_process->command_line); - process->undefined = NULL; process->maps = g_ptr_array_new (); g_hash_table_insert ( @@ -441,13 +439,12 @@ state_new (void) typedef struct { - gulong address; + gulong address; char *name; } kernel_symbol_t; static void -parse_kallsym_line (const char *line, - GArray *table) +parse_kallsym_line (const char *line, GArray *table) { char **tokens = g_strsplit_set (line, " \t", -1);