Re: [PATCH] fix obj vma sorting

From: Martin J. Bligh (mbligh@aracnet.com)
Date: Wed Apr 09 2003 - 13:33:11 EST


>> 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.




-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Tue Apr 15 2003 - 22:00:18 EST