lkml.org 
[lkml]   [2012]   [Jul]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH 04/14] uprobes: cleanup and document write_opcode()->lock_page(old_page)
    The comment above write_opcode()->lock_page(old_page) tells about
    the race with do_wp_page(). I don't really understand which exactly
    race it means, but afaics this lock_page() was not enough to close
    all races with do_wp_page().

    Anyway, since 77fc4af1 this code is always called with ->mmap_sem
    hold for writing so we can forget about do_wp_page().

    However, we can't simply remove this lock_page(), and the only
    (afaics) reason is __replace_page()->try_to_free_swap().

    Nothing in write_opcode() needs it, move it into __replace_page()
    and fix the comment.

    Signed-off-by: Oleg Nesterov <oleg@redhat.com>
    Acked-by: Srikar Dronamraju <srikar.vnet.ibm.com>
    ---
    kernel/events/uprobes.c | 27 ++++++++++++++-------------
    1 files changed, 14 insertions(+), 13 deletions(-)

    diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
    index dff8da2..67697db 100644
    --- a/kernel/events/uprobes.c
    +++ b/kernel/events/uprobes.c
    @@ -139,10 +139,15 @@ static int __replace_page(struct vm_area_struct *vma, unsigned long addr,
    struct mm_struct *mm = vma->vm_mm;
    spinlock_t *ptl;
    pte_t *ptep;
    + int err;

    + /* freeze PageSwapCache() for try_to_free_swap() below */
    + lock_page(page);
    +
    + err = -EAGAIN;
    ptep = page_check_address(page, mm, addr, &ptl, 0);
    if (!ptep)
    - return -EAGAIN;
    + goto unlock;

    get_page(kpage);
    page_add_new_anon_rmap(kpage, vma, addr);
    @@ -162,7 +167,10 @@ static int __replace_page(struct vm_area_struct *vma, unsigned long addr,
    put_page(page);
    pte_unmap_unlock(ptep, ptl);

    - return 0;
    + err = 0;
    + unlock:
    + unlock_page(page);
    + return err;
    }

    /**
    @@ -216,15 +224,10 @@ retry:
    ret = -ENOMEM;
    new_page = alloc_page_vma(GFP_HIGHUSER_MOVABLE, vma, vaddr);
    if (!new_page)
    - goto put_out;
    + goto put_old;

    __SetPageUptodate(new_page);

    - /*
    - * lock page will serialize against do_wp_page()'s
    - * PageAnon() handling
    - */
    - lock_page(old_page);
    /* copy the page now that we've got it stable */
    vaddr_old = kmap_atomic(old_page);
    vaddr_new = kmap_atomic(new_page);
    @@ -237,15 +240,13 @@ retry:

    ret = anon_vma_prepare(vma);
    if (ret)
    - goto unlock_out;
    + goto put_new;

    ret = __replace_page(vma, vaddr, old_page, new_page);

    -unlock_out:
    - unlock_page(old_page);
    +put_new:
    page_cache_release(new_page);
    -
    -put_out:
    +put_old:
    put_page(old_page);

    if (unlikely(ret == -EAGAIN))
    --
    1.5.5.1


    \
     
     \ /
      Last update: 2012-07-29 21:01    [W:6.795 / U:0.288 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site