linux-kernel - Re: [PATCH 0/4] Fix a crash when block device is read and block size is changed at the same time

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 31 Aug 2012 16:11:56 -0400
From:	Jeff Moyer <jmoyer@...hat.com>
To:	Mikulas Patocka <mpatocka@...hat.com>
Cc:	Eric Dumazet <eric.dumazet@...il.com>,
	Jens Axboe <axboe@...nel.dk>,
	Andrea Arcangeli <aarcange@...hat.com>,
	Jan Kara <jack@...e.cz>, dm-devel@...hat.com,
	linux-kernel@...r.kernel.org,
	Alexander Viro <viro@...iv.linux.org.uk>,
	kosaki.motohiro@...fujitsu.com, linux-fsdevel@...r.kernel.org,
	lwoodman@...hat.com, "Alasdair G. Kergon" <agk@...hat.com>
Subject: Re: [PATCH 0/4] Fix a crash when block device is read and block size is changed at the same time

Mikulas Patocka <mpatocka@...hat.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?

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