linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: mark.rutland@arm.com (Mark Rutland)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: perf: remove erroneous check on active_events
Date: Mon, 16 May 2011 10:27:40 +0100	[thread overview]
Message-ID: <000001cc13ab$81236e60$836a4b20$@rutland@arm.com> (raw)
In-Reply-To: <20110428161242.GE3052@pulham.picochip.com>

> Yup, I messed up there.  How about the patch below that eliminates the
> atomic_t's entirely?  This means that we always lock the mutex in event
> destruction rather than just for the last one but that seems acceptable
> to me.

Whilst this solves the race condition, it still leaves behind some of the
problems caused by having the counter in the first place:

- Events which can't actually be armed on the hardware will be initialised,
  but still take a slot that could otherwise be used.

- Events which can be armed on the hardware may be blocked by the above, or
  other events which are currently context-switched out (and are essentially
  independent).

- The maximum number of events we can initialise it that which can be
  supported by one core. As the number of cores scale upwards, we're going
to
  waste the majority of the hardware counters.

By removing the counter, we lazily deny events we cannot allocate in the
worst
case. We can still guarantee schedulability of groups if users are sensible
with them.

By having the counter, we aggressively (possibly erroneously) deny events.
As
we cannot check the possibility of arming single events until they hit the
hardware, this can only be considered a heuristic (and not a great one).

> The MIPS perf events code is based on this so could do with the same 
> change.

Agreed. Deleting 4 lines is an easy patch to port. I'll post the ARM changes
to Russell's patch system.

Mark.

      parent reply	other threads:[~2011-05-16  9:27 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-27  9:57 [PATCH] ARM: perf: remove erroneous check on active_events Mark Rutland
     [not found] ` <BANLkTi=J=4R2v5P2iOsf4M5_FMwWJ=JrZw@mail.gmail.com>
2011-04-28 15:21   ` Ashwin Chaugule
2011-04-28 15:41 ` Russell King - ARM Linux
2011-04-28 16:12   ` Jamie Iles
2011-04-28 17:25     ` Will Deacon
2011-05-16  9:27     ` Mark Rutland [this message]

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='000001cc13ab$81236e60$836a4b20$@rutland@arm.com' \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).