| Message ID | 1472418058-28659-5-git-send-email-maddy@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On 28 August 2016 at 16:00, Madhavan Srinivasan <maddy@linux.vnet.ibm.com> wrote: > diff --git a/kernel/events/core.c b/kernel/events/core.c > index 274288819829..e16bf4d057d1 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -5371,16 +5371,24 @@ u64 __attribute__((weak)) perf_arch_reg_value(struct perf_arch_regs *regs, > > static void > perf_output_sample_regs(struct perf_output_handle *handle, > - struct pt_regs *regs, u64 mask) > + struct perf_regs *regs, u64 mask) > { > int bit; > DECLARE_BITMAP(_mask, 64); > + u64 arch_regs_mask = regs->arch_regs_mask; > > bitmap_from_u64(_mask, mask); > for_each_set_bit(bit, _mask, sizeof(mask) * BITS_PER_BYTE) { > u64 val; > > - val = perf_reg_value(regs, bit); > + val = perf_reg_value(regs->regs, bit); > + perf_output_put(handle, val); > + } > + > + bitmap_from_u64(_mask, arch_regs_mask); > + for_each_set_bit(bit, _mask, sizeof(mask) * BITS_PER_BYTE) { > + u64 val; > + val = perf_arch_reg_value(regs->arch_regs, bit); > perf_output_put(handle, val); > } > } > @@ -5792,7 +5800,7 @@ void perf_output_sample(struct perf_output_handle *handle, > if (abi) { > u64 mask = event->attr.sample_regs_user; > perf_output_sample_regs(handle, > - data->regs_user.regs, > + &data->regs_user, > mask); > } > } > @@ -5827,7 +5835,7 @@ void perf_output_sample(struct perf_output_handle *handle, > u64 mask = event->attr.sample_regs_intr; > > perf_output_sample_regs(handle, > - data->regs_intr.regs, > + &data->regs_intr, > mask); > } > } > -- > 2.7.4 > I would like to suggest a slightly different version. Would it make more sense to have something like following: @@ -5792,7 +5800,7 @@ void perf_output_sample(struct perf_output_handle *handle, if (abi) { u64 mask = event->attr.sample_regs_user; perf_output_sample_regs(handle, data->regs_user.regs, mask); } + + if (arch_regs_mask) { + perf_output_pmu_regs(handle, data->regs_users.arch_regs, arch_regs_mask); + } } Somehow I don't like outputting the two sets of registers through the same function call. -- Nilay
On Tuesday 30 August 2016 09:41 PM, Nilay Vaish wrote: > On 28 August 2016 at 16:00, Madhavan Srinivasan > <maddy@linux.vnet.ibm.com> wrote: >> diff --git a/kernel/events/core.c b/kernel/events/core.c >> index 274288819829..e16bf4d057d1 100644 >> --- a/kernel/events/core.c >> +++ b/kernel/events/core.c >> @@ -5371,16 +5371,24 @@ u64 __attribute__((weak)) perf_arch_reg_value(struct perf_arch_regs *regs, >> >> static void >> perf_output_sample_regs(struct perf_output_handle *handle, >> - struct pt_regs *regs, u64 mask) >> + struct perf_regs *regs, u64 mask) >> { >> int bit; >> DECLARE_BITMAP(_mask, 64); >> + u64 arch_regs_mask = regs->arch_regs_mask; >> >> bitmap_from_u64(_mask, mask); >> for_each_set_bit(bit, _mask, sizeof(mask) * BITS_PER_BYTE) { >> u64 val; >> >> - val = perf_reg_value(regs, bit); >> + val = perf_reg_value(regs->regs, bit); >> + perf_output_put(handle, val); >> + } >> + >> + bitmap_from_u64(_mask, arch_regs_mask); >> + for_each_set_bit(bit, _mask, sizeof(mask) * BITS_PER_BYTE) { >> + u64 val; >> + val = perf_arch_reg_value(regs->arch_regs, bit); >> perf_output_put(handle, val); >> } >> } >> @@ -5792,7 +5800,7 @@ void perf_output_sample(struct perf_output_handle *handle, >> if (abi) { >> u64 mask = event->attr.sample_regs_user; >> perf_output_sample_regs(handle, >> - data->regs_user.regs, >> + &data->regs_user, >> mask); >> } >> } >> @@ -5827,7 +5835,7 @@ void perf_output_sample(struct perf_output_handle *handle, >> u64 mask = event->attr.sample_regs_intr; >> >> perf_output_sample_regs(handle, >> - data->regs_intr.regs, >> + &data->regs_intr, >> mask); >> } >> } >> -- >> 2.7.4 >> > I would like to suggest a slightly different version. Would it make > more sense to have something like following: I agree we are outputting two different structures, but since we use the INTR_REG infrastructure to dump the arch pmu registers, I preferred to extend perf_output_sample_regs. But I guess I can break it up. Maddy > > @@ -5792,7 +5800,7 @@ void perf_output_sample(struct perf_output_handle *handle, > if (abi) { > u64 mask = event->attr.sample_regs_user; > perf_output_sample_regs(handle, > data->regs_user.regs, > mask); > } > + > + if (arch_regs_mask) { > + perf_output_pmu_regs(handle, > data->regs_users.arch_regs, arch_regs_mask); > + } > } > > > Somehow I don't like outputting the two sets of registers through the > same function call. > > -- > Nilay >
diff --git a/kernel/events/core.c b/kernel/events/core.c index 274288819829..e16bf4d057d1 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -5371,16 +5371,24 @@ u64 __attribute__((weak)) perf_arch_reg_value(struct perf_arch_regs *regs, static void perf_output_sample_regs(struct perf_output_handle *handle, - struct pt_regs *regs, u64 mask) + struct perf_regs *regs, u64 mask) { int bit; DECLARE_BITMAP(_mask, 64); + u64 arch_regs_mask = regs->arch_regs_mask; bitmap_from_u64(_mask, mask); for_each_set_bit(bit, _mask, sizeof(mask) * BITS_PER_BYTE) { u64 val; - val = perf_reg_value(regs, bit); + val = perf_reg_value(regs->regs, bit); + perf_output_put(handle, val); + } + + bitmap_from_u64(_mask, arch_regs_mask); + for_each_set_bit(bit, _mask, sizeof(mask) * BITS_PER_BYTE) { + u64 val; + val = perf_arch_reg_value(regs->arch_regs, bit); perf_output_put(handle, val); } } @@ -5792,7 +5800,7 @@ void perf_output_sample(struct perf_output_handle *handle, if (abi) { u64 mask = event->attr.sample_regs_user; perf_output_sample_regs(handle, - data->regs_user.regs, + &data->regs_user, mask); } } @@ -5827,7 +5835,7 @@ void perf_output_sample(struct perf_output_handle *handle, u64 mask = event->attr.sample_regs_intr; perf_output_sample_regs(handle, - data->regs_intr.regs, + &data->regs_intr, mask); } }
Extend perf_output_sample_regs() to take in perf_regs structure as a parameter instead of pt_regs. Add code to check for arch_regs_mask and dump the arch registers to the output sample. Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Jiri Olsa <jolsa@kernel.org> Cc: Arnaldo Carvalho de Melo <acme@kernel.org> Cc: Stephane Eranian <eranian@gmail.com> Cc: Russell King <linux@arm.linux.org.uk> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <will.deacon@arm.com> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> Cc: Michael Ellerman <mpe@ellerman.id.au> Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com> --- kernel/events/core.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)