Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1755836AbcCBQW1 (ORCPT <rfc822;w@1wt.eu>);
	Wed, 2 Mar 2016 11:22:27 -0500
Received: from mail.kernel.org ([198.145.29.136]:37551 "EHLO mail.kernel.org"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753373AbcCBQW0 (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Wed, 2 Mar 2016 11:22:26 -0500
Date: Wed, 2 Mar 2016 13:22:20 -0300
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Cc: linux-kernel@vger.kernel.org, hemant@linux.vnet.ibm.com,
        naveen.n.rao@linux.vnet.ibm.com
Subject: Re: [RFC 4/4] perf kvm: Fix output fields instead of 'trace' for
 perf kvm report on powerpc
Message-ID: <20160302162220.GS3604@kernel.org>
References: <1456304865-15131-1-git-send-email-ravi.bangoria@linux.vnet.ibm.com>
 <1456304865-15131-5-git-send-email-ravi.bangoria@linux.vnet.ibm.com>
 <20160302142556.GR3604@kernel.org>
 <56D70AE8.1080501@linux.vnet.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <56D70AE8.1080501@linux.vnet.ibm.com>
X-Url: http://acmel.wordpress.com
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1736
Lines: 58

Em Wed, Mar 02, 2016 at 09:16:48PM +0530, Ravi Bangoria escreveu:
> Thanks Arnaldo,
> 
> Please find my comments.
> 
> On Wednesday 02 March 2016 07:55 PM, Arnaldo Carvalho de Melo wrote:
> >Em Wed, Feb 24, 2016 at 02:37:45PM +0530, Ravi Bangoria escreveu:
> >>  		use_browser = 0;
> >>+	if (!field_order &&
> >>+	    is_perf_data_reorded_on_ppc(session->evlist) &&
> >>+	    perf_guest_only())
> >>+		field_order = "overhead,comm,dso,sym";
> >>+
> >Can you please do it as:
> >
> >__weak void arch__override_field_order(struct perf_evlist *evlist, const char **field_order)
> >{
> >}
> 
> So you mean like this - Just implement only weak function and move code into
> it?
> ie. No strong implementation at this point of time.
> 
> Like,
> 
> __weak void arch__override_field_order(struct perf_evlist *evlist, const
> char **f_order)
> {
>     if (!field_order &&
>         is_perf_data_reorded_on_ppc(session->evlist) &&

Oh, I see, ugh, when running on x86_64 we wouldn't use this, so we need
to have per arch default field orders, now I have to recall why is it
that we need this per-arch field order :-\

- Arnaldo

>         perf_guest_only())
>             *field_order = "overhead,comm,dso,sym";
> }
> 
> Then I can do that.
> 
> But if you are proposing to implement a strong function and move this code
> into in, then we won't be able to enable cross arch reporting.
> 
> >
> >This way we don't see any arch specific stuff in the tool, also I
> >haven't seen any doc update, are you sure nothing needs to be added to
> >tools/perf/Documentaton/ for any of these patches?
> >
> >I think this needs to be documented further, probably in
> >tools/perf/design.txt too?
> 
> Yes, I'll do this in next version.
> 
> Regards,
> Ravi