Messages in this thread | | | Date | Fri, 29 Sep 2017 00:29:38 -0700 | From | Masami Hiramatsu <> | Subject | Re: [PATCH -tip v3 7/7] kprobes: Use synchronize_rcu_tasks() for optprobe with CONFIG_PREEMPT |
| |
On Thu, 28 Sep 2017 09:22:20 +0200 Ingo Molnar <mingo@kernel.org> wrote:
> > * Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > To enable jump optimized probe with CONFIG_PREEMPT, use > > synchronize_rcu_tasks() to wait for all tasks preempted > > on trampoline code back on track. > > This sentence does not parse. It's missing a verb, but I'm not sure.
Hmm, how about this?
Use synchthnize_rcu_tasks() to wait for all tasks preempted on trampoline code back on track so that jump optimized probe can be enabled with CONFIG_PREEMPT.
> > > Since the jump optimized kprobes can replace multiple > > instructions, there can be tasks which are preempted > > on the 2nd (or 3rd) instructions. If the kprobe > > replaces those instructions by a jump instruction, > > when those tasks back to the preempted place, it is > > a middle of the jump instruction and causes a kernel > > panic. > > > Again, sentence appears to be missing a verb and also an adjective I think. >
Hmm, I couldn't understand, I think you are pointing below sentence, ---- If the kprobe replaces those instructions by a jump instruction, when those tasks back to the preempted place, it is a middle of the jump instruction and causes a kernel panic. ----
Of course "If" and "when" look ugly, but both have verb...
> > To avoid such tragedies in advance, kprobe optimizer > > prepare a detour route using normal kprobe (e.g. > > int3 breakpoint on x86), and wait for the tasks which > > is interrrupted on such place by synchronize_sched() > > when CONFIG_PREEMPT=n. > > s/tragedies/mishaps
I got it.
> > Part after the first comma does not parse.
Yeah, some typos, but
kprobe optimizer prepares a detour route using normal kprobe () and waits for the tasks, which is interrupted on such place, by synchronize_sched(), when CONFIG_PREEMPT=n.
will be able to parsed. ( at least google translate can ...)
> > Also the way to refer to kprobes is "kprobes" and "normal kprobes". > Use 'kprobe' only when talking about a specific kprobe instance or such. > You use this correctly later on in the changelog ... > > > If CONFIG_PREEMPT=y, things be more complicated, because > > s/be/are or s/be/get
thanks, get is preferred :)
> > > such interrupted thread can be preempted (other thread > > can be scheduled in interrupt handler.) So, kprobes > > full stop in the wrong place. > > > optimizer has to wait for those tasks scheduled normally. > > missing verb.
kprobe optimizer must wait for those ...
will it work?
> > > In this case we can use synchronize_rcu_tasks() which > > ensures that all preempted tasks back on track and > > schedule it. > > More careful changelogs please. > > > + * are done. Because optprobe may modify multiple instructions, > > + * there is a chance that the Nth instruction is interrupted. In that > > + * case, running interrupt can return to the Nth byte of jump > > + * instruction. This can be avoided by waiting for returning of > > + * such interrupts, since (until here) the first byte of the optimized > > + * probe is already replaced with normal kprobe (sw breakpoint) and > > + * all threads which reach to the probed address will hit it and > > + * bypass the copied instructions (instead of executing the original.) > > + * With CONFIG_PREEMPT, such interrupts can be preepmted. To wait > > + * for such thread, we will use synchronize_rcu_tasks() which ensures > > + * all preeempted tasks are scheduled normally (not preempted). > > + * So we can ensure there is no threads preempted at probed address. > > What? Interrupts cannot be preempted.
Steve, could you correct me if I'm wrong. I thought if the kernel is compiled with CONFIG_PREEMPT=y, even in the kernel, it can be preempted suddenly. It means timer interrupt occurs at kernel path and it yield to new task (=preempt.) Do I miss something?
> > Also, "To wait for such threads", or "To wait for such a thread".
OK,
Thank you,
> > Thanks, > > Ingo
-- Masami Hiramatsu <mhiramat@kernel.org>
| |