Messages in this thread | | | Date | Tue, 28 Feb 2012 11:52:50 +0900 | From | Masami Hiramatsu <> | Subject | Re: Re: [PATCH v3 -tip] [BUGFIX] x86/kprobes: Fix to recover instructions on optimized path |
| |
(2012/02/27 18:34), Ingo Molnar wrote: > > * Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote: > >> + >> +#ifdef CONFIG_OPTPROBES >> +static unsigned long __recover_optprobed_insn(struct kprobe *kp, >> + kprobe_opcode_t *buf, >> + unsigned long addr) >> +{ >> + long offs = addr - (unsigned long)kp->addr - 1; >> + struct optimized_kprobe *op = container_of(kp, struct optimized_kprobe, kp); >> + >> + /* >> + * If the kprobe can be optimized, original bytes which can be >> + * overwritten by jump destination address. In this case, original >> + * bytes must be recovered from op->optinsn.copied_insn buffer. >> + */ >> + memcpy(buf, (void *)addr, MAX_INSN_SIZE * sizeof(kprobe_opcode_t)); >> + if (addr == (unsigned long)kp->addr) { >> + buf[0] = kp->opcode; >> + memcpy(buf + 1, op->optinsn.copied_insn, RELATIVE_ADDR_SIZE); >> + } else >> + memcpy(buf, op->optinsn.copied_insn + offs, RELATIVE_ADDR_SIZE - offs); >> + >> + return (unsigned long)buf; >> +} >> +#endif > > Why not stick this into a new kprobes-opt.c file?
Would you mean that I should split all optprobe stuffs into new file?
> >> + >> +/* >> + * Recover the probed instruction at addr for further analysis. >> + * Caller must lock kprobes by kprobe_mutex, or disable preemption >> + * for preventing to release referencing kprobes. >> + */ >> +static unsigned long recover_probed_instruction(kprobe_opcode_t *buf, >> + unsigned long addr) >> +{ >> + struct kprobe *kp; >> +#ifdef CONFIG_OPTPROBES >> + int i; >> + >> + for (i = 0; i < RELATIVEJUMP_SIZE; i++) { >> + kp = get_kprobe((void *)addr - i); >> + if (kp && kprobe_optready(kp)) >> + return __recover_optprobed_insn(kp, buf, addr); >> + } >> +#endif > > This should be a separate, kprobes_recover_opt() method and be > inside kprobes-opt.c as well.
OK, I'll do that. But I think it should be separated work. Just for the bugfix, I think this should go into this style, because this should be pushed into stable tree too.
Thank you,
-- Masami HIRAMATSU Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@hitachi.com
| |