Message-ID: <1295963775.28776.1056.camel@laptop>
Date:	Tue, 25 Jan 2011 14:56:15 +0100
From:	Peter Zijlstra <peterz@...radead.org>
To:	Srikar Dronamraju <srikar@...ux.vnet.ibm.com>
Cc:	Ingo Molnar <mingo@...e.hu>, Steven Rostedt <rostedt@...dmis.org>,
	Arnaldo Carvalho de Melo <acme@...radead.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	Christoph Hellwig <hch@...radead.org>,
	Andi Kleen <andi@...stfloor.org>,
	Oleg Nesterov <oleg@...hat.com>,
	LKML <linux-kernel@...r.kernel.org>,
	SystemTap <systemtap@...rces.redhat.com>,
	Linux-mm <linux-mm@...r.kernel.org>,
	Jim Keniston <jkenisto@...ux.vnet.ibm.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Ananth N Mavinakayanahalli <ananth@...ibm.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Subject: Re: [RFC] [PATCH 2.6.37-rc5-tip 10/20] 10: uprobes: task specific
 information.

On Thu, 2010-12-16 at 15:29 +0530, Srikar Dronamraju wrote:
> Uprobes needs to maintain some task specific information include if a
> task is currently uprobed, the currently handing uprobe, any arch
> specific information (for example to handle rip relative instructions),
> the per-task slot where the original instruction is copied to before
> single-stepping.

This can go away once you have per-task xol slots and boosted probes,
because then you can write the complete replacement sequence on trap and
never need to come back until you hit another probe, right?

> +/*
> + * uprobe_utask -- not a user-visible struct.
> + * Corresponds to a thread in a probed process.
> + * Guarded by uproc->mutex.
> + */
> +struct uprobe_task {
> +	unsigned long xol_vaddr;
> +	unsigned long vaddr;
> +
> +	enum uprobe_task_state state;
> +	struct uprobe_task_arch_info tskinfo;
> +
> +	struct uprobe *active_uprobe;
> +};

So xol_vaddr is the start of the xol slot,
vaddr is the trap address, we store those so that you still have the
state during the single-step things?

I guess you could obtain the xol slot information from the IP during
single-step, but since you have storage anyway, this might be cheaper.

And the active_probe is again due to single-step, right? Why exactly do
you need that? If you trap, acquire a new slot, write the replacement
sequence, single step through it, and release the slot once you're back
to the original code stream. I'm not quite seeing where you need the
probe during stepping.

Ah, I think I found it while reading patch 13, you need the pre/post_xol
callbacks, can't you simply synthesize their effect into the replacement
sequence?

  push %rax
  mov $vaddr, %rax
  $INSN
  pop %rax
  jmp $next_insn

like replacements would obviate the need for the pre/post callbacks and
allow you to run straight through.

It doesn't look too hard to create simple sequences for each
UPROBE_FIX_* thingy:

pre:
  push %rax; mov $vaddr, %rax && UPROBE_FIX_RIP_AX
  push %rcx; mov $vaddr, %rcx && UPROBE_FIX_RIP_CX

INSN

post:
  pop %rax && UPROBE_FIX_RIP_AX
  pop %rcx && UPROBE_FIX_RIP_CX
  add $correction, $offset(%rsp) && UPROBE_FIX_CALL
  jmp $next_insn

you already have all the logic of computing the various constants there.
And your slots are 128bytes long, which should fit sequences like that
just fine I think.

It would also remove the whole single-step need since they're proper
boosted probes.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists