lkml.org 
[lkml]   [2012]   [Jun]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH 4/5] 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>
    ---
    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 16a3b69..442064d 100644
    --- a/kernel/events/uprobes.c
    +++ b/kernel/events/uprobes.c
    @@ -138,10 +138,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);
    @@ -161,7 +166,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;
    }

    /**
    @@ -215,15 +223,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);
    @@ -236,15 +239,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-06-19 22:21    [W:0.028 / U:34.276 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site