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

[Ingo Molnar <mingo@kernel.org>]

* Andi Kleen <andi@firstfloor.org> 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