From: Amitabha R. <ami...@gm...> - 2006-12-16 05:34:17
|
Print out the name of the anon map when possible. This will help identify regions like VDSO. Signed-off-by: Amitabha Roy <ami...@gm...> --- diff -pur oprofile-0.9.2/daemon/opd_anon.c oprofile-0.9.2-uspace-fix/daemon/opd_anon.c --- oprofile-0.9.2/daemon/opd_anon.c 2006-03-20 00:47:01.000000000 +0530 +++ oprofile-0.9.2-uspace-fix/daemon/opd_anon.c 2006-12-16 10:28:32.000000000 +0530 @@ -104,7 +104,7 @@ static void clear_anon_maps(struct trans static void -add_anon_mapping(struct transient * trans, vma_t start, vma_t end) +add_anon_mapping(struct transient * trans, vma_t start, vma_t end, char * name) { unsigned long hash = hash_anon(trans->tgid, trans->app_cookie); struct anon_mapping * m = xmalloc(sizeof(struct anon_mapping)); @@ -112,6 +112,7 @@ add_anon_mapping(struct transient * tran m->app_cookie = trans->app_cookie; m->start = start; m->end = end; + strncpy(m->name, name, MAX_IMAGE_NAME_SIZE + 1); list_add_tail(&m->list, &hashes[hash]); list_add_tail(&m->lru_list, &lru); if (++nr_lru == LRU_SIZE) @@ -139,16 +140,18 @@ static void get_anon_maps(struct transie return; while (fgets(buf, PATH_MAX, fp) != NULL) { - char tmp[20+1]; + char tmp[MAX_IMAGE_NAME_SIZE + 1]; + char name[MAX_IMAGE_NAME_SIZE + 1]; /* Note that this actually includes all mappings, * since we want stuff like [heap] */ + name[0]=0; ret = sscanf(buf, "%lx-%lx %20s %20s %20s %20s %20s", - &start, &end, tmp, tmp, tmp, tmp, tmp); + &start, &end, tmp, tmp, tmp, tmp, name, tmp); if (ret < 6) continue; - add_anon_mapping(trans, start, end); + add_anon_mapping(trans, start, end, name); } fclose(fp); Only in oprofile-0.9.2-uspace-fix/daemon: opd_anon.c~ diff -pur oprofile-0.9.2/daemon/opd_anon.h oprofile-0.9.2-uspace-fix/daemon/opd_anon.h --- oprofile-0.9.2/daemon/opd_anon.h 2005-05-02 20:36:57.000000000 +0530 +++ oprofile-0.9.2-uspace-fix/daemon/opd_anon.h 2006-12-15 23:41:41.000000000 +0530 @@ -25,6 +25,9 @@ struct transient; */ #define VMA_SHIFT 13 +/* Maximum size of the image name considered */ +#define MAX_IMAGE_NAME_SIZE 20 + struct anon_mapping { /** start of the mapping */ vma_t start; @@ -38,6 +41,7 @@ struct anon_mapping { struct list_head list; /** lru list */ struct list_head lru_list; + char name[MAX_IMAGE_NAME_SIZE+1]; }; /** diff -pur oprofile-0.9.2/daemon/opd_mangling.c oprofile-0.9.2-uspace-fix/daemon/opd_mangling.c --- oprofile-0.9.2/daemon/opd_mangling.c 2005-05-02 20:36:58.000000000 +0530 +++ oprofile-0.9.2-uspace-fix/daemon/opd_mangling.c 2006-12-16 11:00:20.000000000 +0530 @@ -57,8 +57,10 @@ static char const * get_dep_name(struct static char * mangle_anon(struct anon_mapping const * anon) { char * name = xmalloc(PATH_MAX); - snprintf(name, 1024, "%u.0x%llx.0x%llx", (unsigned int)anon->tgid, - anon->start, anon->end); + + snprintf(name, 1024, " %s %u.0x%llx.0x%llx", anon->name, + (unsigned int)anon->tgid, anon->start, anon->end); + return name; } |
From: Amitabha R. <ami...@gm...> - 2006-12-21 13:17:02
|
Hi John Don't know if you've see this patch ? I had sent it out as a userspace only solution to the VDSO labeling problem following your suggestions. Thanks -Amitabha ---------- Forwarded message ---------- From: Amitabha Roy <ami...@gm...> Date: Dec 16, 2006 11:04 AM Subject: [PATCH] Print out the name of the anon region if possible To: opr...@li... Cc: John Levon <le...@mo...> Print out the name of the anon map when possible. This will help identify regions like VDSO. Signed-off-by: Amitabha Roy <ami...@gm...> --- diff -pur oprofile-0.9.2/daemon/opd_anon.c oprofile-0.9.2-uspace-fix/daemon/opd_anon.c --- oprofile-0.9.2/daemon/opd_anon.c 2006-03-20 00:47:01.000000000 +0530 +++ oprofile-0.9.2-uspace-fix/daemon/opd_anon.c 2006-12-16 10:28:32.000000000 +0530 @@ -104,7 +104,7 @@ static void clear_anon_maps(struct trans static void -add_anon_mapping(struct transient * trans, vma_t start, vma_t end) +add_anon_mapping(struct transient * trans, vma_t start, vma_t end, char * name) { unsigned long hash = hash_anon(trans->tgid, trans->app_cookie); struct anon_mapping * m = xmalloc(sizeof(struct anon_mapping)); @@ -112,6 +112,7 @@ add_anon_mapping(struct transient * tran m->app_cookie = trans->app_cookie; m->start = start; m->end = end; + strncpy(m->name, name, MAX_IMAGE_NAME_SIZE + 1); list_add_tail(&m->list, &hashes[hash]); list_add_tail(&m->lru_list, &lru); if (++nr_lru == LRU_SIZE) @@ -139,16 +140,18 @@ static void get_anon_maps(struct transie return; while (fgets(buf, PATH_MAX, fp) != NULL) { - char tmp[20+1]; + char tmp[MAX_IMAGE_NAME_SIZE + 1]; + char name[MAX_IMAGE_NAME_SIZE + 1]; /* Note that this actually includes all mappings, * since we want stuff like [heap] */ + name[0]=0; ret = sscanf(buf, "%lx-%lx %20s %20s %20s %20s %20s", - &start, &end, tmp, tmp, tmp, tmp, tmp); + &start, &end, tmp, tmp, tmp, tmp, name, tmp); if (ret < 6) continue; - add_anon_mapping(trans, start, end); + add_anon_mapping(trans, start, end, name); } fclose(fp); Only in oprofile-0.9.2-uspace-fix/daemon: opd_anon.c~ diff -pur oprofile-0.9.2/daemon/opd_anon.h oprofile-0.9.2-uspace-fix/daemon/opd_anon.h --- oprofile-0.9.2/daemon/opd_anon.h 2005-05-02 20:36:57.000000000 +0530 +++ oprofile-0.9.2-uspace-fix/daemon/opd_anon.h 2006-12-15 23:41:41.000000000 +0530 @@ -25,6 +25,9 @@ struct transient; */ #define VMA_SHIFT 13 +/* Maximum size of the image name considered */ +#define MAX_IMAGE_NAME_SIZE 20 + struct anon_mapping { /** start of the mapping */ vma_t start; @@ -38,6 +41,7 @@ struct anon_mapping { struct list_head list; /** lru list */ struct list_head lru_list; + char name[MAX_IMAGE_NAME_SIZE+1]; }; /** diff -pur oprofile-0.9.2/daemon/opd_mangling.c oprofile-0.9.2-uspace-fix/daemon/opd_mangling.c --- oprofile-0.9.2/daemon/opd_mangling.c 2005-05-02 20:36:58.000000000 +0530 +++ oprofile-0.9.2-uspace-fix/daemon/opd_mangling.c 2006-12-16 11:00:20.000000000 +0530 @@ -57,8 +57,10 @@ static char const * get_dep_name(struct static char * mangle_anon(struct anon_mapping const * anon) { char * name = xmalloc(PATH_MAX); - snprintf(name, 1024, "%u.0x%llx.0x%llx", (unsigned int)anon->tgid, - anon->start, anon->end); + + snprintf(name, 1024, " %s %u.0x%llx.0x%llx", anon->name, + (unsigned int)anon->tgid, anon->start, anon->end); + return name; } |
From: John L. <le...@mo...> - 2006-12-21 13:21:23
|
On Sat, Dec 16, 2006 at 11:04:12AM +0530, Amitabha Roy wrote: > static void > -add_anon_mapping(struct transient * trans, vma_t start, vma_t end) > +add_anon_mapping(struct transient * trans, vma_t start, vma_t end, char * > name) Your mailer mangled the patch john |
From: John L. <le...@mo...> - 2006-12-21 13:23:31
|
On Sat, Dec 16, 2006 at 11:04:12AM +0530, Amitabha Roy wrote: > + name[0]=0; name[0] = '\0'; > - snprintf(name, 1024, "%u.0x%llx.0x%llx", (unsigned int)anon->tgid, > - anon->start, anon->end); > + > + snprintf(name, 1024, " %s %u.0x%llx.0x%llx", anon->name, > + (unsigned int)anon->tgid, anon->start, anon->end); No spaces in the file names please. Where have you changed the demangling code to understand the new format? regards john |
From: Amitabha R. <ami...@gm...> - 2006-12-21 13:59:40
|
gmail mangles patches ? :( Ok I will use an attachment next time. Sorry. It outputs stuff like (no spaces now): 1 50.0000 sshd 1 50.0000 anon (tgid:[vdso] 4117 range:0xb7f8c000-0xb7f8d000) and with -l 1 0.2551 anon (tgid:[vdso] 4117 range:0xb7f8c000-0xb7f8d000) sshd (no symbols) Also I couldnt find any code flow that would demangle this. Can you point me to a file/function that you want me to fix? Or perhaps a use case that would exercise the code you have in mind ? Thanks -Amitabha On 12/21/06, John Levon <le...@mo...> wrote: > On Sat, Dec 16, 2006 at 11:04:12AM +0530, Amitabha Roy wrote: > > > + name[0]=0; > > name[0] = '\0'; > > > - snprintf(name, 1024, "%u.0x%llx.0x%llx", (unsigned int)anon->tgid, > > - anon->start, anon->end); > > + > > + snprintf(name, 1024, " %s %u.0x%llx.0x%llx", anon->name, > > + (unsigned int)anon->tgid, anon->start, anon->end); > > No spaces in the file names please. Where have you changed the > demangling code to understand the new format? > > regards > john > |
From: Amitabha R. <ami...@gm...> - 2006-12-21 15:18:53
Attachments:
oprofile-uspace-patch.txt
|
John I've attached the updated [with your suggestions] patch (to hopefully get around the mangling). Let me know if you still have concerns. Thanks -Amitabha On 12/21/06, Amitabha Roy <ami...@gm...> wrote: > gmail mangles patches ? :( > Ok I will use an attachment next time. Sorry. > > It outputs stuff like (no spaces now): > 1 50.0000 sshd > 1 50.0000 anon (tgid:[vdso] 4117 range:0xb7f8c000-0xb7f8d000) > > and with -l > > 1 0.2551 anon (tgid:[vdso] 4117 range:0xb7f8c000-0xb7f8d000) > sshd (no symbols) > > > > Also I couldnt find any code flow that would demangle this. Can you > point me to a file/function that you want me to fix? Or perhaps a use > case that would exercise the code you have in mind ? > > Thanks > -Amitabha > > > > > On 12/21/06, John Levon <le...@mo...> wrote: > > On Sat, Dec 16, 2006 at 11:04:12AM +0530, Amitabha Roy wrote: > > > > > + name[0]=0; > > > > name[0] = '\0'; > > > > > - snprintf(name, 1024, "%u.0x%llx.0x%llx", (unsigned int)anon->tgid, > > > - anon->start, anon->end); > > > + > > > + snprintf(name, 1024, " %s %u.0x%llx.0x%llx", anon->name, > > > + (unsigned int)anon->tgid, anon->start, anon->end); > > > > No spaces in the file names please. Where have you changed the > > demangling code to understand the new format? > > > > regards > > john > > > |
From: Amitabha R. <ami...@gm...> - 2006-12-22 12:20:54
|
John Is this patch ok, or does it need more work ? -Amitabha On 12/21/06, Amitabha Roy <ami...@gm...> wrote: > John > > I've attached the updated [with your suggestions] patch (to hopefully > get around the mangling). > Let me know if you still have concerns. > > Thanks > -Amitabha > > > On 12/21/06, Amitabha Roy <ami...@gm...> wrote: > > gmail mangles patches ? :( > > Ok I will use an attachment next time. Sorry. > > > > It outputs stuff like (no spaces now): > > 1 50.0000 sshd > > 1 50.0000 anon (tgid:[vdso] 4117 range:0xb7f8c000-0xb7f8d000) > > > > and with -l > > > > 1 0.2551 anon (tgid:[vdso] 4117 range:0xb7f8c000-0xb7f8d000) > > sshd (no symbols) > > > > > > > > Also I couldnt find any code flow that would demangle this. Can you > > point me to a file/function that you want me to fix? Or perhaps a use > > case that would exercise the code you have in mind ? > > > > Thanks > > -Amitabha > > > > > > > > > > On 12/21/06, John Levon <le...@mo...> wrote: > > > On Sat, Dec 16, 2006 at 11:04:12AM +0530, Amitabha Roy wrote: > > > > > > > + name[0]=0; > > > > > > name[0] = '\0'; > > > > > > > - snprintf(name, 1024, "%u.0x%llx.0x%llx", (unsigned int)anon->tgid, > > > > - anon->start, anon->end); > > > > + > > > > + snprintf(name, 1024, " %s %u.0x%llx.0x%llx", anon->name, > > > > + (unsigned int)anon->tgid, anon->start, anon->end); > > > > > > No spaces in the file names please. Where have you changed the > > > demangling code to understand the new format? > > > > > > regards > > > john > > > > > > > > |
From: John L. <le...@mo...> - 2006-12-22 12:29:42
|
On Fri, Dec 22, 2006 at 05:50:53PM +0530, Amitabha Roy wrote: > Is this patch ok, or does it need more work ? > >> 1 50.0000 anon (tgid:[vdso] 4117 range:0xb7f8c000-0xb7f8d000) This looks obviously wrong. It should be something like: 1 50.0000 [vdso] (tgid:4117 range:0xb7f8c000-0xb7f8d000) The demangling fix will be somewhere in libpp most likely. regards john |
From: Amitabha R. <ami...@gm...> - 2006-12-22 12:54:46
|
I think it looks ok. The anon should be there to make sure no one is confused about backing store for the pages (not a file). It is arguable about putting the name after the tgid, that can be fixed. Are you saying that anon mangles also go through a demangling process ? Why ? There are no symbols there. Can you give me a use case which would actually put that mangled name through a demangle ? I couldnt find anything after looking around (I dont the code that well but I ran as many tests as I could think of). -Amitabha On 12/22/06, John Levon <le...@mo...> wrote: > On Fri, Dec 22, 2006 at 05:50:53PM +0530, Amitabha Roy wrote: > > > Is this patch ok, or does it need more work ? > > > >> 1 50.0000 anon (tgid:[vdso] 4117 range:0xb7f8c000-0xb7f8d000) > > This looks obviously wrong. It should be something like: > > 1 50.0000 [vdso] (tgid:4117 range:0xb7f8c000-0xb7f8d000) > > The demangling fix will be somewhere in libpp most likely. > > regards > john > |
From: John L. <le...@mo...> - 2006-12-22 13:45:41
|
On Fri, Dec 22, 2006 at 06:24:46PM +0530, Amitabha Roy wrote: > I think it looks ok. The anon should be there to make sure no one is > confused about backing store for the pages (not a file). It is > arguable about putting the name after the tgid, that can be fixed. It's not arguable, it's plainly meaningless. > Are you saying that anon mangles also go through a demangling process ? Why Of course, the filename for the anon samples file is demangled into what you see in opreport. regards john |
From: Amitabha R. <ami...@gm...> - 2006-12-22 15:21:28
Attachments:
oprofile-uspace-patch.txt
|
Ah. Ok. I did that. Also ran into the demangling code because it broke and changed that too to comprehend the new format for anon regions. The output looks like : GLOBAL_POWER_E...| samples| %| ------------------ 9 81.8182 oprofiled 2 18.1818 [vdso] (tgid:6150 range:0xb7fa6000-0xb7fa7000) Which is what you had wanted ? The sample files have {anon:region_name} in the path now. If the anon region has no name in the map file, it is simply called anon, which would look like this for the above example : 2 18.1818 anon (tgid:6150 range:0xb7fa6000-0xb7fa7000) Please let me know if this is ok. I would like to close on this soon. Thanks -Amitabha On 12/22/06, John Levon <le...@mo...> wrote: > On Fri, Dec 22, 2006 at 06:24:46PM +0530, Amitabha Roy wrote: > > > I think it looks ok. The anon should be there to make sure no one is > > confused about backing store for the pages (not a file). It is > > arguable about putting the name after the tgid, that can be fixed. > > It's not arguable, it's plainly meaningless. > > > Are you saying that anon mangles also go through a demangling process ? Why > > Of course, the filename for the anon samples file is demangled into what > you see in opreport. > > regards > john > |
From: John L. <le...@mo...> - 2006-12-22 16:56:12
|
On Fri, Dec 22, 2006 at 08:51:28PM +0530, Amitabha Roy wrote: > 2 18.1818 [vdso] (tgid:6150 range:0xb7fa6000-0xb7fa7000) Much nicer. > Please let me know if this is ok. I would like to close on this soon. I'll review/apply it as soon as I find time. thanks, john |
From: John L. <le...@mo...> - 2007-02-01 17:39:25
|
On Fri, Dec 22, 2006 at 08:51:28PM +0530, Amitabha Roy wrote: > + name[0]='\0'; > ret = sscanf(buf, "%lx-%lx %20s %20s %20s %20s %20s", > - &start, &end, tmp, tmp, tmp, tmp, tmp); > + &start, &end, tmp, tmp, tmp, tmp, name, tmp); > if (ret < 6) > continue; Hmm: ffffffffff600000-ffffffffffe00000 ---p 00000000 00:00 0 [vdso] This should be: start, &end, tmp, tmp, tmp, tmp, name); Surely? regards john |