All of lore.kernel.org
 help / color / mirror / Atom feed
From: mark.rutland@arm.com (Mark Rutland)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCHv3 5/5] arm-cci: CCI-500: Work around PMU counter writes
Date: Fri, 11 Dec 2015 12:14:27 +0000	[thread overview]
Message-ID: <20151211121427.GA20666@leverpostej> (raw)
In-Reply-To: <566AB36D.9050209@arm.com>

On Fri, Dec 11, 2015 at 11:28:45AM +0000, Suzuki K. Poulose wrote:
> On 10/12/15 15:42, Mark Rutland wrote:
> >On Tue, Nov 17, 2015 at 06:03:27PM +0000, Suzuki K. Poulose wrote:
> >>The CCI PMU driver sets the event counter to the half of the maximum
> >>value(2^31) it can count before we start the counters via
> >>pmu_event_set_period(). This is done to give us the best chance to
> >>handle the overflow interrupt, taking care of extreme interrupt latencies.
> 
> 
> >
> >This should work, but it seems very heavyweight given we do it for each
> >write.
> >
> >Can we not amortize this by using the {start,commit,cancel}_txn hooks?
> >
> >Either we can handle 1-4 and 6-8 in those, or we can copy everything
> >into a shadow state and apply it all in one go at commit_txn time.
> 
> I took a look at it. The only worrying part is, if pmu->add() will be
> called outside *_txn().

It looks like that happns.

If we __perf_event_enable an events which is not a leader, we may call
event_sched_in (which will call pmu->add) outside of a transaction. The
__perf_event_disable path is similar w.r.t. pmu->del.

So it does look like we can't rely on being in a transaction there.

Assuming that's deliberate, we could follow the example of other PMU
drivers and keep track of whether or not we're in a transaction. If not,
we do all the heavyweight work inline.

Thanks,
Mark.

WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com>
To: "Suzuki K. Poulose" <Suzuki.Poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org, punit.agrawal@arm.com,
	arm@kernel.org, linux-kernel@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCHv3 5/5] arm-cci: CCI-500: Work around PMU counter writes
Date: Fri, 11 Dec 2015 12:14:27 +0000	[thread overview]
Message-ID: <20151211121427.GA20666@leverpostej> (raw)
In-Reply-To: <566AB36D.9050209@arm.com>

On Fri, Dec 11, 2015 at 11:28:45AM +0000, Suzuki K. Poulose wrote:
> On 10/12/15 15:42, Mark Rutland wrote:
> >On Tue, Nov 17, 2015 at 06:03:27PM +0000, Suzuki K. Poulose wrote:
> >>The CCI PMU driver sets the event counter to the half of the maximum
> >>value(2^31) it can count before we start the counters via
> >>pmu_event_set_period(). This is done to give us the best chance to
> >>handle the overflow interrupt, taking care of extreme interrupt latencies.
> 
> 
> >
> >This should work, but it seems very heavyweight given we do it for each
> >write.
> >
> >Can we not amortize this by using the {start,commit,cancel}_txn hooks?
> >
> >Either we can handle 1-4 and 6-8 in those, or we can copy everything
> >into a shadow state and apply it all in one go at commit_txn time.
> 
> I took a look at it. The only worrying part is, if pmu->add() will be
> called outside *_txn().

It looks like that happns.

If we __perf_event_enable an events which is not a leader, we may call
event_sched_in (which will call pmu->add) outside of a transaction. The
__perf_event_disable path is similar w.r.t. pmu->del.

So it does look like we can't rely on being in a transaction there.

Assuming that's deliberate, we could follow the example of other PMU
drivers and keep track of whether or not we're in a transaction. If not,
we do all the heavyweight work inline.

Thanks,
Mark.

  parent reply	other threads:[~2015-12-11 12:14 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-17 18:03 [PATCHv3 0/5] arm-cci500: Workaround pmu_event_set_period Suzuki K. Poulose
2015-11-17 18:03 ` Suzuki K. Poulose
2015-11-17 18:03 ` [PATCHv3 1/5] arm-cci: Refactor CCI PMU enable/disable methods Suzuki K. Poulose
2015-11-17 18:03   ` Suzuki K. Poulose
2015-12-10 15:26   ` Mark Rutland
2015-12-10 15:26     ` Mark Rutland
2015-11-17 18:03 ` [PATCHv3 2/5] arm-cci: Get the status of a counter Suzuki K. Poulose
2015-11-17 18:03   ` Suzuki K. Poulose
2015-12-10 15:33   ` Mark Rutland
2015-12-10 15:33     ` Mark Rutland
2015-11-17 18:03 ` [PATCHv3 3/5] arm-cci: Add routines to enable/disable all counters Suzuki K. Poulose
2015-11-17 18:03   ` Suzuki K. Poulose
2015-12-10 15:32   ` Mark Rutland
2015-12-10 15:32     ` Mark Rutland
2015-12-10 15:42     ` Suzuki K. Poulose
2015-12-10 15:42       ` Suzuki K. Poulose
2015-12-10 15:47       ` Mark Rutland
2015-12-10 15:47         ` Mark Rutland
2015-11-17 18:03 ` [PATCHv3 4/5] arm-cci: Add hooks for pmu_write_counter Suzuki K. Poulose
2015-11-17 18:03   ` Suzuki K. Poulose
2015-11-17 18:03 ` [PATCHv3 5/5] arm-cci: CCI-500: Work around PMU counter writes Suzuki K. Poulose
2015-11-17 18:03   ` Suzuki K. Poulose
2015-12-10 15:42   ` Mark Rutland
2015-12-10 15:42     ` Mark Rutland
2015-12-11 11:28     ` Suzuki K. Poulose
2015-12-11 11:28       ` Suzuki K. Poulose
2015-12-11 12:10       ` Peter Zijlstra
2015-12-11 12:10         ` Peter Zijlstra
2015-12-11 12:14       ` Mark Rutland [this message]
2015-12-11 12:14         ` Mark Rutland

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=20151211121427.GA20666@leverpostej \
    --to=mark.rutland@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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.