From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1761751AbcIADmg (ORCPT ); Wed, 31 Aug 2016 23:42:36 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:42013 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1761740AbcIADmd (ORCPT ); Wed, 31 Aug 2016 23:42:33 -0400 X-IBM-Helo: d28dlp01.in.ibm.com X-IBM-MailFrom: maddy@linux.vnet.ibm.com X-IBM-RcptTo: linux-kernel@vger.kernel.org Subject: Re: [PATCH 04/13] perf/core: Extend perf_output_sample_regs() to include perf_arch_regs To: Nilay Vaish References: <1472418058-28659-1-git-send-email-maddy@linux.vnet.ibm.com> <1472418058-28659-5-git-send-email-maddy@linux.vnet.ibm.com> Cc: Linux Kernel list , linuxppc-dev@lists.ozlabs.org, Thomas Gleixner , Ingo Molnar , Peter Zijlstra , Jiri Olsa , Arnaldo Carvalho de Melo , Stephane Eranian , Russell King , Catalin Marinas , Will Deacon , Benjamin Herrenschmidt , Michael Ellerman , Sukadev Bhattiprolu From: Madhavan Srinivasan Date: Thu, 1 Sep 2016 09:12:22 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16090103-0012-0000-0000-000002FE3CFD X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16090103-0013-0000-0000-0000185FD719 Message-Id: <2e315e8f-bb95-b083-cc6c-c7aafd3d0efb@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-09-01_01:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1604210000 definitions=main-1609010035 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tuesday 30 August 2016 09:41 PM, Nilay Vaish wrote: > On 28 August 2016 at 16:00, Madhavan Srinivasan > 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 >