Re: [PATCH 4/4] perf tools: Ask for ID PERF_SAMPLE_ info on all PERF_RECORD_ events

From: Ian Munsie
Date: Fri Dec 03 2010 - 01:05:19 EST


Excerpts from Arnaldo Carvalho de Melo's message of Fri Dec 03 06:39:32 +1100 2010:
> +-T::
> +--timestamp::
> + Sample timestamps. Use it with 'perf report -D' to see the timestamps,
> + for instance.
> +

I'm of two minds about allowing the user to request this. My patches
will enable this by default if sampling from multiple CPUs, which is the
norm unless no_inherit is specified, so this option will not have any
affect unless sampling from a single CPU as well.

> + } else if (err == EINVAL && sample_id_all_avail) {
> + /*
> + * Old kernel, no attr->sample_id_type_all field
> + */
> + sample_id_all_avail = false;
> + goto retry_sample_id;

Should we warn the user here? I guess the warning applies more to my
patches then these though, since I'm fixing a bug whereas you are just
adding a new feature, and samples will still have timestamps in your
patches either way.

> + printf("%Lu ", sample->time);

A little off topic, but I've noticed this in a few places throughout the
perf code - the L length modifier in printf is for a long double, not a
long long int (which is ll). The fact that this works with glibc is a
bug (and we've seen how little sympathy the glibc developers have for
programs not using glibc as per the standard). See the note in
CONFORMING TO in the printf man page.

Acked-by: Ian Munsie <imunsie@xxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/