Re: [patch] HWBKPT: Make bp_len type to u64 generic across the arch

From: Frederic Weisbecker
Date: Sat Jan 30 2010 - 13:38:47 EST


On Sat, Jan 30, 2010 at 10:25:18AM +0530, Mahesh Salgaonkar wrote:
>
> Change 'bp_len' type to __u64 to make it work across the arch.
> The s390 architecture watch point length can be upto 2^64.
>
> reference:
> http://lkml.org/lkml/2010/1/25/212
>
> Based on commit 6aa41f8b01301199af6c9febb24f3c1f5a0bc9d5
>
> Signed-off-by: Mahesh Salgaonkar <mahesh@xxxxxxxxxxxxxxxxxx>
> ---
>
> include/linux/hw_breakpoint.h | 2 +-
> include/linux/perf_event.h | 6 ++----
> kernel/hw_breakpoint.c | 2 +-
> kernel/perf_event.c | 2 +-
> 4 files changed, 5 insertions(+), 7 deletions(-)
>
>
> diff --git a/include/linux/hw_breakpoint.h b/include/linux/hw_breakpoint.h
> index 41235c9..76e7427 100644
> --- a/include/linux/hw_breakpoint.h
> +++ b/include/linux/hw_breakpoint.h
> @@ -44,7 +44,7 @@ static inline int hw_breakpoint_type(struct perf_event *bp)
> return bp->attr.bp_type;
> }
>
> -static inline int hw_breakpoint_len(struct perf_event *bp)
> +static inline unsigned long hw_breakpoint_len(struct perf_event *bp)
> {
> return bp->attr.bp_len;
> }



This should return a u64, or gcc will warn us about loosing
informations in 32 bits arch?



> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 1438463..30c78bd 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -211,11 +211,9 @@ struct perf_event_attr {
> __u32 wakeup_watermark; /* bytes before wakeup */
> };
>
> - __u32 __reserved_2;
> -
> - __u64 bp_addr;
> __u32 bp_type;
> - __u32 bp_len;
> + __u64 bp_addr;
> + __u64 bp_len;
> };



Peter, what do you think about this new layout?
Putting the bp_type right after the wakeup_* fields
is going to remove the padding difference between
64 and 32 archs. That looks better than the __reserved_2
we had.

If this patch can make it for .33, it would be nice.



>
> /*
> diff --git a/kernel/hw_breakpoint.c b/kernel/hw_breakpoint.c
> index 50dbd59..a1f511f 100644
> --- a/kernel/hw_breakpoint.c
> +++ b/kernel/hw_breakpoint.c
> @@ -324,8 +324,8 @@ EXPORT_SYMBOL_GPL(register_user_hw_breakpoint);
> int modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *attr)
> {
> u64 old_addr = bp->attr.bp_addr;
> + u64 old_len = bp->attr.bp_len;
> int old_type = bp->attr.bp_type;
> - int old_len = bp->attr.bp_len;
> int err = 0;
>
> perf_event_disable(bp);
> diff --git a/kernel/perf_event.c b/kernel/perf_event.c
> index 8f8f7aa..1473cc9 100644
> --- a/kernel/perf_event.c
> +++ b/kernel/perf_event.c
> @@ -4720,7 +4720,7 @@ static int perf_copy_attr(struct perf_event_attr __user *uattr,
> if (attr->type >= PERF_TYPE_MAX)
> return -EINVAL;
>
> - if (attr->__reserved_1 || attr->__reserved_2)
> + if (attr->__reserved_1)
> return -EINVAL;
>
> if (attr->sample_type & ~(PERF_SAMPLE_MAX-1))
>


Acked-by: Frederic Weisbecker <fweisbec@xxxxxxxxx>

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