Re: [PATCH 1/5] perf, x86: Add Haswell PEBS record support v3

From: Ingo Molnar
Date: Mon Feb 18 2013 - 03:20:13 EST



* Andi Kleen <andi@xxxxxxxxxxxxxx> wrote:

> --- a/arch/x86/kernel/cpu/perf_event_intel_ds.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_ds.c
> @@ -41,6 +41,22 @@ struct pebs_record_nhm {
> u64 status, dla, dse, lat;
> };
>
> +/*
> + * Same as pebs_record_nhm, with two additional fields.
> + */
> +struct pebs_record_hsw {
> + struct pebs_record_nhm nhm;
> + /*
> + * Real IP of the event. In the Intel documentation this
> + * is called eventingrip.
> + */
> + u64 ip_of_the_event;

Sigh.

In a prior review I objected to the original field's
'eventingrip' name:

http://permalink.gmane.org/gmane.linux.kernel/1434494

... because it's a misnomer on so many levels. (What is an
'eventing'? What 'grip' does it have on anything?
Whyisthenamemergedtogether?)

But, instead of just renaming it to something usable you renamed
it to an equally silly "ip_of_the_event" field name.

Just do a 'git grep of_the_' in the kernel source to see how
silly the name you picked is.

Why are you doing this passive-aggressive crap? Do you want to
drag out the review even more and delay the Haswell enabling
patches to v3.10 or beyond?

Thanks,

Ingo
--
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/