LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: James Hogan <jhogan@kernel.org>
To: Matt Redfearn <matt.redfearn@mips.com>
Cc: Ralf Baechle <ralf@linux-mips.org>,
	Florian Fainelli <f.fainelli@gmail.com>,
	linux-mips@linux-mips.org, Namhyung Kim <namhyung@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	linux-kernel@vger.kernel.org, Ingo Molnar <mingo@redhat.com>,
	Jiri Olsa <jolsa@redhat.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>
Subject: Re: [PATCH v3 5/7] MIPS: perf: Allocate per-core counters on demand
Date: Wed, 16 May 2018 19:05:20 +0100	[thread overview]
Message-ID: <20180516180518.GB12837@jamesdev> (raw)
In-Reply-To: <1524219789-31241-6-git-send-email-matt.redfearn@mips.com>

[-- Attachment #1: Type: text/plain, Size: 3923 bytes --]

On Fri, Apr 20, 2018 at 11:23:07AM +0100, Matt Redfearn wrote:
> Previously when performance counters are per-core, rather than
> per-thread, the number available were divided by 2 on detection, and the
> counters used by each thread in a core were "swizzled" to ensure
> separation. However, this solution is suboptimal since it relies on a
> couple of assumptions:
> a) Always having 2 VPEs / core (number of counters was divided by 2)
> b) Always having a number of counters implemented in the core that is
>    divisible by 2. For instance if an SoC implementation had a single
>    counter and 2 VPEs per core, then this logic would fail and no
>    performance counters would be available.
> The mechanism also does not allow for one VPE in a core using more than
> it's allocation of the per-core counters to count multiple events even
> though other VPEs may not be using them.
> 
> Fix this situation by instead allocating (and releasing) per-core
> performance counters when they are requested. This approach removes the
> above assumptions and fixes the shortcomings.
> 
> In order to do this:
> Add additional logic to mipsxx_pmu_alloc_counter() to detect if a
> sibling is using a per-core counter, and to allocate a per-core counter
> in all sibling CPUs.
> Similarly, add a mipsxx_pmu_free_counter() function to release a
> per-core counter in all sibling CPUs when it is finished with.
> A new spinlock, core_counters_lock, is introduced to ensure exclusivity
> when allocating and releasing per-core counters.
> Since counters are now allocated per-core on demand, rather than being
> reserved per-thread at boot, all of the "swizzling" of counters is
> removed.
> 
> The upshot is that in an SoC with 2 counters / thread, counters are
> reported as:
> Performance counters: mips/interAptiv PMU enabled, 2 32-bit counters
> available to each CPU, irq 18
> 
> Running an instance of a test program on each of 2 threads in a
> core, both threads can use their 2 counters to count 2 events:
> 
> taskset 4 perf stat -e instructions:u,branches:u ./test_prog & taskset 8
> perf stat -e instructions:u,branches:u ./test_prog
> 
>  Performance counter stats for './test_prog':
> 
>              30002      instructions:u
>              10000      branches:u
> 
>        0.005164264 seconds time elapsed
>  Performance counter stats for './test_prog':
> 
>              30002      instructions:u
>              10000      branches:u
> 
>        0.006139975 seconds time elapsed
> 
> In an SoC with 2 counters / core (which can be forced by setting
> cpu_has_mipsmt_pertccounters = 0), counters are reported as:
> Performance counters: mips/interAptiv PMU enabled, 2 32-bit counters
> available to each core, irq 18
> 
> Running an instance of a test program on each of 2 threads in a
> core, now only one thread manages to secure the performance counters to
> count 2 events. The other thread does not get any counters.
> 
> taskset 4 perf stat -e instructions:u,branches:u ./test_prog & taskset 8
> perf stat -e instructions:u,branches:u ./test_prog
> 
>  Performance counter stats for './test_prog':
> 
>      <not counted>       instructions:u
>      <not counted>       branches:u
> 
>        0.005179533 seconds time elapsed
> 
>  Performance counter stats for './test_prog':
> 
>              30002      instructions:u
>              10000      branches:u
> 
>        0.005179467 seconds time elapsed
> 
> Signed-off-by: Matt Redfearn <matt.redfearn@mips.com>

While this sounds like an improvement in practice, being able to use
more counters on single threaded stuff than otherwise, I'm a little
concerned what would happen if a task was migrated to a different CPU
and the perf counters couldn't be obtained on the new CPU due to
counters already being in use. Would the values be incorrectly small?

Cheers
James

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

  reply	other threads:[~2018-05-16 18:05 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-20 10:23 [PATCH v3 0/7] MIPS: perf: MT fixes and improvements Matt Redfearn
2018-04-20 10:23 ` [PATCH v3 1/7] MIPS: Probe for MIPS MT perf counters per TC Matt Redfearn
2018-04-20 10:23 ` [PATCH v3 2/7] MIPS: perf: More robustly probe for the presence of per-tc counters Matt Redfearn
2018-04-20 10:23 ` [PATCH v3 3/7] MIPS: perf: Use correct VPE ID when setting up VPE tracing Matt Redfearn
2018-04-20 10:23 ` [PATCH v3 4/7] MIPS: perf: Fix perf with MT counting other threads Matt Redfearn
2018-05-16 17:59   ` James Hogan
2018-05-17 10:35     ` Matt Redfearn
2018-04-20 10:23 ` [PATCH v3 5/7] MIPS: perf: Allocate per-core counters on demand Matt Redfearn
2018-05-16 18:05   ` James Hogan [this message]
2018-05-17 10:40     ` Matt Redfearn
2018-04-20 10:23 ` [PATCH v3 6/7] MIPS: perf: Fold vpe_id() macro into it's one last usage Matt Redfearn
2018-04-20 10:23 ` [PATCH v3 7/7] MIPS: perf: Fix BMIPS5000 system mode counting Matt Redfearn
2018-05-15 14:44   ` [PATCH v4] " Matt Redfearn
2018-04-20 22:51 ` [PATCH v3 0/7] MIPS: perf: MT fixes and improvements Florian Fainelli
2018-04-23 13:40   ` Matt Redfearn

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=20180516180518.GB12837@jamesdev \
    --to=jhogan@kernel.org \
    --cc=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=f.fainelli@gmail.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=matt.redfearn@mips.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=ralf@linux-mips.org \
    --subject='Re: [PATCH v3 5/7] MIPS: perf: Allocate per-core counters on demand' \
    /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

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).