All of lore.kernel.org
 help / color / mirror / Atom feed
From: jszhang@marvell.com (Jisheng Zhang)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] arm64: add cpu_idle tracepoints to arch_cpu_idle
Date: Thu, 17 Sep 2015 00:56:47 +0800	[thread overview]
Message-ID: <20150917005647.70dc907f@xhacker> (raw)
In-Reply-To: <20150916161605.GA29663@red-moon>

On Wed, 16 Sep 2015 17:16:05 +0100
Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:

> On Wed, Sep 16, 2015 at 04:11:05PM +0100, Jisheng Zhang wrote:
> > Dear Lorenzo,
> > 
> > On Wed, 16 Sep 2015 22:53:12 +0800
> > Jisheng Zhang <jszhang@marvell.com> wrote:
> > 
> > > Dear Lorenzo,
> > > 
> > > On Wed, 16 Sep 2015 15:47:38 +0100
> > > Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
> > > 
> > > > On Wed, Sep 16, 2015 at 03:23:21PM +0100, Jisheng Zhang wrote:
> > > > > Currently, if cpuidle is disabled or not supported, powertop reports
> > > > > zero wakeups and zero events. This is due to the cpu_idle tracepoints
> > > > > are missing.
> > > > > 
> > > > > This patch is to make cpu_idle tracepoints always available even if
> > > > > cpuidle is disabled or not supported.
> > > > > 
> > > > > Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
> > > > 
> > > > Is there a reason why this code cannot be moved to the generic idle loop ?
> > > 
> > > Do you mean the cpu_idle_loop() in kernel/sched/idle.c? To be honest, I
> > 
> > Maybe I know now. we need to trace different idle level, for example:
> > 
> > WFI idle: trace_cpu_idle_rcuidle(1, ...);
> > 
> > deeper idle: trace_cpu_idle_rcuidle(2, ...);
> > 
> > Usually, the first argument of trace_cpu_idle_rcuidle() equals to the index
> > of the idle level.
> > 
> > so generic idle loop is not a good candidate.
> 
> You are adding a trace for tracing state 1 (ie default idle state),
> called from arch_cpu_idle(), which is the default idle call when the
> CPUidle framework is not available, so I suggested moving the traces
> you add to arm/arm64 arch_cpu_idle() calls to kernel/sched/idle.c
> (see default_idle_call()) instead of patching architecture code.
> 
> I think you can't do that because on x86 calling arch_cpu_idle()
> does not always mean entering idle state index 1 if I read the code
> correctly (in particular the mwait based implementation - mwait_idle()).
> 
> So never mind, patch is fine (on arm64, on arm you should be careful
> because some arm_pm_idle implementations trace state 1 already -
> see omap3_pm_idle and if you add traces to arch_cpu_idle you should
> remove the traces from mach implementations).

OOPs, I was debugging the cascaded irq issues on Marvell BG4CT SoC. Yes, this
arm_pm_idle should be taken care on arm, I should ignore arm_pm_idle, I'll
cook v2 for arm platform.

Thanks a lot,
Jisheng

> 
> Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

WARNING: multiple messages have this Message-ID (diff)
From: Jisheng Zhang <jszhang@marvell.com>
To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Catalin Marinas <Catalin.Marinas@arm.com>,
	Will Deacon <Will.Deacon@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] arm64: add cpu_idle tracepoints to arch_cpu_idle
Date: Thu, 17 Sep 2015 00:56:47 +0800	[thread overview]
Message-ID: <20150917005647.70dc907f@xhacker> (raw)
In-Reply-To: <20150916161605.GA29663@red-moon>

On Wed, 16 Sep 2015 17:16:05 +0100
Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:

> On Wed, Sep 16, 2015 at 04:11:05PM +0100, Jisheng Zhang wrote:
> > Dear Lorenzo,
> > 
> > On Wed, 16 Sep 2015 22:53:12 +0800
> > Jisheng Zhang <jszhang@marvell.com> wrote:
> > 
> > > Dear Lorenzo,
> > > 
> > > On Wed, 16 Sep 2015 15:47:38 +0100
> > > Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
> > > 
> > > > On Wed, Sep 16, 2015 at 03:23:21PM +0100, Jisheng Zhang wrote:
> > > > > Currently, if cpuidle is disabled or not supported, powertop reports
> > > > > zero wakeups and zero events. This is due to the cpu_idle tracepoints
> > > > > are missing.
> > > > > 
> > > > > This patch is to make cpu_idle tracepoints always available even if
> > > > > cpuidle is disabled or not supported.
> > > > > 
> > > > > Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
> > > > 
> > > > Is there a reason why this code cannot be moved to the generic idle loop ?
> > > 
> > > Do you mean the cpu_idle_loop() in kernel/sched/idle.c? To be honest, I
> > 
> > Maybe I know now. we need to trace different idle level, for example:
> > 
> > WFI idle: trace_cpu_idle_rcuidle(1, ...);
> > 
> > deeper idle: trace_cpu_idle_rcuidle(2, ...);
> > 
> > Usually, the first argument of trace_cpu_idle_rcuidle() equals to the index
> > of the idle level.
> > 
> > so generic idle loop is not a good candidate.
> 
> You are adding a trace for tracing state 1 (ie default idle state),
> called from arch_cpu_idle(), which is the default idle call when the
> CPUidle framework is not available, so I suggested moving the traces
> you add to arm/arm64 arch_cpu_idle() calls to kernel/sched/idle.c
> (see default_idle_call()) instead of patching architecture code.
> 
> I think you can't do that because on x86 calling arch_cpu_idle()
> does not always mean entering idle state index 1 if I read the code
> correctly (in particular the mwait based implementation - mwait_idle()).
> 
> So never mind, patch is fine (on arm64, on arm you should be careful
> because some arm_pm_idle implementations trace state 1 already -
> see omap3_pm_idle and if you add traces to arch_cpu_idle you should
> remove the traces from mach implementations).

OOPs, I was debugging the cascaded irq issues on Marvell BG4CT SoC. Yes, this
arm_pm_idle should be taken care on arm, I should ignore arm_pm_idle, I'll
cook v2 for arm platform.

Thanks a lot,
Jisheng

> 
> Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>


  reply	other threads:[~2015-09-16 16:56 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-16 14:23 [PATCH] arm64: add cpu_idle tracepoints to arch_cpu_idle Jisheng Zhang
2015-09-16 14:23 ` Jisheng Zhang
2015-09-16 14:47 ` Lorenzo Pieralisi
2015-09-16 14:47   ` Lorenzo Pieralisi
2015-09-16 14:53   ` Jisheng Zhang
2015-09-16 14:53     ` Jisheng Zhang
2015-09-16 15:11     ` Jisheng Zhang
2015-09-16 15:11       ` Jisheng Zhang
2015-09-16 16:16       ` Lorenzo Pieralisi
2015-09-16 16:16         ` Lorenzo Pieralisi
2015-09-16 16:56         ` Jisheng Zhang [this message]
2015-09-16 16:56           ` Jisheng Zhang
2015-10-19 17:47 ` Catalin Marinas
2015-10-19 17:47   ` Catalin Marinas

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=20150917005647.70dc907f@xhacker \
    --to=jszhang@marvell.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.