linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: lorenzo.pieralisi@arm.com (Lorenzo Pieralisi)
To: linux-arm-kernel@lists.infradead.org
Subject: arm64 function_graph tracer panic with CONFIG_DYNAMIC_FTRACE
Date: Tue, 17 Nov 2015 10:12:55 +0000	[thread overview]
Message-ID: <20151117101255.GA940@red-moon> (raw)
In-Reply-To: <564A773D.6040209@linaro.org>

On Tue, Nov 17, 2015 at 09:39:25AM +0900, AKASHI Takahiro wrote:
> On 11/17/2015 12:48 AM, Lorenzo Pieralisi wrote:
> >On Mon, Nov 16, 2015 at 01:45:19PM +0000, Catalin Marinas wrote:
> >
> >[...]
> >
> >>>There are some other functions which are called by cpu_suspend(), e.g. psci_system_suspend().
> >>>Should we apply a similar fix to them?
> >>
> >>I think we need to apply the fix to any function which does not return.
> >>In general, this should apply to all finishers passed to cpu_suspend()
> >>and the subsequent callees.
> >
> >Yes, I prefer Steven's suggestion though it seems to me the issue
> >is only related to the graph tracer and by pausing/resuming tracing
> >across cpu_suspend() we would solve the problem without having to
> >patch the finishers (and we can still trace them with the function
> >tracer).
> 
> Aha, I didn't know this option. Yes, the issue is function_graph specific.
> I confirmed that we could fix it by sandwiching __cpu_suspend_enter()
> in cpu_suspend() between pause/unpause_graph_tracing().

Yes, the problem is that we are pushing trace contexts on the trace
stack but we are not popping them (ie suspend finishers do not
return), then we pop the wrong trace stack when we go back to cpu_suspend()
from cpu_resume().

> >Takahiro, do you want me to send a patch or you update yours ?
> 
> I think you're the best person.
> one question: do we need 'notrace' against __cpu_suspend_save()?

No, I will remove that with a separate patch.

> >>Do we need such annotation for cpu_die() as well? It probably doesn't
> >>matter as the CPU is coming back on a completely different path anyway.
> >
> >I will test this too in the process.
> 
> Function_graph related info is per-task, and it means that, if cpu_die()
> destroys idle thread, we don't have to care.
> But testing is always crucial.

I am testing the hotplug path with the graph tracer, it seems to work
fine I will continue testing it.

Patch coming, please have a look.

Thanks,
Lorenzo

  reply	other threads:[~2015-11-17 10:12 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-12 10:42 arm64 function_graph tracer panic with CONFIG_DYNAMIC_FTRACE Catalin Marinas
2015-11-13  5:51 ` AKASHI Takahiro
2015-11-13 15:16   ` Jungseok Lee
2015-11-16  1:56   ` AKASHI Takahiro
2015-11-16 13:45     ` Catalin Marinas
2015-11-16 15:48       ` Lorenzo Pieralisi
2015-11-17  0:39         ` AKASHI Takahiro
2015-11-17 10:12           ` Lorenzo Pieralisi [this message]
2015-11-16 14:20     ` Steven Rostedt

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=20151117101255.GA940@red-moon \
    --to=lorenzo.pieralisi@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).