lkml.org 
[lkml]   [2009]   [Dec]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH 2/9] perf: Remove pointless union that wraps the hw breakpoint fields
From
Date
On Sun, 2009-12-06 at 08:34 +0100, Frederic Weisbecker wrote:
> It stands to anonymize a structure, but structures can already
> anonymize by themselves.
>
> Reported-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: "K. Prasad" <prasad@linux.vnet.ibm.com>
> ---
> include/linux/perf_event.h | 14 ++++++--------
> 1 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index a61e4de..53230e9 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -215,14 +215,12 @@ struct perf_event_attr {
> __u32 wakeup_watermark; /* bytes before wakeup */
> };
>
> - union {
> - struct { /* Hardware breakpoint info */
> - __u64 bp_addr;
> - __u32 bp_type;
> - __u32 bp_len;
> - __u64 __bp_reserved_1;
> - __u64 __bp_reserved_2;
> - };
> + struct { /* Hardware breakpoint info */
> + __u64 bp_addr;
> + __u32 bp_type;
> + __u32 bp_len;
> + __u64 __bp_reserved_1;
> + __u64 __bp_reserved_2;
> };
>
> __u32 __reserved_2;

So I'm a bit puzzled by the need for
- that structure to begin with
- specialized __bp reserves

Furthermore, you still got the packing wrong, leading to different
structure layout on 32 and 64 bit platforms,..

How about?

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 89098e3..5595154 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -215,17 +215,12 @@ struct perf_event_attr {
__u32 wakeup_watermark; /* bytes before wakeup */
};

- struct { /* Hardware breakpoint info */
- __u64 bp_addr;
- __u32 bp_type;
- __u32 bp_len;
- __u64 __bp_reserved_1;
- __u64 __bp_reserved_2;
- };
-
__u32 __reserved_2;

- __u64 __reserved_3;
+ /* Hardware breakpoint info */
+ __u64 bp_addr;
+ __u32 bp_type;
+ __u32 bp_len;
};

/*



\
 
 \ /
  Last update: 2009-12-06 11:43    [W:0.056 / U:0.292 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site