lkml.org 
[lkml]   [2003]   [Apr]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] fix obj vma sorting
    >> Hmmm. Something somewhere went wrong. Some semaphore blew up
    >> somewhere ... I'm not convinced that this is your patch
    >> causing the problem, I just thought that since vma_link seems
    >> to have gone up rather in the profile. I'm playing with getting
    >> some better data on what actually happened, but in case someone
    >> is feeling psychic.
    >>
    >> The main thing I changed here (66-mjb2 -> 67-mjb0.2) was to pick up
    >> Andrew's rmap speedups, and drop the objrmap code I had for the stuff
    >
    > I haven't examined it, but I'm guessing 66-mjb2 did not have Dave's
    > vma sorting in at all? Its linear search would certainly raise the
    > time spent in __vma_link (notable in your diffprofile), which would
    > increase the pressure on i_shared_sem.

    No it didn't ... but I think 67-mm1 did.

    > (Whether it's a worthwhile optimization remains to be seen: like
    > rmap generally, it speeds up page_referenced and try_to_unmap at
    > the expense of the fast path. One improvement would be for fork
    > to just slot dst vma in next to src vma instead of linear search.)
    >
    > I don't think my fix to the sort order could have slowed it down
    > further (though once there are stray entries out of order, it may
    > be hard to predict how things will work out). But without it
    > page_referenced and try_to_unmap sometimes couldn't quite find
    > all the mappings they were looking for.

    It is that fix ... I just backed that one patch off and recompared:

    DISCLAIMER: SPEC(tm) and the benchmark name SDET(tm) are registered
    trademarks of the Standard Performance Evaluation Corporation. This
    benchmarking was performed for research purposes only, and the run results
    are non-compliant and not-comparable with any published results.

    Results are shown as percentages of the first set displayed

    SDET 32 (see disclaimer)
    Throughput Std. Dev
    2.5.67 100.0% 0.3%
    2.5.67-mjb0.2 151.7% 0.5%
    2.5.67-mjb0.2-nosort 207.1% 0.0%

    SDET 64 (see disclaimer)
    Throughput Std. Dev
    2.5.67 100.0% 0.4%
    2.5.67-mjb0.2 147.0% 0.5%
    2.5.67-mjb0.2-nosort 201.5% 0.2%

    SDET 128 (see disclaimer)
    Throughput Std. Dev
    2.5.67 100.0% 5.1%
    2.5.67-mjb0.2 144.5% 0.1%
    2.5.67-mjb0.2-nosort 188.6% 0.3%


    I think it's that sem, which seems to be heavily contented.
    Quite possibly for glibc's address_space or something.
    (even though it says "-nosort", it's just your sort fix I
    backed out ... otherwise it's what was in -mm).

    >> he had. *However*, what he had worked fine. I also picked up your
    >> sorting patch here Hugh ... this bit worries me:
    >>
    >> +static void move_vma_start(struct vm_area_struct *vma, unsigned long addr)
    >
    > It does use i_shared_sem where it wasn't used before, yes, but it's
    > only called by one case of vma_merge and one case of split_vma:
    > unless your tests are doing a lot of vma splitting (e.g. mprotecting
    > ranges which break up vmas), I wouldn't expect it to figure highly.
    > I can see it's there in the plus part of your diffprofile, but I'm
    > too inexperienced at reading these things, without the original
    > profiles, to tell whether it's being used a surprising amount.

    Here's the diffprofile for just your patch ... where it's positive,
    that's the increase in the number of ticks by applying your patch.
    Where it's negative, that's the decrease. The %age is the change from
    the first to the second profile:

    larry:/var/bench/results# diffprofile 2.5.67-mjb0.2{-nosort,}/sdetbench/64/profile
    7148 24.9% total
    6482 37.7% default_idle
    1466 842.5% __down
    442 566.7% __wake_up
    435 378.3% schedule
    251 0.0% move_vma_start
    149 876.5% __vma_link
    72 40.2% remove_shared_vm_struct
    46 35.1% copy_mm
    20 60.6% vma_link
    12 300.0% default_wake_function
    11 137.5% rb_insert_color
    ...
    -20 -37.0% number
    -20 -12.6% do_anonymous_page
    -21 -36.8% fd_install
    -23 -27.7% __find_get_block
    -24 -55.8% flush_signal_handlers
    -27 -45.0% __set_page_dirty_buffers
    -28 -26.7% kmem_cache_free
    -28 -7.5% find_get_page
    -29 -34.1% buffered_rmqueue
    -32 -34.8% path_release
    -33 -32.0% file_move
    -35 -60.3% __read_lock_failed
    -35 -43.8% .text.lock.highmem
    -37 -59.7% .text.lock.namei
    -37 -29.1% pte_alloc_one
    -40 -10.3% page_add_rmap
    -41 -41.4% free_hot_cold_page
    -44 -60.3% .text.lock.file_table
    -54 -18.4% __copy_to_user_ll
    -58 -43.0% follow_mount
    -62 -29.0% path_lookup
    -85 -20.9% __d_lookup
    -86 -20.4% release_pages
    -99 -68.8% .text.lock.dcache
    -100 -15.4% page_remove_rmap
    -106 -36.6% atomic_dec_and_lock
    -126 -16.8% zap_pte_range
    -141 -66.8% .text.lock.dec_and_lock

    Note the massive increase in down() (and some of the vma ops).
    The things that are cheaper are probably just because of less
    contention, I guess.

    > When you say "*However*, what he had worked fine", are you saying
    > you profiled before adding in my patch on top? The diffprofile of
    > the before and after my patch should in that case illuminate.

    Well, I hadn't ... but I should have done, and I have now ;-)

    I'll attach the two raw profiles for you as well. profile.with
    is with your patch, profile.without is without ... I was looking
    at SDET 64, since it showed the most dramatic difference.

    M.[unhandled content-type:application/octet-stream][unhandled content-type:application/octet-stream]
    \
     
     \ /
      Last update: 2005-03-22 13:34    [W:3.845 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site