From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] arm64: ftrace: stop using kstop_machine to enable/disable tracing
Date: Thu, 3 Dec 2015 09:38:21 +0000 [thread overview]
Message-ID: <20151203093821.GD7023@arm.com> (raw)
In-Reply-To: <56600992.4040005@huawei.com>
On Thu, Dec 03, 2015 at 05:21:22PM +0800, libin wrote:
>
> on 2015/12/2 20:36, Will Deacon wrote:
> > On Sat, Nov 28, 2015 at 03:50:09PM +0800, Li Bin wrote:
> >> On arm64, kstop_machine which is hugely disruptive to a running
> >> system is not needed to convert nops to ftrace calls or back,
> >> because that modifed code is a single 32bit instructions which
> >> is impossible to cross cache (or page) boundaries, and the used str
> >> instruction is single-copy atomic.
> > This commit message is misleading, since the single-copy atomicity
> > guarantees don't apply to the instruction-side. Instead, the architecture
> > calls out a handful of safe instructions in "Concurrent modification and
> > execution of instructions".
>
> Right, thank you for your comments.
>
> > Now, those safe instructions *do* include NOP, B and BL, so that should
> > be sufficient for ftrace provided that we don't patch condition codes
> > (and I don't think we do).
>
> Yes, and so far this assumption has no probem, but in order to avoid exceeding these
> safe insturctions in the future, we can use aarch64_insn_hotpatch_safe() to verify the
> instruction to determine whether needs stop_machine() to synchronize or use
> aarch64_insn_patch_text directly. Right or I am missing something?
I think you're missing the case where the instruction changes under our
feet after we've read it but before we've replaced it (e.g. due to module
unloading). I think that's why ftrace_modify_code has the comment about
lack of locking thanks to stop_machine.
Will
WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will.deacon@arm.com>
To: libin <huawei.libin@huawei.com>
Cc: rostedt@goodmis.org, mingo@redhat.com, catalin.marinas@arm.com,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, guohanjun@huawei.com,
dingtianhong@huawei.com
Subject: Re: [PATCH] arm64: ftrace: stop using kstop_machine to enable/disable tracing
Date: Thu, 3 Dec 2015 09:38:21 +0000 [thread overview]
Message-ID: <20151203093821.GD7023@arm.com> (raw)
In-Reply-To: <56600992.4040005@huawei.com>
On Thu, Dec 03, 2015 at 05:21:22PM +0800, libin wrote:
>
> on 2015/12/2 20:36, Will Deacon wrote:
> > On Sat, Nov 28, 2015 at 03:50:09PM +0800, Li Bin wrote:
> >> On arm64, kstop_machine which is hugely disruptive to a running
> >> system is not needed to convert nops to ftrace calls or back,
> >> because that modifed code is a single 32bit instructions which
> >> is impossible to cross cache (or page) boundaries, and the used str
> >> instruction is single-copy atomic.
> > This commit message is misleading, since the single-copy atomicity
> > guarantees don't apply to the instruction-side. Instead, the architecture
> > calls out a handful of safe instructions in "Concurrent modification and
> > execution of instructions".
>
> Right, thank you for your comments.
>
> > Now, those safe instructions *do* include NOP, B and BL, so that should
> > be sufficient for ftrace provided that we don't patch condition codes
> > (and I don't think we do).
>
> Yes, and so far this assumption has no probem, but in order to avoid exceeding these
> safe insturctions in the future, we can use aarch64_insn_hotpatch_safe() to verify the
> instruction to determine whether needs stop_machine() to synchronize or use
> aarch64_insn_patch_text directly. Right or I am missing something?
I think you're missing the case where the instruction changes under our
feet after we've read it but before we've replaced it (e.g. due to module
unloading). I think that's why ftrace_modify_code has the comment about
lack of locking thanks to stop_machine.
Will
next prev parent reply other threads:[~2015-12-03 9:38 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-28 7:50 [PATCH] arm64: ftrace: stop using kstop_machine to enable/disable tracing Li Bin
2015-11-28 7:50 ` Li Bin
2015-11-28 15:58 ` Steven Rostedt
2015-11-28 15:58 ` Steven Rostedt
2015-11-30 2:03 ` libin
2015-11-30 2:03 ` libin
2015-12-02 12:36 ` Will Deacon
2015-12-02 12:36 ` Will Deacon
2015-12-02 13:16 ` Will Deacon
2015-12-02 13:16 ` Will Deacon
2015-12-03 9:39 ` libin
2015-12-03 9:39 ` libin
2015-12-03 11:48 ` Will Deacon
2015-12-03 11:48 ` Will Deacon
2015-12-03 15:07 ` Steven Rostedt
2015-12-03 15:07 ` Steven Rostedt
2015-12-02 14:02 ` Steven Rostedt
2015-12-02 14:02 ` Steven Rostedt
2015-12-03 9:21 ` libin
2015-12-03 9:21 ` libin
2015-12-03 9:38 ` Will Deacon [this message]
2015-12-03 9:38 ` Will Deacon
2015-12-03 15:05 ` Steven Rostedt
2015-12-03 15:05 ` Steven Rostedt
2015-12-03 15:09 ` Will Deacon
2015-12-03 15:09 ` Will Deacon
2015-12-03 15:31 ` Steven Rostedt
2015-12-03 15:31 ` Steven Rostedt
2015-12-04 1:00 ` libin
2015-12-04 1:00 ` libin
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=20151203093821.GD7023@arm.com \
--to=will.deacon@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 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.