On 4/9/21 12:49 PM, Daniel Axtens wrote:
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.bangoria@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.
Right. This helps in next patch.
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.
True. there is no issue in setting them as this test does all load/stores in userspace. So all events will be recorded irrespective of settings of exclude_{kernel,hv,guest}.
- 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),
Sure will fix it.
Apart from that, this seems good but I haven't checked in super fine detail just yet :)
Thanks for the review. -Ravi