Re: [PATCH 4/7] uprobes: Fix handle_swbp() vs unregister() + register() race

2012-10-07 Thread Srikar Dronamraju
* Oleg Nesterov [2012-10-06 20:53:37]: > On 10/06, Srikar Dronamraju wrote: > > > > > > > > for the future changes... (say, we can remove bp if consumers do not > > > want to trace this task). Not sure it makes sense to change it right > > > now. > > > > > > So. Should I leave this patch as is?

Re: [PATCH 4/7] uprobes: Fix handle_swbp() vs unregister() + register() race

2012-10-07 Thread Srikar Dronamraju
* Oleg Nesterov o...@redhat.com [2012-10-06 20:53:37]: On 10/06, Srikar Dronamraju wrote: for the future changes... (say, we can remove bp if consumers do not want to trace this task). Not sure it makes sense to change it right now. So. Should I leave this patch as is? Or do

Re: [PATCH 4/7] uprobes: Fix handle_swbp() vs unregister() + register() race

2012-10-06 Thread Oleg Nesterov
On 10/06, Srikar Dronamraju wrote: > > > > > for the future changes... (say, we can remove bp if consumers do not > > want to trace this task). Not sure it makes sense to change it right > > now. > > > > So. Should I leave this patch as is? Or do you want me to move this > > check into

Re: [PATCH 4/7] uprobes: Fix handle_swbp() vs unregister() + register() race

2012-10-06 Thread Srikar Dronamraju
> > for the future changes... (say, we can remove bp if consumers do not > want to trace this task). Not sure it makes sense to change it right > now. > > So. Should I leave this patch as is? Or do you want me to move this > check into handler_chain() and make it return "bool restart"? Lets

Re: [PATCH 4/7] uprobes: Fix handle_swbp() vs unregister() + register() race

2012-10-06 Thread Oleg Nesterov
On 10/06, Srikar Dronamraju wrote: > > * Oleg Nesterov [2012-09-30 21:42:11]: > > > @@ -1391,6 +1392,16 @@ static struct uprobe *find_active_uprobe(unsigned > > long bp_vaddr, int *is_swbp) > > if (!uprobe && test_and_clear_bit(MMF_RECALC_UPROBES, >flags)) > >

Re: [PATCH 4/7] uprobes: Fix handle_swbp() vs unregister() + register() race

2012-10-06 Thread Srikar Dronamraju
* Oleg Nesterov [2012-09-30 21:42:11]: > Strictly speaking this race was added by me in 56bb4cf6. However > I think that this bug is just another indication that we should > move copy_insn/uprobe_analyze_insn code from install_breakpoint() > to uprobe_register(), there are a lot of other reasons

Re: [PATCH 4/7] uprobes: Fix handle_swbp() vs unregister() + register() race

2012-10-06 Thread Srikar Dronamraju
* Oleg Nesterov o...@redhat.com [2012-09-30 21:42:11]: Strictly speaking this race was added by me in 56bb4cf6. However I think that this bug is just another indication that we should move copy_insn/uprobe_analyze_insn code from install_breakpoint() to uprobe_register(), there are a lot of

Re: [PATCH 4/7] uprobes: Fix handle_swbp() vs unregister() + register() race

2012-10-06 Thread Oleg Nesterov
On 10/06, Srikar Dronamraju wrote: * Oleg Nesterov o...@redhat.com [2012-09-30 21:42:11]: @@ -1391,6 +1392,16 @@ static struct uprobe *find_active_uprobe(unsigned long bp_vaddr, int *is_swbp) if (!uprobe test_and_clear_bit(MMF_RECALC_UPROBES, mm-flags))

Re: [PATCH 4/7] uprobes: Fix handle_swbp() vs unregister() + register() race

2012-10-06 Thread Srikar Dronamraju
for the future changes... (say, we can remove bp if consumers do not want to trace this task). Not sure it makes sense to change it right now. So. Should I leave this patch as is? Or do you want me to move this check into handler_chain() and make it return bool restart? Lets keep it as

Re: [PATCH 4/7] uprobes: Fix handle_swbp() vs unregister() + register() race

2012-10-06 Thread Oleg Nesterov
On 10/06, Srikar Dronamraju wrote: for the future changes... (say, we can remove bp if consumers do not want to trace this task). Not sure it makes sense to change it right now. So. Should I leave this patch as is? Or do you want me to move this check into handler_chain() and make

Re: [PATCH 4/7] uprobes: Fix handle_swbp() vs unregister() + register() race

2012-10-02 Thread Oleg Nesterov
On 09/30, Oleg Nesterov wrote: > > + if (uprobe && unlikely(!(uprobe->flags & UPROBE_COPY_INSN))) { > + uprobe = NULL; > + *is_swbp = 0; > + } OOPS. this obvioulsy needs put_uprobe(uprobe). I updated this patch in my tree.

Re: [PATCH 4/7] uprobes: Fix handle_swbp() vs unregister() + register() race

2012-10-02 Thread Oleg Nesterov
On 09/30, Oleg Nesterov wrote: + if (uprobe unlikely(!(uprobe-flags UPROBE_COPY_INSN))) { + uprobe = NULL; + *is_swbp = 0; + } OOPS. this obvioulsy needs put_uprobe(uprobe). I updated this patch in my tree.