lkml.org 
[lkml]   [2011]   [Mar]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH RFC] x86: avoid atomic operation in test_and_set_bit_lock if possible
    Date
    On Friday, March 25, 2011 04:40:13 pm Ingo Molnar wrote:
    > * Jan Beulich <JBeulich@novell.com> wrote:
    > > >>> On 24.03.11 at 18:19, Ingo Molnar <mingo@elte.hu> wrote:
    > > > * Jan Beulich <JBeulich@novell.com> wrote:
    > > >> Are you certain? Iirc the lock prefix implies minimally a read-for-
    > > >> ownership (if CPUs are really smart enough to optimize away the
    > > >> write - I wonder whether that would be correct at all when it
    > > >> comes to locked operations), which means a cacheline can still be
    > > >> bouncing heavily.
    > > >
    > > > Yeah. On what workload was this?
    > > >
    > > > Generally you use test_and_set_bit() if you expect it to be 'owned' by
    > > > whoever calls it, and released by someone else.
    > > >
    > > > It would be really useful to run perf top on an affected box and see
    > > > which kernel function causes this. It might be better to add a
    > > > test_bit() to the affected codepath - instead of bloating all
    > > > test_and_set_bit() users.
    > >
    > > Indeed, I agree with you and Linus in this aspect.
    > >
    > > > Note that the patch can also cause overhead: the test_bit() can miss
    > > > the cache, it will bring in the cacheline shared, and the subsequent
    > > > test_and_set() call will then dirty the cacheline - so the CPU might
    > > > miss again and has to wait for other CPUs to first flush this
    > > > cacheline.
    > > >
    > > > So we really need more details here.
    > >
    > > The problem was observed with __lock_page() (in a variant not
    > > upstream for reasons not known to me), and prefixing e.g.
    > > trylock_page() with an extra PageLocked() check yielded the
    > > below quoted improvements.
    >
    > The page lock flag is indeed one of those (rather rare) exceptions to
    > typical object locking patterns. So in that particular case adding the
    > PageLocked() test to trylock_page() would be the right approach to
    > improving performance.
    >
    > In the common case this change actively hurts for various reasons:
    >
    > - can turn a cache miss into two cache misses
    > - adds an often unnecessary branch instruction
    > - adds often unnecessary bloat
    > - leaks a barrier
    >

    Yes, I think I am observing these ill-effects when testing the code copied to
    user-space.

    Thanks
    Nikanth


    \
     
     \ /
      Last update: 2011-03-25 13:11    [W:0.024 / U:30.280 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site