Messages in this thread | | | Date | Tue, 9 Apr 2013 16:16:15 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH v1 2/9] uretprobes: Reserve the first slot in xol_vma for trampoline |
| |
On 04/07, Srikar Dronamraju wrote: > > * Anton Arapov <anton@redhat.com> [2013-04-03 18:00:32]: > > > Allocate trampoline page, as the very first one in uprobed > > task xol area, and fill it with breakpoint opcode. > > > > Also introduce get_trampoline_vaddr() helper, to wrap the > > trampoline address extraction from area->vaddr. That removes > > confusion and eases the debug experience in case ->vaddr > > notion will be changed. > > > > v1 changes: > > * rework get_trampoline_vaddr() helper. > > * init xol_area->slot_count. > > > > Signed-off-by: Anton Arapov <anton@redhat.com> > > Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Thanks!
> > +static unsigned long get_trampoline_vaddr(void) > > +{ > > + struct xol_area *area; > > + unsigned long trampoline_vaddr = -1; > > + > > + area = current->mm->uprobes_state.xol_area; > > + smp_read_barrier_depends(); > > check: do we need this barrier?
In theory yes. For example, we do not want the false positive in handle_swbp() which does
if (bp_vaddr == get_trampoline_vaddr()) handler_uretprobe();
In theory we can race with another thread which initializes area (in particular area->vaddr) and then sets uprobes_state.xol_area = area in xol_add_vma().
If we see ->xol_area != NULL we must ensure that we read the correct value of area->vaddr, so we need a barrier.
In short. Please note get_xol_area()->smp_read_barrier_depends(), get_trampoline_vaddr() needs it for the same reason.
Oleg.
| |