On Mon, Nov 30, 2020 at 09:27:57AM -0800, [email protected] wrote:

SNIP

> @@ -172,7 +172,7 @@ dump_raw_samples(struct perf_tool *tool,
>  {
>       struct perf_mem *mem = container_of(tool, struct perf_mem, tool);
>       struct addr_location al;
> -     const char *fmt;
> +     const char *fmt, *field_sep;
>  
>       if (machine__resolve(machine, &al, sample) < 0) {
>               fprintf(stderr, "problem processing %d event, skipping it.\n",
> @@ -186,60 +186,41 @@ dump_raw_samples(struct perf_tool *tool,
>       if (al.map != NULL)
>               al.map->dso->hit = 1;
>  
> -     if (mem->phys_addr) {
> -             if (symbol_conf.field_sep) {
> -                     fmt = "%d%s%d%s0x%"PRIx64"%s0x%"PRIx64"%s0x%016"PRIx64
> -                           "%s%"PRIu64"%s0x%"PRIx64"%s%s:%s\n";
> -             } else {
> -                     fmt = "%5d%s%5d%s0x%016"PRIx64"%s0x016%"PRIx64
> -                           "%s0x%016"PRIx64"%s%5"PRIu64"%s0x%06"PRIx64
> -                           "%s%s:%s\n";
> -                     symbol_conf.field_sep = " ";
> -             }
> -
> -             printf(fmt,
> -                     sample->pid,
> -                     symbol_conf.field_sep,
> -                     sample->tid,
> -                     symbol_conf.field_sep,
> -                     sample->ip,
> -                     symbol_conf.field_sep,
> -                     sample->addr,
> -                     symbol_conf.field_sep,
> -                     sample->phys_addr,
> -                     symbol_conf.field_sep,
> -                     sample->weight,
> -                     symbol_conf.field_sep,
> -                     sample->data_src,
> -                     symbol_conf.field_sep,
> -                     al.map ? (al.map->dso ? al.map->dso->long_name : "???") 
> : "???",
> -                     al.sym ? al.sym->name : "???");
> +     field_sep = symbol_conf.field_sep;

hum, what's the point of having field_sep?

> +     if (field_sep) {
> +             fmt = "%d%s%d%s0x%"PRIx64"%s0x%"PRIx64"%s";
>       } else {
> -             if (symbol_conf.field_sep) {
> -                     fmt = "%d%s%d%s0x%"PRIx64"%s0x%"PRIx64"%s%"PRIu64
> -                           "%s0x%"PRIx64"%s%s:%s\n";
> -             } else {
> -                     fmt = "%5d%s%5d%s0x%016"PRIx64"%s0x016%"PRIx64
> -                           "%s%5"PRIu64"%s0x%06"PRIx64"%s%s:%s\n";
> -                     symbol_conf.field_sep = " ";
> -             }
> +             fmt = "%5d%s%5d%s0x%016"PRIx64"%s0x016%"PRIx64"%s";
> +             symbol_conf.field_sep = " ";
> +     }
> +     printf(fmt,
> +             sample->pid,
> +             symbol_conf.field_sep,
> +             sample->tid,
> +             symbol_conf.field_sep,
> +             sample->ip,
> +             symbol_conf.field_sep,
> +             sample->addr,
> +             symbol_conf.field_sep);
>  
> -             printf(fmt,
> -                     sample->pid,
> -                     symbol_conf.field_sep,
> -                     sample->tid,
> -                     symbol_conf.field_sep,
> -                     sample->ip,
> -                     symbol_conf.field_sep,
> -                     sample->addr,
> -                     symbol_conf.field_sep,
> -                     sample->weight,
> -                     symbol_conf.field_sep,
> -                     sample->data_src,
> -                     symbol_conf.field_sep,
> -                     al.map ? (al.map->dso ? al.map->dso->long_name : "???") 
> : "???",
> -                     al.sym ? al.sym->name : "???");
> +     if (mem->phys_addr) {
> +             printf("0x%016"PRIx64"%s",
> +                     sample->phys_addr,
> +                     symbol_conf.field_sep);
>       }
> +
> +     if (field_sep)
> +             fmt = "%"PRIu64"%s0x%"PRIx64"%s%s:%s\n";
> +     else
> +             fmt = "%5"PRIu64"%s0x%06"PRIx64"%s%s:%s\n";
> +
> +     printf(fmt,
> +             sample->weight,
> +             symbol_conf.field_sep,
> +             sample->data_src,
> +             symbol_conf.field_sep,
> +             al.map ? (al.map->dso ? al.map->dso->long_name : "???") : "???",
> +             al.sym ? al.sym->name : "???");
>  out_put:
>       addr_location__put(&al);
>       return 0;
> @@ -287,10 +268,12 @@ static int report_raw_events(struct perf_mem *mem)
>       if (ret < 0)
>               goto out_delete;
>  
> +     printf("# PID, TID, IP, ADDR, ");
> +
>       if (mem->phys_addr)
> -             printf("# PID, TID, IP, ADDR, PHYS ADDR, LOCAL WEIGHT, DSRC, 
> SYMBOL\n");
> -     else
> -             printf("# PID, TID, IP, ADDR, LOCAL WEIGHT, DSRC, SYMBOL\n");
> +             printf("PHYS ADDR, ");
> +
> +     printf("LOCAL WEIGHT, DSRC, SYMBOL\n");

if phys addr is the only member, can't we just squeeze it via
  "%s", mem->phys_addr ? "PHYS ADDR" : "",
like I mentioned in the other reply? and same also above?

thanks,
jirka

>  
>       ret = perf_session__process_events(session);
>  
> -- 
> 2.17.1
> 

Reply via email to