From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754951Ab2HaUMR (ORCPT ); Fri, 31 Aug 2012 16:12:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:21939 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754892Ab2HaUMN (ORCPT ); Fri, 31 Aug 2012 16:12:13 -0400 From: Jeff Moyer To: Mikulas Patocka Cc: Eric Dumazet , Jens Axboe , Andrea Arcangeli , Jan Kara , dm-devel@redhat.com, linux-kernel@vger.kernel.org, Alexander Viro , kosaki.motohiro@jp.fujitsu.com, linux-fsdevel@vger.kernel.org, lwoodman@redhat.com, "Alasdair G. Kergon" Subject: Re: [PATCH 0/4] Fix a crash when block device is read and block size is changed at the same time References: <20120628111541.GB17515@quack.suse.cz> <1343508252.2626.13184.camel@edumazet-glaptop> <1343556630.2626.13257.camel@edumazet-glaptop> <1343586962.2626.13266.camel@edumazet-glaptop> X-PGP-KeyID: 1F78E1B4 X-PGP-CertKey: F6FE 280D 8293 F72C 65FD 5A58 1FF8 A7CA 1F78 E1B4 X-PCLoadLetter: What the f**k does that mean? Date: Fri, 31 Aug 2012 16:11:56 -0400 In-Reply-To: (Mikulas Patocka's message of "Fri, 31 Aug 2012 15:27:38 -0400 (EDT)") Message-ID: User-Agent: Gnus/5.110011 (No Gnus v0.11) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Mikulas Patocka 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