All of lore.kernel.org
 help / color / mirror / Atom feed
From: Robert Richter <robert.richter@amd.com>
To: Paul Mundt <lethal@linux-sh.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Matt Fleming <matt@console-pimps.org>,
	Will Deacon <will.deacon@arm.com>,
	Russell King <linux@arm.linux.org.uk>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-sh@vger.kernel.org" <linux-sh@vger.kernel.org>,
	Ingo Molnar <mingo@elte.hu>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	"linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Deng-Cheng Zhu <dengcheng.zhu@gmail.com>,
	Grant Likely <grant.likely@secretlab.ca>
Subject: Re: [PATCH 2/7] perf: New helper function for pmu name
Date: Wed, 6 Oct 2010 15:18:25 +0200	[thread overview]
Message-ID: <20101006131825.GO13563@erda.amd.com> (raw)
In-Reply-To: <20101006123950.GA29118@linux-sh.org>

(cc'ing Peter)

On 06.10.10 08:39:50, Paul Mundt wrote:
> On Wed, Oct 06, 2010 at 02:27:36PM +0200, Robert Richter wrote:
> > On 04.10.10 16:44:20, Matt Fleming wrote:
> > > Introduce perf_pmu_name() helper function that returns the name of the
> > > pmu. This gives us a generic way to get the name of a pmu regardless of
> > > how an architecture identifies it internally, e.g. ARM uses an id
> > > whereas SH currently uses a string.
> > 
> > I rather want use here the solution we discussed earlier, simply
> > including <asm/perf_event.h> and then access sh_pmu->name directly
> > from oprofile.
> > 
> No. Exposing sh_pmu generically is unacceptable. This is already

I don't want to be the perf_pmu_name() discussion a show stopper for
this patch set. We don't have a generic perf interface available now
that allows us to detect the pmu type for sh cpus. I also don't see a
name string as a final solution for pmu detection. So it will be more
than just adding perf_pmu_name().

Until then, as we currently would only need the name for SH, I don't
see something wrong here to include architectural interfaces from
<asm/perf_event.h> directly by architectural code until a general
solution is available.  If you run a 'git grep include..asm/perf' this
is common for other architectures.

> centrally managed through perf, and if oprofile needs any additional
> information then it needs to get that through the perf layer. We already
> have the situation that effectively every architecture with perf support
> implements a name string already, so making this part of the perf API
> hardly seems like that big of a stretch. If the perf people are violently
> opposed to this, then of course we can look at alternatives.

I am not against adding the pmu name to the perf API. But the oprofile
cpu_type strings are oprofile centric esp. for the userland. So these
strings will remain part of oprofile. Also I don't think we want to
polute the perf pmu names with it.

> sh_pmu is created for use solely by the perf events code, we are not
> going to have oprofile poking around in data structures it has no place
> knowing anything about when 99% of everything else it is doing is already
> abstracted cleanly through the perf events interfaces. Likewise for
> special oprofile-specific APIs.

So, I am also fine with implementing a generic perf_pmu_name() for sh
and then derive the oprofile cpu_type string from it in the oprofile
code.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center


WARNING: multiple messages have this Message-ID (diff)
From: Robert Richter <robert.richter@amd.com>
To: Paul Mundt <lethal@linux-sh.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Matt Fleming <matt@console-pimps.org>,
	Will Deacon <will.deacon@arm.com>,
	Russell King <linux@arm.linux.org.uk>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-sh@vger.kernel.org" <linux-sh@vger.kernel.org>,
	Ingo Molnar <mingo@elte.hu>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	"linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Deng-Cheng Zhu <dengcheng.zhu@gmail.com>,
	Grant Likely <grant.likely@secretlab.ca>
Subject: Re: [PATCH 2/7] perf: New helper function for pmu name
Date: Wed, 06 Oct 2010 13:18:25 +0000	[thread overview]
Message-ID: <20101006131825.GO13563@erda.amd.com> (raw)
In-Reply-To: <20101006123950.GA29118@linux-sh.org>

(cc'ing Peter)

On 06.10.10 08:39:50, Paul Mundt wrote:
> On Wed, Oct 06, 2010 at 02:27:36PM +0200, Robert Richter wrote:
> > On 04.10.10 16:44:20, Matt Fleming wrote:
> > > Introduce perf_pmu_name() helper function that returns the name of the
> > > pmu. This gives us a generic way to get the name of a pmu regardless of
> > > how an architecture identifies it internally, e.g. ARM uses an id
> > > whereas SH currently uses a string.
> > 
> > I rather want use here the solution we discussed earlier, simply
> > including <asm/perf_event.h> and then access sh_pmu->name directly
> > from oprofile.
> > 
> No. Exposing sh_pmu generically is unacceptable. This is already

I don't want to be the perf_pmu_name() discussion a show stopper for
this patch set. We don't have a generic perf interface available now
that allows us to detect the pmu type for sh cpus. I also don't see a
name string as a final solution for pmu detection. So it will be more
than just adding perf_pmu_name().

Until then, as we currently would only need the name for SH, I don't
see something wrong here to include architectural interfaces from
<asm/perf_event.h> directly by architectural code until a general
solution is available.  If you run a 'git grep include..asm/perf' this
is common for other architectures.

> centrally managed through perf, and if oprofile needs any additional
> information then it needs to get that through the perf layer. We already
> have the situation that effectively every architecture with perf support
> implements a name string already, so making this part of the perf API
> hardly seems like that big of a stretch. If the perf people are violently
> opposed to this, then of course we can look at alternatives.

I am not against adding the pmu name to the perf API. But the oprofile
cpu_type strings are oprofile centric esp. for the userland. So these
strings will remain part of oprofile. Also I don't think we want to
polute the perf pmu names with it.

> sh_pmu is created for use solely by the perf events code, we are not
> going to have oprofile poking around in data structures it has no place
> knowing anything about when 99% of everything else it is doing is already
> abstracted cleanly through the perf events interfaces. Likewise for
> special oprofile-specific APIs.

So, I am also fine with implementing a generic perf_pmu_name() for sh
and then derive the oprofile cpu_type string from it in the oprofile
code.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center


WARNING: multiple messages have this Message-ID (diff)
From: robert.richter@amd.com (Robert Richter)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/7] perf: New helper function for pmu name
Date: Wed, 6 Oct 2010 15:18:25 +0200	[thread overview]
Message-ID: <20101006131825.GO13563@erda.amd.com> (raw)
In-Reply-To: <20101006123950.GA29118@linux-sh.org>

(cc'ing Peter)

On 06.10.10 08:39:50, Paul Mundt wrote:
> On Wed, Oct 06, 2010 at 02:27:36PM +0200, Robert Richter wrote:
> > On 04.10.10 16:44:20, Matt Fleming wrote:
> > > Introduce perf_pmu_name() helper function that returns the name of the
> > > pmu. This gives us a generic way to get the name of a pmu regardless of
> > > how an architecture identifies it internally, e.g. ARM uses an id
> > > whereas SH currently uses a string.
> > 
> > I rather want use here the solution we discussed earlier, simply
> > including <asm/perf_event.h> and then access sh_pmu->name directly
> > from oprofile.
> > 
> No. Exposing sh_pmu generically is unacceptable. This is already

I don't want to be the perf_pmu_name() discussion a show stopper for
this patch set. We don't have a generic perf interface available now
that allows us to detect the pmu type for sh cpus. I also don't see a
name string as a final solution for pmu detection. So it will be more
than just adding perf_pmu_name().

Until then, as we currently would only need the name for SH, I don't
see something wrong here to include architectural interfaces from
<asm/perf_event.h> directly by architectural code until a general
solution is available.  If you run a 'git grep include..asm/perf' this
is common for other architectures.

> centrally managed through perf, and if oprofile needs any additional
> information then it needs to get that through the perf layer. We already
> have the situation that effectively every architecture with perf support
> implements a name string already, so making this part of the perf API
> hardly seems like that big of a stretch. If the perf people are violently
> opposed to this, then of course we can look at alternatives.

I am not against adding the pmu name to the perf API. But the oprofile
cpu_type strings are oprofile centric esp. for the userland. So these
strings will remain part of oprofile. Also I don't think we want to
polute the perf pmu names with it.

> sh_pmu is created for use solely by the perf events code, we are not
> going to have oprofile poking around in data structures it has no place
> knowing anything about when 99% of everything else it is doing is already
> abstracted cleanly through the perf events interfaces. Likewise for
> special oprofile-specific APIs.

So, I am also fine with implementing a generic perf_pmu_name() for sh
and then derive the oprofile cpu_type string from it in the oprofile
code.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

  reply	other threads:[~2010-10-06 13:18 UTC|newest]

Thread overview: 150+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-04 20:44 [PATCH V4 0/7] Generalise ARM perf-events backend for oprofile Matt Fleming
2010-10-04 20:44 ` Matt Fleming
2010-10-04 20:44 ` Matt Fleming
2010-10-04 20:44 ` [PATCH 1/7] perf: Add helper function to return number of counters Matt Fleming
2010-10-04 20:44   ` Matt Fleming
2010-10-04 20:44   ` Matt Fleming
2010-10-05  8:15   ` Paul Mundt
2010-10-05  8:15     ` Paul Mundt
2010-10-05  8:15     ` Paul Mundt
2010-10-06 12:14   ` Robert Richter
2010-10-06 12:14     ` Robert Richter
2010-10-06 12:14     ` Robert Richter
2010-10-06 12:14     ` [PATCH 1/7] perf: Add helper function to return number of Robert Richter
2010-10-06 12:14     ` [PATCH 1/7] perf: Add helper function to return number of counters Robert Richter
2010-10-06 12:35     ` Robert Richter
2010-10-06 12:35       ` Robert Richter
2010-10-06 12:35       ` [PATCH 1/7] perf: Add helper function to return number of Robert Richter
2010-10-06 13:41     ` [PATCH 1/7] perf: Add helper function to return number of counters Peter Zijlstra
2010-10-06 13:41       ` Peter Zijlstra
2010-10-06 13:41       ` [PATCH 1/7] perf: Add helper function to return number of Peter Zijlstra
2010-10-04 20:44 ` [PATCH 2/7] perf: New helper function for pmu name Matt Fleming
2010-10-04 20:44   ` Matt Fleming
2010-10-04 20:44   ` Matt Fleming
2010-10-05  8:10   ` Paul Mundt
2010-10-05  8:10     ` Paul Mundt
2010-10-05  8:10     ` Paul Mundt
2010-10-05  8:10     ` Paul Mundt
2010-10-06 12:27   ` Robert Richter
2010-10-06 12:27     ` Robert Richter
2010-10-06 12:27     ` Robert Richter
2010-10-06 12:39     ` Paul Mundt
2010-10-06 12:39       ` Paul Mundt
2010-10-06 12:39       ` Paul Mundt
2010-10-06 13:18       ` Robert Richter [this message]
2010-10-06 13:18         ` Robert Richter
2010-10-06 13:18         ` Robert Richter
2010-10-06 13:30         ` Paul Mundt
2010-10-06 13:30           ` Paul Mundt
2010-10-06 13:30           ` Paul Mundt
2010-10-06 14:13           ` Paul Mundt
2010-10-06 14:13             ` Paul Mundt
2010-10-06 14:13             ` Paul Mundt
2010-10-06 15:37             ` Robert Richter
2010-10-06 15:37               ` Robert Richter
2010-10-06 15:37               ` Robert Richter
2010-10-06 15:46               ` Paul Mundt
2010-10-06 15:46                 ` Paul Mundt
2010-10-06 15:46                 ` Paul Mundt
2010-10-06 15:50           ` Robert Richter
2010-10-06 15:50             ` Robert Richter
2010-10-06 15:50             ` Robert Richter
2010-10-06 15:57             ` Paul Mundt
2010-10-06 15:57               ` Paul Mundt
2010-10-06 15:57               ` Paul Mundt
2010-10-06 18:15   ` Robert Richter
2010-10-06 18:15     ` Robert Richter
2010-10-06 18:15     ` Robert Richter
2010-10-06 18:15     ` Robert Richter
2010-10-06 18:15     ` Robert Richter
2010-10-06 18:38     ` Paul Mundt
2010-10-06 18:38       ` Paul Mundt
2010-10-06 18:38       ` Paul Mundt
2010-10-04 20:44 ` [PATCH 3/7] ARM: oprofile: Rename op_arm to oprofile_perf Matt Fleming
2010-10-04 20:44   ` Matt Fleming
2010-10-04 20:44   ` Matt Fleming
2010-10-06 12:41   ` Robert Richter
2010-10-06 12:41     ` Robert Richter
2010-10-06 12:41     ` Robert Richter
2010-10-04 20:44 ` [PATCH 4/7] ARM: oprofile: Move non-ARM code into separate init/exit Matt Fleming
2010-10-04 20:44   ` Matt Fleming
2010-10-04 20:44   ` Matt Fleming
2010-10-06 13:33   ` Robert Richter
2010-10-06 13:33     ` Robert Richter
2010-10-06 13:33     ` [PATCH 4/7] ARM: oprofile: Move non-ARM code into separate Robert Richter
2010-10-06 13:41     ` [PATCH 4/7] ARM: oprofile: Move non-ARM code into separate init/exit Paul Mundt
2010-10-06 13:41       ` Paul Mundt
2010-10-06 13:41       ` Paul Mundt
2010-10-06 14:49     ` Robert Richter
2010-10-06 14:49       ` Robert Richter
2010-10-06 14:49       ` [PATCH 4/7] ARM: oprofile: Move non-ARM code into separate Robert Richter
2010-10-06 14:53       ` [PATCH 4/7] ARM: oprofile: Move non-ARM code into separate init/exit Paul Mundt
2010-10-06 14:53         ` Paul Mundt
2010-10-06 14:53         ` Paul Mundt
2010-10-06 14:59         ` Robert Richter
2010-10-06 14:59           ` Robert Richter
2010-10-06 14:59           ` [PATCH 4/7] ARM: oprofile: Move non-ARM code into separate Robert Richter
2010-10-06 15:00           ` [PATCH 4/7] ARM: oprofile: Move non-ARM code into separate init/exit Paul Mundt
2010-10-06 15:00             ` Paul Mundt
2010-10-06 15:00             ` Paul Mundt
2010-10-06 18:23           ` Grant Likely
2010-10-06 18:23             ` Grant Likely
2010-10-06 18:23             ` Grant Likely
2010-10-06 18:44             ` Robert Richter
2010-10-06 18:44               ` Robert Richter
2010-10-06 18:44               ` [PATCH 4/7] ARM: oprofile: Move non-ARM code into separate Robert Richter
2010-10-06 18:50               ` [PATCH 4/7] ARM: oprofile: Move non-ARM code into separate init/exit Paul Mundt
2010-10-06 18:50                 ` Paul Mundt
2010-10-06 18:50                 ` Paul Mundt
2010-10-06 18:58               ` Grant Likely
2010-10-06 18:58                 ` Grant Likely
2010-10-06 18:58                 ` Grant Likely
2010-10-06 19:22                 ` Robert Richter
2010-10-06 19:22                   ` Robert Richter
2010-10-06 19:22                   ` [PATCH 4/7] ARM: oprofile: Move non-ARM code into separate Robert Richter
2010-10-04 20:44 ` [PATCH 5/7] oprofile: Abstract the perf-events backend Matt Fleming
2010-10-04 20:44   ` Matt Fleming
2010-10-04 20:44   ` Matt Fleming
2010-10-06 18:34   ` Robert Richter
2010-10-06 18:34     ` Robert Richter
2010-10-06 18:34     ` Robert Richter
2010-10-04 20:44 ` [PATCH 6/7] sh: oprofile: Use perf-events oprofile backend Matt Fleming
2010-10-04 20:44   ` Matt Fleming
2010-10-04 20:44   ` Matt Fleming
2010-10-05  8:08   ` Paul Mundt
2010-10-05  8:08     ` Paul Mundt
2010-10-05  8:08     ` Paul Mundt
2010-10-05  8:08     ` Paul Mundt
2010-10-06 18:35   ` Robert Richter
2010-10-06 18:35     ` Robert Richter
2010-10-06 18:35     ` Robert Richter
2010-10-06 19:45     ` Matt Fleming
2010-10-06 19:45       ` Matt Fleming
2010-10-06 19:45       ` Matt Fleming
2010-10-06 21:03       ` Paul Mundt
2010-10-06 21:03         ` Paul Mundt
2010-10-06 21:03         ` Paul Mundt
2010-10-04 20:44 ` [PATCH 7/7] sh: Annotate oprofile_arch_exit() with __exit marker Matt Fleming
2010-10-04 20:44   ` Matt Fleming
2010-10-04 20:44   ` Matt Fleming
2010-10-05  8:16   ` Paul Mundt
2010-10-05  8:16     ` Paul Mundt
2010-10-05  8:16     ` Paul Mundt
2010-10-06 18:37   ` Robert Richter
2010-10-06 18:37     ` Robert Richter
2010-10-06 18:37     ` [PATCH 7/7] sh: Annotate oprofile_arch_exit() with __exit Robert Richter
  -- strict thread matches above, loose matches on Subject: below --
2010-10-09  0:46 [PATCH V5 0/7] Generalise ARM perf-events backend for oprofile Matt Fleming
2010-10-09  0:46 ` [PATCH 2/7] perf: New helper function for pmu name Matt Fleming
2010-10-09  0:46   ` Matt Fleming
2010-10-09  0:46   ` Matt Fleming
2010-10-11  9:18   ` Robert Richter
2010-10-11  9:18     ` Robert Richter
2010-10-11  9:18     ` Robert Richter
2010-10-11  9:18     ` Robert Richter
2010-10-11  9:18     ` Robert Richter
2010-10-11  9:31     ` Peter Zijlstra
2010-10-11  9:31       ` Peter Zijlstra
2010-10-11  9:31       ` Peter Zijlstra
2010-10-11  9:31       ` Peter Zijlstra
2010-10-11 15:31   ` Paul Mundt
2010-10-11 15:31     ` Paul Mundt
2010-10-11 15:31     ` Paul Mundt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20101006131825.GO13563@erda.amd.com \
    --to=robert.richter@amd.com \
    --cc=acme@redhat.com \
    --cc=dengcheng.zhu@gmail.com \
    --cc=fweisbec@gmail.com \
    --cc=grant.likely@secretlab.ca \
    --cc=lethal@linux-sh.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sh@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=matt@console-pimps.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.