Hi Ravi,
> perf-hwbreak selftest opens hw-breakpoint event at multiple places for
> which it has same code repeated. Coalesce that code into a function.
>
> Signed-off-by: Ravi Bangoria <ravi.bango...@linux.ibm.com>
> ---
> .../selftests/powerpc/ptrace/perf-hwbreak.c | 78 +++++++++----------
This doesn't simplify things very much for the code as it stands now,
but I think your next patch adds a bunch of calls to these functions, so
I agree that it makes sense to consolidate them now.
> 1 file changed, 38 insertions(+), 40 deletions(-)
>
> diff --git a/tools/testing/selftests/powerpc/ptrace/perf-hwbreak.c
> b/tools/testing/selftests/powerpc/ptrace/perf-hwbreak.c
> index c1f324afdbf3..bde475341c8a 100644
> --- a/tools/testing/selftests/powerpc/ptrace/perf-hwbreak.c
> +++ b/tools/testing/selftests/powerpc/ptrace/perf-hwbreak.c
> @@ -34,28 +34,46 @@
>
> #define DAWR_LENGTH_MAX ((0x3f + 1) * 8)
>
> -static inline int sys_perf_event_open(struct perf_event_attr *attr, pid_t
> pid,
> - int cpu, int group_fd,
> - unsigned long flags)
> +static void perf_event_attr_set(struct perf_event_attr *attr,
> + __u32 type, __u64 addr, __u64 len,
> + bool exclude_user)
> {
> - attr->size = sizeof(*attr);
> - return syscall(__NR_perf_event_open, attr, pid, cpu, group_fd, flags);
> + memset(attr, 0, sizeof(struct perf_event_attr));
> + attr->type = PERF_TYPE_BREAKPOINT;
> + attr->size = sizeof(struct perf_event_attr);
> + attr->bp_type = type;
> + attr->bp_addr = addr;
> + attr->bp_len = len;
> + attr->exclude_kernel = 1;
> + attr->exclude_hv = 1;
> + attr->exclude_guest = 1;
Only 1 of the calls to perf sets exclude_{kernel,hv,guest} - I assume
there's no issue with setting them always but I wanted to check.
> + attr->exclude_user = exclude_user;
> + attr->disabled = 1;
> }
>
> - /* setup counters */
> - memset(&attr, 0, sizeof(attr));
> - attr.disabled = 1;
> - attr.type = PERF_TYPE_BREAKPOINT;
> - attr.bp_type = readwriteflag;
> - attr.bp_addr = (__u64)ptr;
> - attr.bp_len = sizeof(int);
> - if (arraytest)
> - attr.bp_len = DAWR_LENGTH_MAX;
> - attr.exclude_user = exclude_user;
> - break_fd = sys_perf_event_open(&attr, 0, -1, -1, 0);
> + break_fd = perf_process_event_open_exclude_user(readwriteflag,
> (__u64)ptr,
> + arraytest ? DAWR_LENGTH_MAX : sizeof(int),
> + exclude_user);
checkpatch doesn't like this very much:
CHECK: Alignment should match open parenthesis
#103: FILE: tools/testing/selftests/powerpc/ptrace/perf-hwbreak.c:115:
+ break_fd = perf_process_event_open_exclude_user(readwriteflag,
(__u64)ptr,
+ arraytest ? DAWR_LENGTH_MAX : sizeof(int),
Apart from that, this seems good but I haven't checked in super fine
detail just yet :)
Kind regards,
Daniel
