All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Ellerman <mpe@ellerman.id.au>
To: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-s390@vger.kernel.org, sparclinux@vger.kernel.org
Subject: Re: [[PATCH v6 02/10] perf: Add a flags parameter to pmu txn interfaces
Date: Fri, 04 Sep 2015 20:07:29 +1000	[thread overview]
Message-ID: <1441361249.3777.3.camel@ellerman.id.au> (raw)
In-Reply-To: <1441336073-22750-3-git-send-email-sukadev@linux.vnet.ibm.com>

On Thu, 2015-09-03 at 20:07 -0700, Sukadev Bhattiprolu wrote:
> Currently, the PMU interface allows reading only one counter at a time.
> But some PMUs like the 24x7 counters in Power, support reading several
> counters at once. To leveage this functionality, extend the transaction
> interface to support a "transaction type".
> 
> The first type, PERF_PMU_TXN_ADD, refers to the existing transactions,
> i.e. used to _schedule_ all the events on the PMU as a group. A second
> transaction type, PERF_PMU_TXN_READ, will be used in a follow-on patch,
> by the 24x7 counters to read several counters at once.
> 
> Extend the transaction interfaces to the PMU to accept a 'txn_flags'
> parameter and use this parameter to ignore any transactions that are
> not of type PERF_PMU_TXN_ADD.
> 
> Thanks to Peter Zijlstra for his input.
> 
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> 
> ---
>  arch/powerpc/perf/core-book3s.c  |   30 +++++++++++++++++++++++++++++-

These powerpc changes look OK to me.

So for those you can have an:

Acked-by: Michael Ellerman <mpe@ellerman.id.au>


Having said that, we do end up repeating a lot of boiler plate for each arch,
which is a pity. eg:

> +static void power_pmu_start_txn(struct pmu *pmu, unsigned int txn_flags)
>  {
>  	struct cpu_hw_events *cpuhw = this_cpu_ptr(&cpu_hw_events);
>
> +	WARN_ON_ONCE(cpuhw->txn_flags);		/* txn already in flight */
> +
> +	cpuhw->txn_flags = txn_flags;
> +	if (txn_flags & ~PERF_PMU_TXN_ADD)
> +		return;
> +

And so on.

But I can't think of an easy way to avoid that, so it's not a blocker, but
maybe someone can think of a nice solution to avoid it?

cheers

WARNING: multiple messages have this Message-ID (diff)
From: Michael Ellerman <mpe@ellerman.id.au>
To: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-s390@vger.kernel.org, sparclinux@vger.kernel.org
Subject: Re: [[PATCH v6 02/10] perf: Add a flags parameter to pmu txn interfaces
Date: Fri, 04 Sep 2015 10:07:29 +0000	[thread overview]
Message-ID: <1441361249.3777.3.camel@ellerman.id.au> (raw)
In-Reply-To: <1441336073-22750-3-git-send-email-sukadev@linux.vnet.ibm.com>

On Thu, 2015-09-03 at 20:07 -0700, Sukadev Bhattiprolu wrote:
> Currently, the PMU interface allows reading only one counter at a time.
> But some PMUs like the 24x7 counters in Power, support reading several
> counters at once. To leveage this functionality, extend the transaction
> interface to support a "transaction type".
> 
> The first type, PERF_PMU_TXN_ADD, refers to the existing transactions,
> i.e. used to _schedule_ all the events on the PMU as a group. A second
> transaction type, PERF_PMU_TXN_READ, will be used in a follow-on patch,
> by the 24x7 counters to read several counters at once.
> 
> Extend the transaction interfaces to the PMU to accept a 'txn_flags'
> parameter and use this parameter to ignore any transactions that are
> not of type PERF_PMU_TXN_ADD.
> 
> Thanks to Peter Zijlstra for his input.
> 
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> 
> ---
>  arch/powerpc/perf/core-book3s.c  |   30 +++++++++++++++++++++++++++++-

These powerpc changes look OK to me.

So for those you can have an:

Acked-by: Michael Ellerman <mpe@ellerman.id.au>


Having said that, we do end up repeating a lot of boiler plate for each arch,
which is a pity. eg:

> +static void power_pmu_start_txn(struct pmu *pmu, unsigned int txn_flags)
>  {
>  	struct cpu_hw_events *cpuhw = this_cpu_ptr(&cpu_hw_events);
>
> +	WARN_ON_ONCE(cpuhw->txn_flags);		/* txn already in flight */
> +
> +	cpuhw->txn_flags = txn_flags;
> +	if (txn_flags & ~PERF_PMU_TXN_ADD)
> +		return;
> +

And so on.

But I can't think of an easy way to avoid that, so it's not a blocker, but
maybe someone can think of a nice solution to avoid it?

cheers




  reply	other threads:[~2015-09-04 10:07 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-04  3:07 [PATCH v6 0/10] perf: Implement group-read of events using txn interface Sukadev Bhattiprolu
2015-09-04  3:07 ` Sukadev Bhattiprolu
2015-09-04  3:07 ` Sukadev Bhattiprolu
2015-09-04  3:07 ` [[PATCH v6 01/10] sparc/perf: Remove unnecessary assignment Sukadev Bhattiprolu
2015-09-04  3:07   ` Sukadev Bhattiprolu
2015-09-04  3:07   ` Sukadev Bhattiprolu
2015-09-13 11:10   ` [tip:perf/core] sparc, perf/sparc: " tip-bot for Sukadev Bhattiprolu
2015-09-04  3:07 ` [[PATCH v6 02/10] perf: Add a flags parameter to pmu txn interfaces Sukadev Bhattiprolu
2015-09-04  3:07   ` Sukadev Bhattiprolu
2015-09-04  3:07   ` Sukadev Bhattiprolu
2015-09-04 10:07   ` Michael Ellerman [this message]
2015-09-04 10:07     ` Michael Ellerman
2015-09-13 11:10   ` [tip:perf/core] perf/core: Add a 'flags' parameter to the PMU transactional interfaces tip-bot for Sukadev Bhattiprolu
2015-09-04  3:07 ` [[PATCH v6 03/10] perf: Split perf_event_read() and perf_event_count() Sukadev Bhattiprolu
2015-09-04  3:07   ` Sukadev Bhattiprolu
2015-09-04  3:07   ` Sukadev Bhattiprolu
2015-09-13 11:11   ` [tip:perf/core] perf/core: " tip-bot for Sukadev Bhattiprolu
2015-09-04  3:07 ` [[PATCH v6 04/10] perf: Rename perf_event_read_{one, group}, perf_read_hw Sukadev Bhattiprolu
2015-09-04  3:07   ` [[PATCH v6 04/10] perf: Rename perf_event_read_{one,group}, perf_read_hw Sukadev Bhattiprolu
2015-09-04  3:07   ` Sukadev Bhattiprolu
2015-09-04  3:07   ` [[PATCH v6 04/10] perf: Rename perf_event_read_{one, group}, perf_read_hw Sukadev Bhattiprolu
2015-09-13 11:11   ` [tip:perf/core] perf/core: Rename perf_event_read_{one,group}, perf_read_hw tip-bot for Peter Zijlstra (Intel)
2015-09-04  3:07 ` [[PATCH v6 05/10] perf: Add group reads to perf_event_read() Sukadev Bhattiprolu
2015-09-04  3:07   ` Sukadev Bhattiprolu
2015-09-04  3:07   ` Sukadev Bhattiprolu
2015-09-04  3:07 ` [[PATCH v6 06/10] perf: Invert perf_read_group() loops Sukadev Bhattiprolu
2015-09-04  3:07   ` Sukadev Bhattiprolu
2015-09-04  3:07   ` Sukadev Bhattiprolu
2015-09-13 11:12   ` [tip:perf/core] perf/core: " tip-bot for Peter Zijlstra
2015-09-04  3:07 ` [[PATCH v6 07/10] perf: Add return value for perf_event_read() Sukadev Bhattiprolu
2015-09-04  3:07   ` Sukadev Bhattiprolu
2015-09-04  3:07   ` Sukadev Bhattiprolu
2015-09-13 11:12   ` [tip:perf/core] perf/core: " tip-bot for Sukadev Bhattiprolu
2015-09-04  3:07 ` [[PATCH v6 08/10] Define PERF_PMU_TXN_READ interface Sukadev Bhattiprolu
2015-09-04  3:07   ` Sukadev Bhattiprolu
2015-09-04  3:07   ` Sukadev Bhattiprolu
2015-09-13 11:13   ` [tip:perf/core] perf/core: " tip-bot for Sukadev Bhattiprolu
2015-09-04  3:07 ` [[PATCH v6 09/10] powerpc/perf/hv-24x7: Use PMU_TXN_READ interface Sukadev Bhattiprolu
2015-09-04  3:07   ` Sukadev Bhattiprolu
2015-09-04  3:07   ` Sukadev Bhattiprolu
2015-09-08  9:07   ` Michael Ellerman
2015-09-08  9:07     ` Michael Ellerman
2015-09-08 11:29     ` Peter Zijlstra
2015-09-08 11:29       ` Peter Zijlstra
2015-09-09  2:15       ` Michael Ellerman
2015-09-09  2:15         ` Michael Ellerman
2015-09-09 21:12         ` Sukadev Bhattiprolu
2015-09-09 21:12           ` Sukadev Bhattiprolu
2015-09-10  0:43           ` Michael Ellerman
2015-09-10  0:43             ` Michael Ellerman
2015-09-13 11:13   ` [tip:perf/core] powerpc, perf/powerpc/hv-24x7: " tip-bot for Sukadev Bhattiprolu
2015-09-04  3:07 ` [[PATCH v6 10/10] perf: Drop PERF_EVENT_TXN Sukadev Bhattiprolu
2015-09-04  3:07   ` Sukadev Bhattiprolu
2015-09-04  3:07   ` Sukadev Bhattiprolu
2015-09-13 11:13   ` [tip:perf/core] perf/core: " tip-bot for Sukadev Bhattiprolu

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=1441361249.3777.3.camel@ellerman.id.au \
    --to=mpe@ellerman.id.au \
    --cc=acme@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=sparclinux@vger.kernel.org \
    --cc=sukadev@linux.vnet.ibm.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.