From patchwork Thu Sep 17 01:12:41 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kiyoshi Ueda X-Patchwork-Id: 48191 X-Patchwork-Delegate: agk@redhat.com Received: from hormel.redhat.com (hormel1.redhat.com [209.132.177.33]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n8H1E6Kj022827 for ; Thu, 17 Sep 2009 01:14:06 GMT Received: from listman.util.phx.redhat.com (listman.util.phx.redhat.com [10.8.4.110]) by hormel.redhat.com (Postfix) with ESMTP id 57AFD619596; Wed, 16 Sep 2009 21:14:06 -0400 (EDT) Received: from int-mx03.intmail.prod.int.phx2.redhat.com (nat-pool.util.phx.redhat.com [10.8.5.200]) by listman.util.phx.redhat.com (8.13.1/8.13.1) with ESMTP id n8H1E4AE030757 for ; Wed, 16 Sep 2009 21:14:04 -0400 Received: from mx1.redhat.com (ext-mx09.extmail.prod.ext.phx2.redhat.com [10.5.110.13]) by int-mx03.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id n8H1E40f002943; Wed, 16 Sep 2009 21:14:04 -0400 Received: from tyo201.gate.nec.co.jp (TYO201.gate.nec.co.jp [202.32.8.193]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id n8H1DrvM032412; Wed, 16 Sep 2009 21:13:54 -0400 Received: from mailgate3.nec.co.jp ([10.7.69.195]) by tyo201.gate.nec.co.jp (8.13.8/8.13.4) with ESMTP id n8H1DrZ5022492; Thu, 17 Sep 2009 10:13:53 +0900 (JST) Received: (from root@localhost) by mailgate3.nec.co.jp (8.11.7/3.7W-MAILGATE-NEC) id n8H1Drb03949; Thu, 17 Sep 2009 10:13:53 +0900 (JST) Received: from mail03.kamome.nec.co.jp (mail03.kamome.nec.co.jp [10.25.43.7]) by mailsv.nec.co.jp (8.13.8/8.13.4) with ESMTP id n8H1DqGT006835; Thu, 17 Sep 2009 10:13:52 +0900 (JST) Received: from shikibu.jp.nec.com ([10.26.220.2] [10.26.220.2]) by mail02.kamome.nec.co.jp with ESMTP id BT-MMP-2005259; Thu, 17 Sep 2009 10:12:41 +0900 Received: from elcondor.linux.bs1.fc.nec.co.jp ([10.34.125.195] [10.34.125.195]) by mail.jp.nec.com with ESMTP; Thu, 17 Sep 2009 10:12:41 +0900 Message-ID: <4AB18D09.9090300@ct.jp.nec.com> Date: Thu, 17 Sep 2009 10:12:41 +0900 From: Kiyoshi Ueda User-Agent: Thunderbird 2.0.0.23 (X11/20090825) MIME-Version: 1.0 To: Alasdair Kergon X-RedHat-Spam-Score: 0 () X-Scanned-By: MIMEDefang 2.67 on 10.5.11.16 X-Scanned-By: MIMEDefang 2.67 on 10.5.110.13 X-loop: dm-devel@redhat.com Cc: device-mapper development Subject: [dm-devel] [PATCH] dm: dec_pending() needs locking to save error value X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.5 Precedence: junk Reply-To: device-mapper development List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com Hi Alasdair, While implementing the barrier support patch for request-based dm, I found a possible bug in bio-based dm. In the patch posted here: http://patchwork.kernel.org/patch/48189/ I added 'barrier_error_lock' to struct mapped_device to save error values from multiple barrier I/Os safely like below: if (error) { spin_lock_irqsave(&md->barrier_error_lock, flags); if (!md->barrier_error || error == -EOPNOTSUPP || (md->barrier_error != -EOPNOTSUPP && error == DM_ENDIO_REQUEUE)) md->barrier_error = error; spin_unlock_irqrestore(&md->barrier_error_lock, flags); } On the other hand, bio-based dm has been doing the same thing in dec_pending() without any lock: if (error && !(io->error > 0 && __noflush_suspending(md))) io->error = error; Though I have never experienced actual problem without locking and just found this during code inspection, I believe it needs locking here, too. This patch adds the locking. I've done compile, boot and basic I/O testings. Signed-off-by: Kiyoshi Ueda Signed-off-by: Jun'ichi Nomura Cc: Alasdair G Kergon --- drivers/md/dm.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel Index: 2.6.31/drivers/md/dm.c =================================================================== --- 2.6.31.orig/drivers/md/dm.c +++ 2.6.31/drivers/md/dm.c @@ -47,6 +47,7 @@ struct dm_io { atomic_t io_count; struct bio *bio; unsigned long start_time; + spinlock_t endio_lock; }; /* @@ -590,8 +591,12 @@ static void dec_pending(struct dm_io *io struct mapped_device *md = io->md; /* Push-back supersedes any I/O errors */ - if (error && !(io->error > 0 && __noflush_suspending(md))) - io->error = error; + if (unlikely(error)) { + spin_lock_irqsave(&io->endio_lock, flags); + if (!(io->error > 0 && __noflush_suspending(md))) + io->error = error; + spin_unlock_irqrestore(&io->endio_lock, flags); + } if (atomic_dec_and_test(&io->io_count)) { if (io->error == DM_ENDIO_REQUEUE) { @@ -1300,6 +1305,7 @@ static void __split_and_process_bio(stru atomic_set(&ci.io->io_count, 1); ci.io->bio = bio; ci.io->md = md; + spin_lock_init(&ci.io->endio_lock); ci.sector = bio->bi_sector; ci.sector_count = bio_sectors(bio); if (unlikely(bio_empty_barrier(bio)))