lkml.org 
[lkml]   [2012]   [Aug]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 0/4] Fix a crash when block device is read and block size is changed at the same time


    On Fri, 31 Aug 2012, Jeff Moyer wrote:

    > Mikulas Patocka <mpatocka@redhat.com> writes:
    >
    > > On Fri, 31 Aug 2012, Mikulas Patocka wrote:
    > >
    > >> Hi
    > >>
    > >> This is a series of patches to prevent a crash when when someone is
    > >> reading block device and block size is changed simultaneously. (the crash
    > >> is already happening in the production environment)
    > >>
    > >> The first patch adds a rw-lock to struct block_device, but doesn't use the
    > >> lock anywhere. The reason why I submit this as a separate patch is that on
    > >> my computer adding an unused field to this structure affects performance
    > >> much more than any locking changes.
    > >>
    > >> The second patch uses the rw-lock. The lock is locked for read when doing
    > >> I/O on the block device and it is locked for write when changing block
    > >> size.
    > >>
    > >> The third patch converts the rw-lock to a percpu rw-lock for better
    > >> performance, to avoid cache line bouncing.
    > >>
    > >> The fourth patch is an alternate percpu rw-lock implementation using RCU
    > >> by Eric Dumazet. It avoids any atomic instruction in the hot path.
    > >>
    > >> Mikulas
    > >
    > > I tested performance of patches. I created 4GB ramdisk, I initially filled
    > > it with zeros (so that ramdisk allocation-on-demand doesn't affect the
    > > results).
    > >
    > > I ran fio to perform 8 concurrent accesses on 8 core machine (two
    > > Barcelona Opterons):
    > > time fio --rw=randrw --size=4G --bs=512 --filename=/dev/ram0 --direct=1
    > > --name=job1 --name=job2 --name=job3 --name=job4 --name=job5 --name=job6
    > > --name=job7 --name=job8
    > >
    > > The results actually show that the size of struct block_device and
    > > alignment of subsequent fields in struct inode have far more effect on
    > > result that the type of locking used. (struct inode is placed just after
    > > struct block_device in "struct bdev_inode" in fs/block-dev.c)
    > >
    > > plain kernel 3.5.3: 57.9s
    > > patch 1: 43.4s
    > > patches 1,2: 43.7s
    > > patches 1,2,3: 38.5s
    > > patches 1,2,3,4: 58.6s
    > >
    > > You can see that patch 1 improves the time by 14.5 seconds, but all that
    > > patch 1 does is adding an unused structure field.
    > >
    > > Patch 3 is 4.9 seconds faster than patch 1, althogh patch 1 does no
    > > locking at all and patch 3 does per-cpu locking. So, the reason for the
    > > speedup is different sizeof of struct block_device (and subsequently,
    > > different alignment of struct inode), rather than locking improvement.
    >
    > How many runs did you do? Did you see much run to run variation?

    These results come from two runs (which differed by no more than 1s), but
    I observed the same phenomenon - difference in time due to the size of
    block_device - many times before when I was doing benchmarking when
    developing these patches.

    I actually had to apply something like this to make the results not depend
    on the size of block_dev.

    I would be interested if the same difference could be observed on other
    processors or if it is something specific to AMD K10 architecture.

    ---
    fs/block_dev.c | 5 ++++-
    1 file changed, 4 insertions(+), 1 deletion(-)

    Index: linux-3.5.3-fast/fs/block_dev.c
    ===================================================================
    --- linux-3.5.3-fast.orig/fs/block_dev.c 2012-08-31 22:30:07.000000000 +0200
    +++ linux-3.5.3-fast/fs/block_dev.c 2012-08-31 22:30:43.000000000 +0200
    @@ -31,7 +31,10 @@
    #include "internal.h"

    struct bdev_inode {
    - struct block_device bdev;
    + union {
    + struct block_device bdev;
    + char pad[0x140];
    + };
    struct inode vfs_inode;
    };

    > > I would be interested if other people did performance testing of the
    > > patches too.
    >
    > I'll do some testing next week, but don't expect to get to it before
    > Wednesday.
    >
    > Cheers,
    > Jeff

    Mikulas


    \
     
     \ /
      Last update: 2012-08-31 23:21    [W:4.512 / U:0.068 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site