Re: [PATCH v7 2/3] arm64: implement ftrace with regs

From: Julien Thierry <julien.thierry@arm.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	linux-kernel@vger.kernel.org,
	AKASHI Takahiro <takahiro.akashi@linaro.org>,
	Ingo Molnar <mingo@redhat.com>, Torsten Duwe <duwe@lst.de>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Amit Daniel Kachhap <amit.kachhap@arm.com>,
	live-patching@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v7 2/3] arm64: implement ftrace with regs
Date: Thu, 7 Feb 2019 14:58:55 +0000	[thread overview]
Message-ID: <5efd27b1-81b5-267a-9fe8-67bb084d56eb@arm.com> (raw)
In-Reply-To: <20190207095157.3dea1086@gandalf.local.home>

On 07/02/2019 14:51, Steven Rostedt wrote:
> On Thu, 7 Feb 2019 10:33:50 +0000
> Julien Thierry <julien.thierry@arm.com> wrote:
> 
>> I don't see really much documentation on that function. As far as I can
>> tell it is only called once for each site (and if it didn't, we'd always
>> be placing the same instruction, but I agree it wouldn't be nice). It
>> could depend on how far you can expand the notion of "adjusting" :) .
>>
>> Steven, do you have an opinion on whether it would be acceptable to
>> modify function entry code in ftrace_call_adjust() ?
> 
> Just to make sure I'm on the same page as you are. You want to modify
> the function entry code at the time of the ftrace_call_adjust()?
> 

Yes, that was the intention, the reason being that we want to have an
instruction that is just patched once on each entry for initialization.

> I would update the rec->ip to the offset you want at
> ftrace_call_adjust() but not do any modifications. It really isn't safe
> to do it there. But right after that is called, you will have the arch
> specific call of ftrace_make_nop() called with MCOUNT_ADDR as the
> second parameter to let you know that this is the first time the call
> is made at this address. This is where you can do that initial
> modifications.

Yes, this is was the current version of this patch is doing, I was just
wondering whether we could clean things up a little (i.e. have
ftrace_make_nop() not generate a non-nop instruction :) ).

But we're not going to hack through the API if this is definitely not
what it's intended for. We'll keep it as is and just update the rec->ip
with ftrace_call_adjust().

If we really want to clean things up, we could look into patching this
at instruction at build time. But if it's not trivial I guess we can
keep that for a later time.

Thanks,

-- 
Julien Thierry

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel