Re: [PATCH] uprobes: don't enable/disable signle step if the user did it

2012-07-31 Thread Oleg Nesterov
On 07/31, Srikar Dronamraju wrote: > > * Oleg Nesterov [2012-07-30 16:16:38]: > > > So I think we need arch_uprobe_*able_step(struct uprobe_task *utask). > > Ignoring all problems except the one this patch tries to fix, x86 > > can simply do: > > > > arch_uprobe_enble_step(utask, struct

Re: [PATCH] uprobes: don't enable/disable signle step if the user did it

2012-07-31 Thread Oleg Nesterov
On 07/31, Srikar Dronamraju wrote: * Oleg Nesterov o...@redhat.com [2012-07-30 16:16:38]: So I think we need arch_uprobe_*able_step(struct uprobe_task *utask). Ignoring all problems except the one this patch tries to fix, x86 can simply do: arch_uprobe_enble_step(utask, struct

Re: [PATCH] uprobes: don't enable/disable signle step if the user did it

2012-07-30 Thread Srikar Dronamraju
* Oleg Nesterov [2012-07-30 16:16:38]: > On 07/30, Ananth N Mavinakayanahalli wrote: > > > > On Thu, Jul 26, 2012 at 05:20:43PM +0200, Sebastian Andrzej Siewior wrote: > > > If someone is using single stepping over uprobe brackpoint then after > > > we pass the uprobe single step, single

Re: [PATCH] uprobes: don't enable/disable signle step if the user did it

2012-07-30 Thread Ananth N Mavinakayanahalli
On Mon, Jul 30, 2012 at 04:16:38PM +0200, Oleg Nesterov wrote: > On 07/30, Ananth N Mavinakayanahalli wrote: > > > > On Thu, Jul 26, 2012 at 05:20:43PM +0200, Sebastian Andrzej Siewior wrote: > > > If someone is using single stepping over uprobe brackpoint then after > > > we pass the uprobe

Re: [PATCH] uprobes: don't enable/disable signle step if the user did it

2012-07-30 Thread Sebastian Andrzej Siewior
On 07/30/2012 04:16 PM, Oleg Nesterov wrote: I'd suggest you to make 2 patches: - 1/2 creates arch_uprobe_*_step(...) __weak helpers in kernel/events/uprobes.c which simply call user_*_single_step() and updates the callers Not strictly

Re: [PATCH] uprobes: don't enable/disable signle step if the user did it

2012-07-30 Thread Oleg Nesterov
On 07/30, Ananth N Mavinakayanahalli wrote: > > On Thu, Jul 26, 2012 at 05:20:43PM +0200, Sebastian Andrzej Siewior wrote: > > If someone is using single stepping over uprobe brackpoint then after > > we pass the uprobe single step, single stepping is disabled and the user > > who enebaled them in

Re: [PATCH] uprobes: don't enable/disable signle step if the user did it

2012-07-30 Thread Ananth N Mavinakayanahalli
On Thu, Jul 26, 2012 at 05:20:43PM +0200, Sebastian Andrzej Siewior wrote: > If someone is using single stepping over uprobe brackpoint then after > we pass the uprobe single step, single stepping is disabled and the user > who enebaled them in the first place does not know anything about this. >

Re: [PATCH] uprobes: don't enable/disable signle step if the user did it

2012-07-30 Thread Ananth N Mavinakayanahalli
On Thu, Jul 26, 2012 at 05:20:43PM +0200, Sebastian Andrzej Siewior wrote: If someone is using single stepping over uprobe brackpoint then after we pass the uprobe single step, single stepping is disabled and the user who enebaled them in the first place does not know anything about this.

Re: [PATCH] uprobes: don't enable/disable signle step if the user did it

2012-07-30 Thread Oleg Nesterov
On 07/30, Ananth N Mavinakayanahalli wrote: On Thu, Jul 26, 2012 at 05:20:43PM +0200, Sebastian Andrzej Siewior wrote: If someone is using single stepping over uprobe brackpoint then after we pass the uprobe single step, single stepping is disabled and the user who enebaled them in the

Re: [PATCH] uprobes: don't enable/disable signle step if the user did it

2012-07-30 Thread Sebastian Andrzej Siewior
On 07/30/2012 04:16 PM, Oleg Nesterov wrote: I'd suggest you to make 2 patches: - 1/2 creates arch_uprobe_*_step(...) __weak helpers in kernel/events/uprobes.c which simply call user_*_single_step() and updates the callers Not strictly

Re: [PATCH] uprobes: don't enable/disable signle step if the user did it

2012-07-30 Thread Ananth N Mavinakayanahalli
On Mon, Jul 30, 2012 at 04:16:38PM +0200, Oleg Nesterov wrote: On 07/30, Ananth N Mavinakayanahalli wrote: On Thu, Jul 26, 2012 at 05:20:43PM +0200, Sebastian Andrzej Siewior wrote: If someone is using single stepping over uprobe brackpoint then after we pass the uprobe single step,

Re: [PATCH] uprobes: don't enable/disable signle step if the user did it

2012-07-30 Thread Srikar Dronamraju
* Oleg Nesterov o...@redhat.com [2012-07-30 16:16:38]: On 07/30, Ananth N Mavinakayanahalli wrote: On Thu, Jul 26, 2012 at 05:20:43PM +0200, Sebastian Andrzej Siewior wrote: If someone is using single stepping over uprobe brackpoint then after we pass the uprobe single step, single

Re: [PATCH] uprobes: don't enable/disable signle step if the user did it

2012-07-27 Thread Oleg Nesterov
On 07/27, Sebastian Andrzej Siewior wrote: > > On 07/26/2012 07:31 PM, Oleg Nesterov wrote: >> Well. I agree, this needs changes. To begin with, uprobe should avoid >> user_enable_single_step() which does access_process_vm(). And I suspect >> uprobes have the problems with TIF_FORCED_TF logic. > >

Re: [PATCH] uprobes: don't enable/disable signle step if the user did it

2012-07-27 Thread Sebastian Andrzej Siewior
On 07/26/2012 07:31 PM, Oleg Nesterov wrote: Well. I agree, this needs changes. To begin with, uprobe should avoid user_enable_single_step() which does access_process_vm(). And I suspect uprobes have the problems with TIF_FORCED_TF logic. Why? Shouldn't wee keep the trap flag if the

Re: [PATCH] uprobes: don't enable/disable signle step if the user did it

2012-07-27 Thread Sebastian Andrzej Siewior
On 07/26/2012 07:31 PM, Oleg Nesterov wrote: Well. I agree, this needs changes. To begin with, uprobe should avoid user_enable_single_step() which does access_process_vm(). And I suspect uprobes have the problems with TIF_FORCED_TF logic. Why? Shouldn't wee keep the trap flag if the

Re: [PATCH] uprobes: don't enable/disable signle step if the user did it

2012-07-27 Thread Oleg Nesterov
On 07/27, Sebastian Andrzej Siewior wrote: On 07/26/2012 07:31 PM, Oleg Nesterov wrote: Well. I agree, this needs changes. To begin with, uprobe should avoid user_enable_single_step() which does access_process_vm(). And I suspect uprobes have the problems with TIF_FORCED_TF logic. Why?

Re: [PATCH] uprobes: don't enable/disable signle step if the user did it

2012-07-26 Thread Oleg Nesterov
Well. I agree, this needs changes. To begin with, uprobe should avoid user_enable_single_step() which does access_process_vm(). And I suspect uprobes have the problems with TIF_FORCED_TF logic. But I am not sure about this patch... On 07/26, Sebastian Andrzej Siewior wrote: > > @@ -1528,7

Re: [PATCH] uprobes: don't enable/disable signle step if the user did it

2012-07-26 Thread Sebastian Andrzej Siewior
On 07/26/2012 05:20 PM, Sebastian Andrzej Siewior wrote: diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c index f935327..772eb3a 100644 --- a/kernel/events/uprobes.c +++ b/kernel/events/uprobes.c @@ -1528,7 +1528,10 @@ static void handle_swbp(struct pt_regs *regs)

[PATCH] uprobes: don't enable/disable signle step if the user did it

2012-07-26 Thread Sebastian Andrzej Siewior
If someone is using single stepping over uprobe brackpoint then after we pass the uprobe single step, single stepping is disabled and the user who enebaled them in the first place does not know anything about this. This patch avoids enabling / disabling the single step mode if it is already

[PATCH] uprobes: don't enable/disable signle step if the user did it

2012-07-26 Thread Sebastian Andrzej Siewior
If someone is using single stepping over uprobe brackpoint then after we pass the uprobe single step, single stepping is disabled and the user who enebaled them in the first place does not know anything about this. This patch avoids enabling / disabling the single step mode if it is already

Re: [PATCH] uprobes: don't enable/disable signle step if the user did it

2012-07-26 Thread Sebastian Andrzej Siewior
On 07/26/2012 05:20 PM, Sebastian Andrzej Siewior wrote: diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c index f935327..772eb3a 100644 --- a/kernel/events/uprobes.c +++ b/kernel/events/uprobes.c @@ -1528,7 +1528,10 @@ static void handle_swbp(struct pt_regs *regs)

Re: [PATCH] uprobes: don't enable/disable signle step if the user did it

2012-07-26 Thread Oleg Nesterov
Well. I agree, this needs changes. To begin with, uprobe should avoid user_enable_single_step() which does access_process_vm(). And I suspect uprobes have the problems with TIF_FORCED_TF logic. But I am not sure about this patch... On 07/26, Sebastian Andrzej Siewior wrote: @@ -1528,7 +1528,10