From: Mark Rutland <mark.rutland@arm.com> To: Torsten Duwe <duwe@lst.de> Cc: Will Deacon <will.deacon@arm.com>, Catalin Marinas <catalin.marinas@arm.com>, Julien Thierry <julien.thierry@arm.com>, Steven Rostedt <rostedt@goodmis.org>, Josh Poimboeuf <jpoimboe@redhat.com>, Ingo Molnar <mingo@redhat.com>, Ard Biesheuvel <ard.biesheuvel@linaro.org>, Arnd Bergmann <arnd@arndb.de>, AKASHI Takahiro <takahiro.akashi@linaro.org>, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, live-patching@vger.kernel.org Subject: Re: [PATCH v3 2/4] arm64: implement ftrace with regs Date: Tue, 2 Oct 2018 13:57:34 +0100 [thread overview] Message-ID: <20181002125734.mhkz4o46oxf3mtu6@lakrids.cambridge.arm.com> (raw) In-Reply-To: <20181002121817.GB2398@lst.de> On Tue, Oct 02, 2018 at 02:18:17PM +0200, Torsten Duwe wrote: > Hi Mark, Hi, > thank you for your very detailed feedback, I'll incorporate it > all into the next version, besides one issue: > > On Tue, Oct 02, 2018 at 12:27:41PM +0100, Mark Rutland wrote: > > > > Please use the insn framework, as we do to generate all the other > > instruction sequences in ftrace. > > > > MOV (register) is an alias of ORR (shifted register), i.e. > > > > mov <xd>, <xm> > > > > ... is: > > > > orr <xd>, xzr, <xm> > > > > ... and we have code to generate ORR, so we can add a trivial wrapper to > > generate MOV. > > I had something similar in v2; but it was hardly any better to read or > understand. My main question however is: how do you justify the runtime > overhead of aarch64_insn_gen_logical_shifted_reg for every function that > gets its tracing switched on or off? How do you justify doing something different from the established pattern? Do you have any numbers indicating that this overhead is a problem on a real workload? For the moment at least, please use aarch64_insn_gen_*(), as we do for all other instructions generated in the ftrace code. It's vastly simpler for everyone if we have consistency here. > The result is always the same 4-byte constant, so why not use a macro > and a comment that says what it does? I'd rather that we stick to the usual pattern that we have in arm64. Note that aarch64_insn_gen_nop() also always returns the same 4-byte constant, but it's an out-of-line function in insn.c. There haven't been any complaints as to its overhead so far... Thanks, Mark.
WARNING: multiple messages have this Message-ID (diff)
From: mark.rutland@arm.com (Mark Rutland) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v3 2/4] arm64: implement ftrace with regs Date: Tue, 2 Oct 2018 13:57:34 +0100 [thread overview] Message-ID: <20181002125734.mhkz4o46oxf3mtu6@lakrids.cambridge.arm.com> (raw) In-Reply-To: <20181002121817.GB2398@lst.de> On Tue, Oct 02, 2018 at 02:18:17PM +0200, Torsten Duwe wrote: > Hi Mark, Hi, > thank you for your very detailed feedback, I'll incorporate it > all into the next version, besides one issue: > > On Tue, Oct 02, 2018 at 12:27:41PM +0100, Mark Rutland wrote: > > > > Please use the insn framework, as we do to generate all the other > > instruction sequences in ftrace. > > > > MOV (register) is an alias of ORR (shifted register), i.e. > > > > mov <xd>, <xm> > > > > ... is: > > > > orr <xd>, xzr, <xm> > > > > ... and we have code to generate ORR, so we can add a trivial wrapper to > > generate MOV. > > I had something similar in v2; but it was hardly any better to read or > understand. My main question however is: how do you justify the runtime > overhead of aarch64_insn_gen_logical_shifted_reg for every function that > gets its tracing switched on or off? How do you justify doing something different from the established pattern? Do you have any numbers indicating that this overhead is a problem on a real workload? For the moment at least, please use aarch64_insn_gen_*(), as we do for all other instructions generated in the ftrace code. It's vastly simpler for everyone if we have consistency here. > The result is always the same 4-byte constant, so why not use a macro > and a comment that says what it does? I'd rather that we stick to the usual pattern that we have in arm64. Note that aarch64_insn_gen_nop() also always returns the same 4-byte constant, but it's an out-of-line function in insn.c. There haven't been any complaints as to its overhead so far... Thanks, Mark.
next prev parent reply other threads:[~2018-10-02 12:57 UTC|newest] Thread overview: 96+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-10-01 14:09 [PATCH v3 0/4] arm64 live patching Torsten Duwe 2018-10-01 14:09 ` Torsten Duwe 2018-10-01 14:16 ` [PATCH v3 1/4] DYNAMIC_FTRACE configurable with and without REGS Torsten Duwe 2018-10-01 14:16 ` Torsten Duwe 2018-10-01 14:52 ` Ard Biesheuvel 2018-10-01 14:52 ` Ard Biesheuvel 2018-10-01 15:03 ` Torsten Duwe 2018-10-01 15:03 ` Torsten Duwe 2018-10-01 15:06 ` Ard Biesheuvel 2018-10-01 15:06 ` Ard Biesheuvel 2018-10-01 15:10 ` Torsten Duwe 2018-10-01 15:10 ` Torsten Duwe 2018-10-01 15:14 ` Steven Rostedt 2018-10-01 15:14 ` Steven Rostedt 2018-10-01 14:16 ` [PATCH v3 2/4] arm64: implement ftrace with regs Torsten Duwe 2018-10-01 14:16 ` Torsten Duwe 2018-10-01 15:57 ` Ard Biesheuvel 2018-10-01 15:57 ` Ard Biesheuvel 2018-10-02 10:02 ` Torsten Duwe 2018-10-02 10:02 ` Torsten Duwe 2018-10-02 10:39 ` Ard Biesheuvel 2018-10-02 10:39 ` Ard Biesheuvel 2018-10-02 11:27 ` Mark Rutland 2018-10-02 11:27 ` Mark Rutland 2018-10-02 12:18 ` Torsten Duwe 2018-10-02 12:18 ` Torsten Duwe 2018-10-02 12:57 ` Mark Rutland [this message] 2018-10-02 12:57 ` Mark Rutland 2018-10-01 14:16 ` [PATCH v3 3/4] arm64: implement live patching Torsten Duwe 2018-10-01 14:16 ` Torsten Duwe 2018-10-17 13:39 ` Miroslav Benes 2018-10-17 13:39 ` Miroslav Benes 2018-10-18 12:58 ` Jessica Yu 2018-10-18 12:58 ` Jessica Yu 2018-10-19 11:59 ` Miroslav Benes 2018-10-19 11:59 ` Miroslav Benes 2018-10-19 12:18 ` Jessica Yu 2018-10-19 12:18 ` Jessica Yu 2018-10-19 15:14 ` Miroslav Benes 2018-10-19 15:14 ` Miroslav Benes 2018-10-19 13:46 ` Torsten Duwe 2018-10-19 13:46 ` Torsten Duwe 2018-10-19 13:52 ` Ard Biesheuvel 2018-10-19 13:52 ` Ard Biesheuvel 2018-10-19 15:21 ` Miroslav Benes 2018-10-19 15:21 ` Miroslav Benes 2018-10-20 14:10 ` Ard Biesheuvel 2018-10-20 14:10 ` Ard Biesheuvel 2018-10-22 12:53 ` Miroslav Benes 2018-10-22 12:53 ` Miroslav Benes 2018-10-22 14:54 ` Torsten Duwe 2018-10-22 14:54 ` Torsten Duwe 2018-10-23 17:55 ` [PATCH] arm64/module: use mod->klp_info section header information Jessica Yu 2018-10-23 17:55 ` Jessica Yu 2018-10-23 19:32 ` kbuild test robot 2018-10-23 19:32 ` kbuild test robot 2018-10-24 11:57 ` Miroslav Benes 2018-10-24 11:57 ` Miroslav Benes 2018-10-25 8:08 ` Petr Mladek 2018-10-25 8:08 ` Petr Mladek 2018-10-25 9:00 ` Miroslav Benes 2018-10-25 9:00 ` Miroslav Benes 2018-10-25 11:42 ` Jessica Yu 2018-10-25 11:42 ` Jessica Yu 2018-10-26 17:25 ` [PATCH v2] arm64/module: use mod->klp_info section header information for livepatch modules Jessica Yu 2018-10-26 17:25 ` Jessica Yu 2018-10-29 13:24 ` Miroslav Benes 2018-10-29 13:24 ` Miroslav Benes 2018-10-29 13:32 ` Jessica Yu 2018-10-29 13:32 ` Jessica Yu 2018-10-29 15:28 ` Will Deacon 2018-10-29 15:28 ` Will Deacon 2018-10-30 13:19 ` Jessica Yu 2018-10-30 13:19 ` Jessica Yu 2018-11-01 15:18 ` Miroslav Benes 2018-11-01 15:18 ` Miroslav Benes 2018-11-01 16:07 ` Will Deacon 2018-11-01 16:07 ` Will Deacon 2018-11-05 12:30 ` Ard Biesheuvel 2018-11-05 12:30 ` Ard Biesheuvel 2018-11-05 17:57 ` [PATCH] arm64/module: use plt section indices for relocations Jessica Yu 2018-11-05 17:57 ` Jessica Yu 2018-11-05 18:04 ` Ard Biesheuvel 2018-11-05 18:04 ` Ard Biesheuvel 2018-11-05 18:53 ` [PATCH v2] " Jessica Yu 2018-11-05 18:53 ` Jessica Yu 2018-11-05 18:56 ` Ard Biesheuvel 2018-11-05 18:56 ` Ard Biesheuvel 2018-11-05 19:26 ` Will Deacon 2018-11-05 19:26 ` Will Deacon 2018-11-05 19:49 ` Jessica Yu 2018-11-05 19:49 ` Jessica Yu 2018-11-06 9:44 ` Miroslav Benes 2018-11-06 9:44 ` Miroslav Benes 2018-10-01 14:16 ` [PATCH v3 4/4] arm64: reliable stacktraces Torsten Duwe 2018-10-01 14:16 ` Torsten Duwe
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=20181002125734.mhkz4o46oxf3mtu6@lakrids.cambridge.arm.com \ --to=mark.rutland@arm.com \ --cc=ard.biesheuvel@linaro.org \ --cc=arnd@arndb.de \ --cc=catalin.marinas@arm.com \ --cc=duwe@lst.de \ --cc=jpoimboe@redhat.com \ --cc=julien.thierry@arm.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=live-patching@vger.kernel.org \ --cc=mingo@redhat.com \ --cc=rostedt@goodmis.org \ --cc=takahiro.akashi@linaro.org \ --cc=will.deacon@arm.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: linkBe 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.