lkml.org 
[lkml]   [2012]   [Oct]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/4] uprobes: Kill set_swbp()->is_swbp_at_addr()
* Oleg Nesterov <oleg@redhat.com> [2012-09-23 22:19:45]:

> A separate patch for better documentation.
>
> set_swbp()->is_swbp_at_addr() is not needed for correctness, it is
> harmless to do the unnecessary __replace_page(old_page, new_page)
> when these 2 pages are identical.
>
> And it can not be counted as optimization. mmap/register races are
> very unlikely, while in the likely case is_swbp_at_addr() adds the
> extra get_user_pages() even if the caller is uprobe_mmap(current->mm)
> and returns false.
>
> Note also that the semantics/usage of is_swbp_at_addr() in uprobe.c
> is confusing. set_swbp() uses it to detect the case when this insn
> was already modified by uprobes, that is why it should always compare
> the opcode with UPROBE_SWBP_INSN even if the hardware (like powerpc)
> has other trap insns. It doesn't matter if this "int3" was in fact
> installed by gdb or application itself, we are going to "steal" this
> breakpoint anyway and execute the original insn from vm_file even if
> it no longer matches the memory.
>
> OTOH, handle_swbp()->find_active_uprobe() uses is_swbp_at_addr() to
> figure out whether we need to send SIGTRAP or not if we can not find
> uprobe, so in this case it should return true for all trap variants,
> not only for UPROBE_SWBP_INSN.
>
> This patch removes set_swbp()->is_swbp_at_addr(), the next patches
> will remove it from set_orig_insn() which is similar to set_swbp()
> in this respect. So the only caller will be handle_swbp() and we
> can make its semantics clear.
>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>

Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>



\
 
 \ /
  Last update: 2012-10-06 09:21    [W:0.497 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site