Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1741230 > unrolled thread

Re: [PATCH -tip v3 7/7] kprobes: Use synchronize_rcu_tasks() for optprobe with CONFIG_PREEMPT

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH -tip v3 7/7] kprobes: Use synchronize_rcu_tasks() for  optprobe with CONFIG_PREEMPT Ingo Molnar <mingo@kernel.org> - 2017-09-28 09:30 +0200
    Re: [PATCH -tip v3 7/7] kprobes: Use synchronize_rcu_tasks() for  optprobe with CONFIG_PREEMPT Masami Hiramatsu <mhiramat@kernel.org> - 2017-09-29 09:30 +0200
      Re: [PATCH -tip v3 7/7] kprobes: Use synchronize_rcu_tasks() for  optprobe with CONFIG_PREEMPT Ingo Molnar <mingo@kernel.org> - 2017-09-29 09:40 +0200
        Re: [PATCH -tip v3 7/7] kprobes: Use synchronize_rcu_tasks() for  optprobe with CONFIG_PREEMPT Masami Hiramatsu <mhiramat@kernel.org> - 2017-09-29 16:50 +0200
          Re: [PATCH -tip v3 7/7] kprobes: Use synchronize_rcu_tasks() for  optprobe with CONFIG_PREEMPT Ingo Molnar <mingo@kernel.org> - 2017-09-29 19:50 +0200
            Re: [PATCH -tip v3 7/7] kprobes: Use synchronize_rcu_tasks() for  optprobe with CONFIG_PREEMPT Masami Hiramatsu <mhiramat@kernel.org> - 2017-09-30 07:20 +0200

#1741230 — Re: [PATCH -tip v3 7/7] kprobes: Use synchronize_rcu_tasks() for optprobe with CONFIG_PREEMPT

* 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.

> 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.

> 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

Part after the first comma does not parse.

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

> 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.

> 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.

Also, "To wait for such threads", or "To wait for such a thread".

Thanks,

	Ingo

[toc] | [next] | [standalone]

#1741950

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>

[toc] | [prev] | [next] | [standalone]

#1741951

* Masami Hiramatsu <mhiramat@kernel.org> wrote:

> 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.

What's "synchthnize"? ...

More seriously, I still don't understand it. What is 'back on track'?

Do you mean to say:

   We want to wait for all potentially preempted kprobes trampoline execution to 
   have completed. This guarantees that any freed trampoline memory is not in use
   by any task in the system anymore. synchronize_rcu_tasks() gives such a
   guarantee, so use it.

or something else?

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1742177

On Fri, 29 Sep 2017 09:37:55 +0200
Ingo Molnar <mingo@kernel.org> wrote:

> 
> * Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > 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.
> 
> What's "synchthnize"? ...

Oops, it's my typo. my XPS touch pad is really unstable...

> 
> More seriously, I still don't understand it. What is 'back on track'?
> 
> Do you mean to say:
> 
>    We want to wait for all potentially preempted kprobes trampoline execution to 
>    have completed. This guarantees that any freed trampoline memory is not in use
>    by any task in the system anymore. synchronize_rcu_tasks() gives such a
>    guarantee, so use it.

Exactly, this is correct!

Thank you,

> 
> or something else?
> 
> Thanks,
> 
> 	Ingo


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[toc] | [prev] | [next] | [standalone]

#1742264

* Masami Hiramatsu <mhiramat@kernel.org> wrote:

> On Fri, 29 Sep 2017 09:37:55 +0200
> Ingo Molnar <mingo@kernel.org> wrote:
> 
> > 
> > * Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > 
> > > 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.
> > 
> > What's "synchthnize"? ...
> 
> Oops, it's my typo. my XPS touch pad is really unstable...
> 
> > 
> > More seriously, I still don't understand it. What is 'back on track'?
> > 
> > Do you mean to say:
> > 
> >    We want to wait for all potentially preempted kprobes trampoline execution to 
> >    have completed. This guarantees that any freed trampoline memory is not in use
> >    by any task in the system anymore. synchronize_rcu_tasks() gives such a
> >    guarantee, so use it.
> 
> Exactly, this is correct!

Ok, great - please re-send the remaining kprobes patches that I have not applied 
yet - I'll read through the changelogs and fix any bits that might still be 
unclear.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1742505

On Fri, 29 Sep 2017 19:45:28 +0200
Ingo Molnar <mingo@kernel.org> wrote:

> 
> * Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > On Fri, 29 Sep 2017 09:37:55 +0200
> > Ingo Molnar <mingo@kernel.org> wrote:
> > 
> > > 
> > > * Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > > 
> > > > 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.
> > > 
> > > What's "synchthnize"? ...
> > 
> > Oops, it's my typo. my XPS touch pad is really unstable...
> > 
> > > 
> > > More seriously, I still don't understand it. What is 'back on track'?
> > > 
> > > Do you mean to say:
> > > 
> > >    We want to wait for all potentially preempted kprobes trampoline execution to 
> > >    have completed. This guarantees that any freed trampoline memory is not in use
> > >    by any task in the system anymore. synchronize_rcu_tasks() gives such a
> > >    guarantee, so use it.
> > 
> > Exactly, this is correct!
> 
> Ok, great - please re-send the remaining kprobes patches that I have not applied 
> yet - I'll read through the changelogs and fix any bits that might still be 
> unclear.

OK, I got it. I'll check the remaining patches!

Thank you!

> 
> Thanks,
> 
> 	Ingo


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web