lkml.org 
[lkml]   [2011]   [Nov]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v7 3.2-rc2 4/30] uprobes: Define hooks for mmap/munmap.
    From
    Date
    On Thu, 2011-11-24 at 19:17 +0530, Srikar Dronamraju wrote:
    > * Peter Zijlstra <peterz@infradead.org> [2011-11-23 19:10:12]:
    >
    > > On Fri, 2011-11-18 at 16:37 +0530, Srikar Dronamraju wrote:
    > > > + ret = install_breakpoint(vma->vm_mm, uprobe);
    > > > + if (ret == -EEXIST) {
    > > > + atomic_inc(&vma->vm_mm->mm_uprobes_count);
    > > > + ret = 0;
    > > > + }
    > >
    > > Aren't you double counting that probe position here? The one that raced
    > > you to inserting it will also have incremented that counter, no?
    > >
    >
    > No we arent.
    > Because register_uprobe can never race with mmap_uprobe and register
    > before mmap_uprobe registers .(Once we start mmap_region,
    > register_uprobe waits for the read_lock of mmap_sem.)
    >
    > And we badly need this for mmap_uprobe case. Because when we do mremap,
    > or vma_adjust(), we do a munmap_uprobe() followed by mmap_uprobe() which
    > would have decremented the count but not removed it. So when we do a
    > mmap_uprobe, we need to increment the count.

    Ok, so I didn't parse that properly last time around.. but it still
    doesn't make sense, why would munmap_uprobe() decrement the count but
    not uninstall the probe?

    install_breakpoint() returning -EEXIST on two different conditions
    doesn't help either.

    So what I think you're doing is that you're optimizing the unmap case
    since the memory is going to be thrown out fixing up the instruction is
    a waste of time, but this leads to the asymmetry observed above. But you
    fail to mention this in both the changelog or a comment near that
    -EEXIST branch in mmap_uprobe.

    Worse, you don't explain how the other -EEXIST (!consumers) thing
    interacts here, and I just gave up trying to figure that out since it
    made my head hurt.

    Also, your whole series of patches is still utter crap, the splitup
    doesn't work at all, I need to constantly search back and forth between
    patches in order to figure out wtf is happening, and your changelogs
    only seem to add confusion if anything at all.

    Also, you seem to have stuck a whole bunch of random patches at the end
    that fix various things without folding them back in to make the series
    saner/smaller.

    I've now reverted to simply applying all patches and reading the end
    result and using git-blame to figure out what patch something came
    from :-(



    \
     
     \ /
      Last update: 2011-11-28 16:03    [W:4.980 / U:0.068 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site