From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754139AbcBBJHB (ORCPT ); Tue, 2 Feb 2016 04:07:01 -0500 Received: from e32.co.us.ibm.com ([32.97.110.150]:53752 "EHLO e32.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753962AbcBBJG5 (ORCPT ); Tue, 2 Feb 2016 04:06:57 -0500 X-IBM-Helo: d03dlp01.boulder.ibm.com X-IBM-MailFrom: ravi.bangoria@linux.vnet.ibm.com X-IBM-RcptTo: linux-kernel@vger.kernel.org From: Ravi Bangoria Subject: Re: [PATCH v2 2/3] perf kvm: enable record|report feature on powerpc To: Arnaldo Carvalho de Melo References: <1453442292-7790-1-git-send-email-ravi.bangoria@linux.vnet.ibm.com> <1453442292-7790-3-git-send-email-ravi.bangoria@linux.vnet.ibm.com> <20160201210605.GE20817@kernel.org> Cc: mingo@kernel.org, peterz@infradead.org, linux-kernel@vger.kernel.org, hemant@linux.vnet.ibm.com, naveen.n.rao@linux.vnet.ibm.com Message-ID: <56B071A8.4060402@linux.vnet.ibm.com> Date: Tue, 2 Feb 2016 14:36:48 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <20160201210605.GE20817@kernel.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16020209-0005-0000-0000-00001BFD9D6C Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org HI acme, On Tuesday 02 February 2016 02:36 AM, Arnaldo Carvalho de Melo wrote: > Em Fri, Jan 22, 2016 at 11:28:11AM +0530, Ravi Bangoria escreveu: >> + return event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK; >> +} > This hunk and the next should be on the previous patch, that is not even > compiling... > > You have to compile patch by patch, we can't just test at the end of a > patchkit like this, this destroys bisection ;-\ Didn't aware about that. Will take care of compiling each patch separately next time onwards. > Also you first need to put in place a way to override how to obtain the > cpumode, then you should use it. > > Also this mode doesn't look feasible at all, think about processing > perf.data files generated in !powerpc systems being analysed in a > powerpc system. This has to be dependend on the architecture of the > machine where the perf.data file was recorded, not on the archictecture > of the machine the binary was built for. Valid point. I'll re-think about approach in this case. > It is only when you do live analysis, like with 'perf trace' and 'perf > top' that its guaranteed to be all on the same machine. > > IIRC in one of the patches in this series you introduce and use a > library function on the same patch, please break it into two patches as > well, lemme see what is the name... > > Yeah, it is also in this patch: > > perf_evlist__arch_add_default(struct perf_evlist *evlist) > > Please add this in a separate patch, stating in the changeset comment > why it is needed and how architectures can override it. Will do that. Thanks for reviewing. Regards, Ravi