From: rostedt at goodmis.org (Steven Rostedt) Subject: [RFC][PATCH 1/2] x86: Allow breakpoints to emulate call functions Date: Mon, 6 May 2019 21:04:16 -0400 [thread overview] Message-ID: <20190506210416.2489a659@oasis.local.home> (raw) In-Reply-To: <CAHk-=wj3R_s0RTJOmTBNaUPhu4fz2shNBUr4M6Ej65UYSNCs-g@mail.gmail.com> On Mon, 6 May 2019 15:06:57 -0700 Linus Torvalds <torvalds at linux-foundation.org> wrote: > On Mon, May 6, 2019 at 2:45 PM Steven Rostedt <rostedt at goodmis.org> wrote: > > > > To do that we would need to rewrite the logic to update each of those > > 40,000 calls one at a time, or group them together to what gets > > changed. > > Stephen, YOU ARE NOT LISTENING. (note, it's Steven ;-) I'm listening, I'm just trying to understand. > > You are already fixing the value of the call in the instruction as > part of the instruction rewriting. > > When you do things like this: > > unsigned long ip = (unsigned long)(&ftrace_call); > unsigned char *new; > int ret; > > new = ftrace_call_replace(ip, (unsigned long)func); > ret = update_ftrace_func(ip, new); > > you have already decided to rewrite the instruction with one single > fixed call target: "func". > > I'm just saying that you should ALWAYS use the same call target in the > int3 emulation. > > Instead, you hardcode something else than what you are AT THE SAME > TIME rewriting the instruction with. > > See what I'm saying? Yes, but that's not the code I'm worried about. ftrace_replace_code() is, which does: for_ftrace_rec_iter(iter) { rec = ftrace_rec_iter_record(iter); ret = add_breakpoints(rec, enable); if (ret) goto remove_breakpoints; count++; } run_sync(); And there's two more iterator loops that will do the modification of the call site, and then the third loop will remove the breakpoint. That iterator does something special for each individual record. All 40,000 of them. That add_breakpoint() does: static int add_breakpoints(struct dyn_ftrace *rec, int enable) { unsigned long ftrace_addr; int ret; ftrace_addr = ftrace_get_addr_curr(rec); ret = ftrace_test_record(rec, enable); switch (ret) { case FTRACE_UPDATE_IGNORE: return 0; case FTRACE_UPDATE_MAKE_CALL: /* converting nop to call */ return add_brk_on_nop(rec); case FTRACE_UPDATE_MODIFY_CALL: case FTRACE_UPDATE_MAKE_NOP: /* converting a call to a nop */ return add_brk_on_call(rec, ftrace_addr); } return 0; } And to get what the target is, we call ftrace_get_addr_curr(), which will return a function based on the flags in the record. Which can be anything from a call to a customized trampoline, to either ftrace_caller, or to ftrace_regs_caller, or it can turn the record into a nop. This is what I'm talking about. We are adding thousands of int3s through out the kernel, and we have a single handler to handle each one of them. The reason I picked ftrace_regs_caller() is because that one does anything that any of the callers can do (albeit slower). If it does not, then ftrace will be broken, because it handles the case that all types of trampolines are attached to a single function, and that code had better do the exact same thing for each of those trampolines as if the trampolines were called directly, because the handlers that those trampolines call, shouldn't care who else is using that function. Note, the only exception to that rule, is that we only allow one function attached to the function to modify the return address (and the record has a flag for that). If a record already modifies the ip address on return, the registering of another ftrace_ops that modifies the ip address will fail to register. > > Stop with the "there could be thousands of targets" arguyment. The > "call" instruction THAT YOU ARE REWRITING has exactly one target. > There aren't 40,000 of them. x86 does not have that kind of "call" > instruction that randomly calls 40k different functions. You are > replacing FIVE BYTES of memory, and the emulation you do should > emulate those FIVE BYTES. > > See? > > Why are you emulating something different than what you are rewriting? I'm not having one call site call 40,000 different functions. You are right about that. But I have 40,000 different call sites that could be calling different functions and all of them are being processed by a single int3 handler. That's my point. -- Steve
WARNING: multiple messages have this Message-ID (diff)
From: rostedt@goodmis.org (Steven Rostedt) Subject: [RFC][PATCH 1/2] x86: Allow breakpoints to emulate call functions Date: Mon, 6 May 2019 21:04:16 -0400 [thread overview] Message-ID: <20190506210416.2489a659@oasis.local.home> (raw) Message-ID: <20190507010416.U6SfApq82mBqIw19uFa4vUackncaSj-Z7XnOyzbpZho@z> (raw) In-Reply-To: <CAHk-=wj3R_s0RTJOmTBNaUPhu4fz2shNBUr4M6Ej65UYSNCs-g@mail.gmail.com> On Mon, 6 May 2019 15:06:57 -0700 Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Mon, May 6, 2019@2:45 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > > > To do that we would need to rewrite the logic to update each of those > > 40,000 calls one at a time, or group them together to what gets > > changed. > > Stephen, YOU ARE NOT LISTENING. (note, it's Steven ;-) I'm listening, I'm just trying to understand. > > You are already fixing the value of the call in the instruction as > part of the instruction rewriting. > > When you do things like this: > > unsigned long ip = (unsigned long)(&ftrace_call); > unsigned char *new; > int ret; > > new = ftrace_call_replace(ip, (unsigned long)func); > ret = update_ftrace_func(ip, new); > > you have already decided to rewrite the instruction with one single > fixed call target: "func". > > I'm just saying that you should ALWAYS use the same call target in the > int3 emulation. > > Instead, you hardcode something else than what you are AT THE SAME > TIME rewriting the instruction with. > > See what I'm saying? Yes, but that's not the code I'm worried about. ftrace_replace_code() is, which does: for_ftrace_rec_iter(iter) { rec = ftrace_rec_iter_record(iter); ret = add_breakpoints(rec, enable); if (ret) goto remove_breakpoints; count++; } run_sync(); And there's two more iterator loops that will do the modification of the call site, and then the third loop will remove the breakpoint. That iterator does something special for each individual record. All 40,000 of them. That add_breakpoint() does: static int add_breakpoints(struct dyn_ftrace *rec, int enable) { unsigned long ftrace_addr; int ret; ftrace_addr = ftrace_get_addr_curr(rec); ret = ftrace_test_record(rec, enable); switch (ret) { case FTRACE_UPDATE_IGNORE: return 0; case FTRACE_UPDATE_MAKE_CALL: /* converting nop to call */ return add_brk_on_nop(rec); case FTRACE_UPDATE_MODIFY_CALL: case FTRACE_UPDATE_MAKE_NOP: /* converting a call to a nop */ return add_brk_on_call(rec, ftrace_addr); } return 0; } And to get what the target is, we call ftrace_get_addr_curr(), which will return a function based on the flags in the record. Which can be anything from a call to a customized trampoline, to either ftrace_caller, or to ftrace_regs_caller, or it can turn the record into a nop. This is what I'm talking about. We are adding thousands of int3s through out the kernel, and we have a single handler to handle each one of them. The reason I picked ftrace_regs_caller() is because that one does anything that any of the callers can do (albeit slower). If it does not, then ftrace will be broken, because it handles the case that all types of trampolines are attached to a single function, and that code had better do the exact same thing for each of those trampolines as if the trampolines were called directly, because the handlers that those trampolines call, shouldn't care who else is using that function. Note, the only exception to that rule, is that we only allow one function attached to the function to modify the return address (and the record has a flag for that). If a record already modifies the ip address on return, the registering of another ftrace_ops that modifies the ip address will fail to register. > > Stop with the "there could be thousands of targets" arguyment. The > "call" instruction THAT YOU ARE REWRITING has exactly one target. > There aren't 40,000 of them. x86 does not have that kind of "call" > instruction that randomly calls 40k different functions. You are > replacing FIVE BYTES of memory, and the emulation you do should > emulate those FIVE BYTES. > > See? > > Why are you emulating something different than what you are rewriting? I'm not having one call site call 40,000 different functions. You are right about that. But I have 40,000 different call sites that could be calling different functions and all of them are being processed by a single int3 handler. That's my point. -- Steve
next prev parent reply other threads:[~2019-05-07 1:04 UTC|newest] Thread overview: 204+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <20190501202830.347656894@goodmis.org> 2019-05-01 20:28 ` [RFC][PATCH 1/2] x86: Allow breakpoints to emulate call functions rostedt 2019-05-01 20:28 ` Steven Rostedt 2019-05-02 3:24 ` rostedt 2019-05-02 3:24 ` Steven Rostedt 2019-05-02 16:21 ` peterz 2019-05-02 16:21 ` Peter Zijlstra 2019-05-02 16:29 ` peterz 2019-05-02 16:29 ` Peter Zijlstra 2019-05-02 18:02 ` torvalds 2019-05-02 18:02 ` Linus Torvalds 2019-05-02 18:18 ` peterz 2019-05-02 18:18 ` Peter Zijlstra 2019-05-02 18:30 ` peterz 2019-05-02 18:30 ` Peter Zijlstra 2019-05-02 18:43 ` torvalds 2019-05-02 18:43 ` Linus Torvalds 2019-05-02 19:28 ` jikos 2019-05-02 19:28 ` Jiri Kosina 2019-05-02 20:25 ` luto 2019-05-02 20:25 ` Andy Lutomirski 2019-05-02 20:21 ` peterz 2019-05-02 20:21 ` Peter Zijlstra 2019-05-02 20:49 ` torvalds 2019-05-02 20:49 ` Linus Torvalds 2019-05-02 21:32 ` peterz 2019-05-02 21:32 ` Peter Zijlstra 2019-05-03 19:24 ` rostedt 2019-05-03 19:24 ` Steven Rostedt 2019-05-03 21:46 ` torvalds 2019-05-03 21:46 ` Linus Torvalds 2019-05-03 22:49 ` rostedt 2019-05-03 22:49 ` Steven Rostedt 2019-05-03 23:07 ` torvalds 2019-05-03 23:07 ` Linus Torvalds 2019-05-04 4:17 ` rostedt 2019-05-04 4:17 ` Steven Rostedt [not found] ` <CAHk-=wiuSFbv_rELND-BLWcP0GSZ0yF=xOAEcf61GE3bU9d=yg@mail.gmail.com> 2019-05-04 18:59 ` torvalds 2019-05-04 18:59 ` Linus Torvalds 2019-05-04 20:12 ` luto 2019-05-04 20:12 ` Andy Lutomirski 2019-05-04 20:28 ` torvalds 2019-05-04 20:28 ` Linus Torvalds 2019-05-04 20:36 ` torvalds 2019-05-04 20:36 ` Linus Torvalds 2019-05-03 22:55 ` luto 2019-05-03 22:55 ` Andy Lutomirski 2019-05-03 23:16 ` torvalds 2019-05-03 23:16 ` Linus Torvalds 2019-05-03 23:32 ` luto 2019-05-03 23:32 ` Andy Lutomirski 2019-05-02 22:52 ` rostedt 2019-05-02 22:52 ` Steven Rostedt 2019-05-02 23:31 ` rostedt 2019-05-02 23:31 ` Steven Rostedt 2019-05-02 23:50 ` rostedt 2019-05-02 23:50 ` Steven Rostedt 2019-05-03 1:51 ` [RFC][PATCH 1/2 v2] " rostedt 2019-05-03 1:51 ` Steven Rostedt 2019-05-03 9:29 ` [RFC][PATCH 1/2] " peterz 2019-05-03 9:29 ` Peter Zijlstra 2019-05-03 13:22 ` rostedt 2019-05-03 13:22 ` Steven Rostedt 2019-05-03 16:20 ` luto 2019-05-03 16:20 ` Andy Lutomirski 2019-05-03 16:31 ` rostedt 2019-05-03 16:31 ` Steven Rostedt 2019-05-03 16:35 ` peterz 2019-05-03 16:35 ` Peter Zijlstra 2019-05-03 16:44 ` luto 2019-05-03 16:44 ` Andy Lutomirski 2019-05-03 16:49 ` rostedt 2019-05-03 16:49 ` Steven Rostedt 2019-05-03 16:32 ` peterz 2019-05-03 16:32 ` Peter Zijlstra 2019-05-03 18:57 ` torvalds 2019-05-03 18:57 ` Linus Torvalds 2019-05-06 8:19 ` peterz 2019-05-06 8:19 ` Peter Zijlstra 2019-05-06 13:56 ` rostedt 2019-05-06 13:56 ` Steven Rostedt 2019-05-06 16:17 ` torvalds 2019-05-06 16:17 ` Linus Torvalds 2019-05-06 16:19 ` torvalds 2019-05-06 16:19 ` Linus Torvalds 2019-05-06 17:06 ` rostedt 2019-05-06 17:06 ` Steven Rostedt 2019-05-06 18:06 ` torvalds 2019-05-06 18:06 ` Linus Torvalds 2019-05-06 18:57 ` rostedt 2019-05-06 18:57 ` Steven Rostedt 2019-05-06 19:46 ` torvalds 2019-05-06 19:46 ` Linus Torvalds 2019-05-06 20:29 ` rostedt 2019-05-06 20:29 ` Steven Rostedt 2019-05-06 20:42 ` torvalds 2019-05-06 20:42 ` Linus Torvalds 2019-05-06 20:44 ` torvalds 2019-05-06 20:44 ` Linus Torvalds 2019-05-06 21:45 ` rostedt 2019-05-06 21:45 ` Steven Rostedt 2019-05-06 22:06 ` torvalds 2019-05-06 22:06 ` Linus Torvalds 2019-05-06 22:31 ` torvalds 2019-05-06 22:31 ` Linus Torvalds 2019-05-07 0:10 ` rostedt 2019-05-07 0:10 ` Steven Rostedt 2019-05-07 1:06 ` torvalds 2019-05-07 1:06 ` Linus Torvalds 2019-05-07 1:04 ` rostedt [this message] 2019-05-07 1:04 ` Steven Rostedt 2019-05-07 1:34 ` rostedt 2019-05-07 1:34 ` Steven Rostedt 2019-05-07 1:34 ` torvalds 2019-05-07 1:34 ` Linus Torvalds 2019-05-07 1:53 ` rostedt 2019-05-07 1:53 ` Steven Rostedt 2019-05-07 2:22 ` torvalds 2019-05-07 2:22 ` Linus Torvalds 2019-05-07 2:58 ` rostedt 2019-05-07 2:58 ` Steven Rostedt 2019-05-07 3:05 ` torvalds 2019-05-07 3:05 ` Linus Torvalds 2019-05-07 3:21 ` rostedt 2019-05-07 3:21 ` Steven Rostedt 2019-05-07 3:28 ` torvalds 2019-05-07 3:28 ` Linus Torvalds 2019-05-07 14:54 ` torvalds 2019-05-07 14:54 ` Linus Torvalds 2019-05-07 15:12 ` rostedt 2019-05-07 15:12 ` Steven Rostedt 2019-05-07 15:25 ` rostedt 2019-05-07 15:25 ` Steven Rostedt 2019-05-07 16:25 ` rostedt 2019-05-07 16:25 ` Steven Rostedt 2019-05-07 15:31 ` torvalds 2019-05-07 15:31 ` Linus Torvalds 2019-05-07 15:45 ` rostedt 2019-05-07 15:45 ` Steven Rostedt 2019-05-07 16:34 ` peterz 2019-05-07 16:34 ` Peter Zijlstra 2019-05-07 17:08 ` torvalds 2019-05-07 17:08 ` Linus Torvalds 2019-05-07 17:21 ` jpoimboe 2019-05-07 17:21 ` Josh Poimboeuf 2019-05-07 21:24 ` rostedt 2019-05-07 21:24 ` Steven Rostedt 2019-05-08 4:50 ` torvalds 2019-05-08 4:50 ` Linus Torvalds 2019-05-08 16:37 ` rostedt 2019-05-08 16:37 ` Steven Rostedt 2019-05-07 17:38 ` peterz 2019-05-07 17:38 ` Peter Zijlstra 2019-05-07 9:51 ` peterz 2019-05-07 9:51 ` Peter Zijlstra 2019-05-07 14:48 ` luto 2019-05-07 14:48 ` Andy Lutomirski 2019-05-07 14:57 ` torvalds 2019-05-07 14:57 ` Linus Torvalds 2019-05-07 14:13 ` mhiramat 2019-05-07 14:13 ` Masami Hiramatsu 2019-05-07 17:15 ` mhiramat 2019-05-07 17:15 ` Masami Hiramatsu 2019-05-06 14:22 ` peterz 2019-05-06 14:22 ` Peter Zijlstra 2019-05-07 8:57 ` peterz 2019-05-07 8:57 ` Peter Zijlstra 2019-05-07 9:18 ` David.Laight 2019-05-07 9:18 ` David Laight 2019-05-07 11:30 ` peterz 2019-05-07 11:30 ` Peter Zijlstra 2019-05-07 12:57 ` David.Laight 2019-05-07 12:57 ` David Laight 2019-05-07 13:14 ` rostedt 2019-05-07 13:14 ` Steven Rostedt 2019-05-07 14:50 ` David.Laight 2019-05-07 14:50 ` David Laight 2019-05-07 14:57 ` rostedt 2019-05-07 14:57 ` Steven Rostedt 2019-05-07 15:46 ` David.Laight 2019-05-07 15:46 ` David Laight 2019-05-07 13:32 ` peterz 2019-05-07 13:32 ` Peter Zijlstra 2019-05-07 9:27 ` peterz 2019-05-07 9:27 ` Peter Zijlstra 2019-05-07 12:27 ` rostedt 2019-05-07 12:27 ` Steven Rostedt 2019-05-07 12:41 ` peterz 2019-05-07 12:41 ` Peter Zijlstra 2019-05-07 12:54 ` rostedt 2019-05-07 12:54 ` Steven Rostedt 2019-05-07 17:22 ` masami.hiramatsu 2019-05-07 17:22 ` Masami Hiramatsu 2019-05-07 14:28 ` peterz 2019-05-07 14:28 ` Peter Zijlstra 2019-05-02 20:48 ` rostedt 2019-05-02 20:48 ` Steven Rostedt 2019-05-06 15:14 ` jpoimboe 2019-05-06 15:14 ` Josh Poimboeuf 2019-05-01 20:28 ` [RFC][PATCH 2/2] ftrace/x86: Emulate call function while updating in breakpoint handler rostedt 2019-05-01 20:28 ` Steven Rostedt 2019-05-03 10:22 ` [RFC][PATCH 1.5/2] x86: Add int3_emulate_call() selftest peterz 2019-05-03 10:22 ` Peter Zijlstra 2019-05-03 18:46 ` rostedt 2019-05-03 18:46 ` 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=20190506210416.2489a659@oasis.local.home \ --to=linux-kselftest@vger.kernel.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: linkBe 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).