On Wednesday 01 September 2004 04:45, William Lee Irwin III wrote:
> Still wrong =( That needs to be strict inequality.
>
>
> --- readprofile.c.orig2 2004-08-31 12:11:54.000000000 -0700
> +++ readprofile.c 2004-08-31 12:44:25.000000000 -0700
> @@ -65,6 +65,7 @@
> int ret = 0;
> long page_mask, start, end;
> unsigned off;
> + ssize_t bufcnt;
>
> if (!state->stext) {
> if (!strcmp(state->sym, "_stext"))
> @@ -101,8 +102,8 @@
> exit(EXIT_FAILURE);
> }
> }
> - if (read(state->fd, state->buf, end - start) == end - start) {
> - for (off = 0; off < (end - start)/sizeof(uint32_t); ++off)
> + if ((bufcnt = read(state->fd, state->buf, end - start)) > 0) {
> + for (off = 0; off < bufcnt/sizeof(uint32_t); ++off)
> state->last->hits += state->buf[off];
> } else {
> ret = 1;
If you're passing profile=0 on boot, it exits very early.
(when readprofile saw the symbol whose address is same with next line's
symbol address)
and,
On ia64, the following System.map was generated:
[...]
a000000100000000 A _stext
a000000100000000 A _text (*)
a000000100000000 T ia64_ivt
a000000100000000 t vhpt_miss
Since the symbol "_text" (*) has absolute symbol type, it could not pass the
is_text() check, and readprofile exits immediately.
BTW, if profile=>1 is passed, The range between start and end in
state_transition() is overlapped every call. Is it intentional?
# readprofile -b (after applied below patch)
[...]
__wake_up_common:
c011f1d8 47 (*)
__wake_up:
c011f1d8 47 (*)
c011f1dc 2
--- readprofile.c.orig 2004-09-02 00:04:55.904405440 +0900
+++ readprofile.c 2004-09-02 00:37:37.277231448 +0900
@@ -25,6 +25,7 @@ struct profile_state {
int fd, shift;
uint32_t *buf;
size_t bufsz;
+ ssize_t bufcnt;
struct sym syms[2], *last, *this;
unsigned long long stext, vaddr;
unsigned long total;
@@ -60,6 +61,32 @@ static long profile_off(unsigned long lo
return (((vaddr - state->stext) >> state->shift) + 1)*sizeof(uint32_t);
}
+int optBin;
+
+static void display_hits(struct profile_state *state)
+{
+ char *name = state->last->name;
+ unsigned long hits = state->last->hits;
+
+ if (!hits)
+ return;
+ if (!optBin)
+ printf("%*lu %s\n", digits(), hits, name);
+ else {
+ unsigned long long vaddr = state->last->vaddr;
+ int shift = state->shift;
+ unsigned int off;
+
+ printf("%s:\n", name);
+ for (off = 0; off < state->bufcnt/sizeof(uint32_t); ++off)
+ if (state->buf[off]) {
+ printf("\t%*llx", digits(),
+ ((vaddr >> shift) + off) << shift);
+ printf("\t%*lu\n", digits(), state->buf[off]);
+ }
+ }
+}
+
static int state_transition(struct profile_state *state)
{
int ret = 0;
@@ -101,16 +128,14 @@ static int state_transition(struct profi
exit(EXIT_FAILURE);
}
}
- if (read(state->fd, state->buf, end - start) == end - start) {
- for (off = 0; off < (end - start)/sizeof(uint32_t); ++off)
+ if ((state->bufcnt = read(state->fd, state->buf, end - start)) > 0) {
+ for (off = 0; off < state->bufcnt/sizeof(uint32_t); ++off)
state->last->hits += state->buf[off];
} else {
ret = 1;
strcpy(state->last->name, "*unknown*");
}
- if (state->last->hits)
- printf("%*lu %s\n", digits(), state->last->hits,
- state->last->name);
+ display_hits(state);
state->total += state->last->hits;
out:
return ret;
@@ -169,7 +194,7 @@ int main(int argc, char * const argv[])
{
FILE *system_map = NULL;
int c, ret, profile_buffer = -1;
- static const char optstr[] = "m:p:";
+ static const char optstr[] = "m:p:b";
while ((c = getopt(argc, argv, optstr)) != EOF) {
switch (c) {
@@ -195,6 +220,9 @@ int main(int argc, char * const argv[])
exit(EXIT_FAILURE);
}
break;
+ case 'b':
+ optBin = 1;
+ break;
case '?':
default:
fprintf(stderr, "usage: %s [ -m mapfile ] "
On Thu, Sep 02, 2004 at 01:09:32AM +0900, mita akinobu wrote:
> If you're passing profile=0 on boot, it exits very early.
> (when readprofile saw the symbol whose address is same with next line's
> symbol address)
> and,
> On ia64, the following System.map was generated:
> [...]
> a000000100000000 A _stext
> a000000100000000 A _text (*)
> a000000100000000 T ia64_ivt
> a000000100000000 t vhpt_miss
> Since the symbol "_text" (*) has absolute symbol type, it could not pass the
> is_text() check, and readprofile exits immediately.
That is bizarre and very irritating, as at first glance it appears to
require strcmp() on every entry until _stext is set.
On Thu, Sep 02, 2004 at 01:09:32AM +0900, mita akinobu wrote:
> BTW, if profile=>1 is passed, The range between start and end in
> state_transition() is overlapped every call. Is it intentional?
> # readprofile -b (after applied below patch)
> [...]
> __wake_up_common:
> c011f1d8 47 (*)
> __wake_up:
> c011f1d8 47 (*)
> c011f1dc 2
Overlapping the symbols was intentional, yes. The way profiling works
is that the shift passed by profile= represents a power-of-two-sized
divisor that the virtual addresses are divided by. These bins don't
respect symbol boundaries and in fact multiple functions may share them.
I choose to account a bin to all symbols whose virtual address range
overlaps it.
> --- readprofile.c.orig 2004-09-02 00:04:55.904405440 +0900
> +++ readprofile.c 2004-09-02 00:37:37.277231448 +0900
> @@ -25,6 +25,7 @@ struct profile_state {
> int fd, shift;
> uint32_t *buf;
> size_t bufsz;
> + ssize_t bufcnt;
> struct sym syms[2], *last, *this;
> unsigned long long stext, vaddr;
> unsigned long total;
> @@ -60,6 +61,32 @@ static long profile_off(unsigned long lo
> return (((vaddr - state->stext) >> state->shift) + 1)*sizeof(uint32_t);
> }
>
> +int optBin;
No global variables and no studlycaps...
On Thu, Sep 02, 2004 at 01:09:32AM +0900, mita akinobu wrote:
> +static void display_hits(struct profile_state *state)
> +{
> + char *name = state->last->name;
> + unsigned long hits = state->last->hits;
> +
> + if (!hits)
> + return;
> + if (!optBin)
> + printf("%*lu %s\n", digits(), hits, name);
> + else {
> + unsigned long long vaddr = state->last->vaddr;
> + int shift = state->shift;
> + unsigned int off;
> +
> + printf("%s:\n", name);
> + for (off = 0; off < state->bufcnt/sizeof(uint32_t); ++off)
> + if (state->buf[off]) {
> + printf("\t%*llx", digits(),
> + ((vaddr >> shift) + off) << shift);
> + printf("\t%*lu\n", digits(), state->buf[off]);
> + }
> + }
> +}
Okay, now that you have a use for ->bufcnt in struct state, it may make
sense to keep it around.
On Thu, Sep 02, 2004 at 01:09:32AM +0900, mita akinobu wrote:
> static int state_transition(struct profile_state *state)
> {
> int ret = 0;
> @@ -101,16 +128,14 @@ static int state_transition(struct profi
> exit(EXIT_FAILURE);
> }
> }
> - if (read(state->fd, state->buf, end - start) == end - start) {
> - for (off = 0; off < (end - start)/sizeof(uint32_t); ++off)
> + if ((state->bufcnt = read(state->fd, state->buf, end - start)) > 0) {
> + for (off = 0; off < state->bufcnt/sizeof(uint32_t); ++off)
> state->last->hits += state->buf[off];
> } else {
> ret = 1;
> strcpy(state->last->name, "*unknown*");
> }
> - if (state->last->hits)
> - printf("%*lu %s\n", digits(), state->last->hits,
> - state->last->name);
> + display_hits(state);
> state->total += state->last->hits;
> out:
> return ret;
It may make more sense to pass a display callback instead, particularly
if potential future usage in a library wants to render to streams that
are not stdout or cares to render to things that aren't streams. This
looks like it's against an older version... this seems to qualify as
enough interest to at least use source control.
-- wli