lkml.org 
[lkml]   [2013]   [Oct]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 03/13] uprobes: allow arch access to xol slot
    On 10/15, David Long wrote:
    >
    > Allow arches to customize how the instruction is filled into the xol
    > slot. ARM will use this to insert an undefined instruction after the
    > real instruction in order to simulate a single step of the instruction
    > without hardware support.

    OK, but

    > +void __weak arch_uprobe_xol_copy(struct arch_uprobe *auprobe, void *vaddr)
    > +{
    > + memcpy(vaddr, auprobe->insn, MAX_UINSN_BYTES);
    > +}
    > +
    > /*
    > * xol_get_insn_slot - allocate a slot for xol.
    > * Returns the allocated slot address or 0.
    > @@ -1246,6 +1251,7 @@ static unsigned long xol_get_insn_slot(struct uprobe *uprobe)
    > {
    > struct xol_area *area;
    > unsigned long xol_vaddr;
    > + void *kaddr;
    >
    > area = get_xol_area();
    > if (!area)
    > @@ -1256,7 +1262,9 @@ static unsigned long xol_get_insn_slot(struct uprobe *uprobe)
    > return 0;
    >
    > /* Initialize the slot */
    > - copy_to_page(area->page, xol_vaddr, uprobe->arch.insn, MAX_UINSN_BYTES);
    > + kaddr = kmap_atomic(area->page);
    > + arch_uprobe_xol_copy(&uprobe->arch, kaddr + (xol_vaddr & ~PAGE_MASK));
    > + kunmap_atomic(kaddr);

    This looks a bit strange and defeats the purpose of generic helper...

    How about

    void __weak arch_uprobe_xol_copy(...)
    {
    copy_to_page(...);
    }

    then just

    - copy_to_page(...);
    + arch_uprobe_xol_copy(...);

    ?

    Or, I am just curious, can't we have an empty "__weak arch_uprobe_xol_copy" if
    we call it right after copy_to_page() ?

    Oleg.



    \
     
     \ /
      Last update: 2013-10-19 19:01    [W:2.545 / U:0.144 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site