LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Florian Fainelli <f.fainelli@gmail.com>
To: Matt Redfearn <matt.redfearn@mips.com>,
	James Hogan <jhogan@kernel.org>,
	Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org, Peter Zijlstra <peterz@infradead.org>,
	oprofile-list@lists.sf.net, Huacai Chen <chenhc@lemote.com>,
	linux-kernel@vger.kernel.org, Paul Burton <paul.burton@mips.com>,
	Robert Richter <rric@kernel.org>, Jiri Olsa <jolsa@redhat.com>,
	Kate Stewart <kstewart@linuxfoundation.org>,
	Ingo Molnar <mingo@redhat.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Maciej W. Rozycki" <macro@mips.com>,
	Namhyung Kim <namhyung@kernel.org>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>
Subject: Re: [PATCH v3 0/7] MIPS: perf: MT fixes and improvements
Date: Fri, 20 Apr 2018 15:51:18 -0700	[thread overview]
Message-ID: <c3ee1ba4-3458-33a7-4c1f-700936a57bfa@gmail.com> (raw)
In-Reply-To: <1524219789-31241-1-git-send-email-matt.redfearn@mips.com>

On 04/20/2018 03:23 AM, Matt Redfearn wrote:
> This series addresses a few issues with how the MIPS performance
> counters code supports the hardware multithreading MT ASE.
> 
> Firstly, implementations of the MT ASE may implement performance
> counters
> per core or per thread(TC). MIPS Techologies implementations signal this
> via a bit in the implmentation specific CONFIG7 register. Since this
> register is implementation specific, checking it should be guarded by a
> PRID check. This also replaces a bit defined by a magic number.
> 
> Secondly, the code currently uses vpe_id(), defined as
> smp_processor_id(), divided by 2, to share core performance counters
> between VPEs. This relies on a couple of assumptions of the hardware
> implementation to function correctly (always 2 VPEs per core, and the
> hardware reading only the least significant bit).
> 
> Finally, the method of sharing core performance counters between VPEs is
> suboptimal since it does not allow one process running on a VPE to use
> all of the performance counters available to it, because the kernel will
> reserve half of the coutners for the other VPE even if it may never use
> them. This reservation at counter probe is replaced with an allocation
> on use strategy.
> 
> Tested on a MIPS Creator CI40 (2C2T MIPS InterAptiv with per-TC
> counters, though for the purposes of testing the per-TC availability was
> hardcoded to allow testing both paths).
> 
> Series applies to v4.16

Sorry it took so long to get that tested.

Sounds like you need to build test this on a BMIPS5000 configuration
(bmips_stb_defconfig should provide that):

In file included from ./arch/mips/include/asm/mach-generic/spaces.h:15:0,
                 from ./arch/mips/include/asm/mach-bmips/spaces.h:16,
                 from ./arch/mips/include/asm/addrspace.h:13,
                 from ./arch/mips/include/asm/barrier.h:11,
                 from ./include/linux/compiler.h:245,
                 from ./include/linux/kernel.h:10,
                 from ./include/linux/cpumask.h:10,
                 from arch/mips/kernel/perf_event_mipsxx.c:18:
arch/mips/kernel/perf_event_mipsxx.c: In function 'mipsxx_pmu_enable_event':
./arch/mips/include/asm/mipsregs.h:738:52: error: suggest parentheses
around '+' in operand of '&' [-Werror=parentheses]
 #define BRCM_PERFCTRL_VPEID(v) (_ULCAST_(1) << (12 + v))

arch/mips/kernel/perf_event_mipsxx.c:385:10: note: in expansion of macro
'BRCM_PERFCTRL_VPEID'
   ctrl = BRCM_PERFCTRL_VPEID(cpu & MIPS_CPUID_TO_COUNTER_MASK);
          ^~~~~~~~~~~~~~~~~~~
  CC      drivers/of/fdt_addres

after fixing that, I tried the following to see whether this would be a
good test case to exercise against:

perf record -a -C 0 taskset -c 1 /bin/true
perf record -a -C 1 taskset -c 0 /bin/true

and would not see anything related to /bin/true running in either case,
which seems like it does the right thing?

Tested-by: Florian Fainelli <f.fainelli@gmail.com>

BTW, for some reason not specifying -a -C <cpu> does lead to lockups,
consistently and for pretty much any perf command, this could be BMIPS
specific, did not get a chance to cross test on a different machine.

> 
> 
> Changes in v3:
> New patch to detect feature presence in cpu-probe.c
> Use flag in cpu_data set by cpu_probe.c to indicate feature presence.
> - rebase on new feature detection
> 
> Changes in v2:
> Fix mipsxx_pmu_enable_event for !#ifdef CONFIG_MIPS_MT_SMP
> - Fix !#ifdef CONFIG_MIPS_PERF_SHARED_TC_COUNTERS build
> - re-use cpuc variable in mipsxx_pmu_alloc_counter,
>   mipsxx_pmu_free_counter rather than having sibling_ version.
> Since BMIPS5000 does not implement per TC counters, we can remove the
> check on cpu_has_mipsmt_pertccounters.
> New patch to fix BMIPS5000 system mode perf.
> 
> Matt Redfearn (7):
>   MIPS: Probe for MIPS MT perf counters per TC
>   MIPS: perf: More robustly probe for the presence of per-tc counters
>   MIPS: perf: Use correct VPE ID when setting up VPE tracing
>   MIPS: perf: Fix perf with MT counting other threads
>   MIPS: perf: Allocate per-core counters on demand
>   MIPS: perf: Fold vpe_id() macro into it's one last usage
>   MIPS: perf: Fix BMIPS5000 system mode counting
> 
>  arch/mips/include/asm/cpu-features.h |   7 ++
>  arch/mips/include/asm/cpu.h          |   2 +
>  arch/mips/include/asm/mipsregs.h     |   6 +
>  arch/mips/kernel/cpu-probe.c         |  12 ++
>  arch/mips/kernel/perf_event_mipsxx.c | 232 +++++++++++++++++++----------------
>  arch/mips/oprofile/op_model_mipsxx.c |   2 -
>  6 files changed, 150 insertions(+), 111 deletions(-)
> 


-- 
Florian

  parent reply	other threads:[~2018-04-20 22:51 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-20 10:23 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
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 ` Florian Fainelli [this message]
2018-04-23 13:40   ` [PATCH v3 0/7] MIPS: perf: MT fixes and improvements 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=c3ee1ba4-3458-33a7-4c1f-700936a57bfa@gmail.com \
    --to=f.fainelli@gmail.com \
    --cc=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=chenhc@lemote.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jhogan@kernel.org \
    --cc=jolsa@redhat.com \
    --cc=kstewart@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=macro@mips.com \
    --cc=matt.redfearn@mips.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=oprofile-list@lists.sf.net \
    --cc=paul.burton@mips.com \
    --cc=peterz@infradead.org \
    --cc=ralf@linux-mips.org \
    --cc=rric@kernel.org \
    --subject='Re: [PATCH v3 0/7] MIPS: perf: MT fixes and improvements' \
    /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).