On Fri, 31 Aug 2012, Jeff Moyer wrote: > Mikulas Patocka <mpatocka@xxxxxxxxxx> 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 -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel