From: Nicholas Piggin <npiggin@gmail.com>
To: linuxppc-dev@lists.ozlabs.org,
Michael Ellerman <mpe@ellerman.id.au>,
"Naveen N. Rao" <naveen.n.rao@linux.ibm.com>
Subject: Re: [RFC PATCH] powerpc/64/ftrace: mprofile-kernel patch out mflr
Date: Thu, 16 May 2019 15:49:51 +1000 [thread overview]
Message-ID: <1557985279.4o349j5g2i.astroid@bobo.none> (raw)
In-Reply-To: <1557821918.xbleq18bk2.naveen@linux.ibm.com>
Naveen N. Rao's on May 14, 2019 6:32 pm:
> Michael Ellerman wrote:
>> "Naveen N. Rao" <naveen.n.rao@linux.ibm.com> writes:
>>> Michael Ellerman wrote:
>>>> Nicholas Piggin <npiggin@gmail.com> writes:
>>>>> The new mprofile-kernel mcount sequence is
>>>>>
>>>>> mflr r0
>>>>> bl _mcount
>>>>>
>>>>> Dynamic ftrace patches the branch instruction with a noop, but leaves
>>>>> the mflr. mflr is executed by the branch unit that can only execute one
>>>>> per cycle on POWER9 and shared with branches, so it would be nice to
>>>>> avoid it where possible.
>>>>>
>>>>> This patch is a hacky proof of concept to nop out the mflr. Can we do
>>>>> this or are there races or other issues with it?
>>>>
>>>> There's a race, isn't there?
>>>>
>>>> We have a function foo which currently has tracing disabled, so the mflr
>>>> and bl are nop'ed out.
>>>>
>>>> CPU 0 CPU 1
>>>> ==================================
>>>> bl foo
>>>> nop (ie. not mflr)
>>>> -> interrupt
>>>> something else enable tracing for foo
>>>> ... patch mflr and branch
>>>> <- rfi
>>>> bl _mcount
>>>>
>>>> So we end up in _mcount() but with r0 not populated.
>>>
>>> Good catch! Looks like we need to patch the mflr with a "b +8" similar
>>> to what we do in __ftrace_make_nop().
>>
>> Would that actually make it any faster though? Nick?
>
> Ok, how about doing this as a 2-step process?
> 1. patch 'mflr r0' with a 'b +8'
> synchronize_rcu_tasks()
> 2. convert 'b +8' to a 'nop'
Good idea. Well the mflr r0 is harmless, so you can leave that in.
You just need to ensure it's not removed before the bl is. So nop
the bl _mcount, then synchronize_rcu_tasks(), then nop the mflr?
Thanks,
Nick
next prev parent reply other threads:[~2019-05-16 5:51 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-13 1:59 [RFC PATCH] powerpc/64/ftrace: mprofile-kernel patch out mflr Nicholas Piggin
2019-04-15 9:42 ` Naveen N. Rao
2019-05-13 3:25 ` Michael Ellerman
2019-05-13 6:47 ` Naveen N. Rao
2019-05-13 11:34 ` Michael Ellerman
2019-05-14 8:32 ` Naveen N. Rao
2019-05-15 12:20 ` Michael Ellerman
2019-05-16 5:49 ` Nicholas Piggin [this message]
2019-05-16 18:22 ` Naveen N. Rao
2019-05-17 9:12 ` Nicholas Piggin
2019-05-17 17:25 ` Naveen N. Rao
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=1557985279.4o349j5g2i.astroid@bobo.none \
--to=npiggin@gmail.com \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mpe@ellerman.id.au \
--cc=naveen.n.rao@linux.ibm.com \
/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.