• Zach Brown's avatar
    [PATCH] dio: lock refcount operations · 5eb6c7a2
    Zach Brown authored
    The wait_for_more_bios() function name was poorly chosen.  While looking to
    clean it up it I noticed that the dio struct refcounting between the bio
    completion and dio submission paths was racey.
    
    The bio submission path was simply freeing the dio struct if
    atomic_dec_and_test() indicated that it dropped the final reference.
    
    The aio bio completion path was dereferencing its dio struct pointer *after
    dropping its reference* based on the remaining number of references.
    
    These two paths could race and result in the aio bio completion path
    dereferencing a freed dio, though this was not observed in the wild.
    
    This moves the refcount under the bio lock so that bio completion can drop
    its reference and decide to wake all in one atomic step.
    
    Once testing and waking is locked dio_await_one() can test its sleeping
    condition and mark itself uninterruptible under the lock.  It gets simpler
    and wait_for_more_bios() disappears.
    
    The addition of the interrupt masking spin lock acquiry in dio_bio_submit()
    looks alarming.  This lock acquiry existed in that path before the recent
    dio completion patch set.  We shouldn't expect significant performance
    regression from returning to the behaviour that existed before the
    completion clean up work.
    
    This passed 4k block ext3 O_DIRECT fsx and aio-stress on an SMP machine.
    Signed-off-by: default avatarZach Brown <zach.brown@oracle.com>
    Cc: Badari Pulavarty <pbadari@us.ibm.com>
    Cc: Suparna Bhattacharya <suparna@in.ibm.com>
    Cc: Jeff Moyer <jmoyer@redhat.com>
    Cc: <xfs-masters@oss.sgi.com>
    Signed-off-by: default avatarAndrew Morton <akpm@osdl.org>
    Signed-off-by: default avatarLinus Torvalds <torvalds@osdl.org>
    5eb6c7a2
direct-io.c 34.9 KB