public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: axboe@kernel.dk, hch@infradead.org, linux-kernel@vger.kernel.org,
	petero2@telia.com, schwidefsky@de.ibm.com,
	heiko.carstens@de.ibm.com, jack@suse.cz,
	akpm@linux-foundation.org, adilger.kernel@dilger.ca,
	tytso@mit.edu, mfasheh@suse.com, joel.becker@oracle.com,
	aelder@sgi.com, dm-devel@redhat.com, drbd-dev@lists.linbit.com,
	neilb@suse.de, leochen@broadcom.com, sbranden@broadcom.com,
	chris.mason@oracle.com, swhiteho@redhat.com,
	shaggy@linux.vnet.ibm.com, joern@logfs.org,
	konishi.ryusuke@lab.ntt.co.jp, reiserfs-devel@vger.kernel.org,
	viro@zeniv.linux.org.uk
Cc: Tejun Heo <tj@kernel.org>
Subject: [PATCH 5/5] block: reorganize claim/release implementation
Date: Mon,  1 Nov 2010 17:15:29 +0100	[thread overview]
Message-ID: <1288628129-12811-6-git-send-email-tj@kernel.org> (raw)
In-Reply-To: <1288628129-12811-1-git-send-email-tj@kernel.org>

With claim/release rolled into blkdev_get/put(), there's no reason to
keep bd_abort/finish_claim(), __bd_claim() and bd_release() as
separate functions.  It only makes the code difficult to follow.
Collapse them into blkdev_get/put().  This will ease future changes
around claim/release.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 fs/block_dev.c |  127 +++++++++++++++++++++-----------------------------------
 1 files changed, 48 insertions(+), 79 deletions(-)

diff --git a/fs/block_dev.c b/fs/block_dev.c
index fc48912..269bfbb 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -772,79 +772,6 @@ static struct block_device *bd_start_claiming(struct block_device *bdev,
 	}
 }
 
-/* releases bdev_lock */
-static void __bd_abort_claiming(struct block_device *whole, void *holder)
-{
-	BUG_ON(whole->bd_claiming != holder);
-	whole->bd_claiming = NULL;
-	wake_up_bit(&whole->bd_claiming, 0);
-
-	spin_unlock(&bdev_lock);
-	bdput(whole);
-}
-
-/**
- * bd_abort_claiming - abort claiming a block device
- * @whole: whole block device returned by bd_start_claiming()
- * @holder: holder trying to claim @bdev
- *
- * Abort a claiming block started by bd_start_claiming().  Note that
- * @whole is not the block device to be claimed but the whole device
- * returned by bd_start_claiming().
- *
- * CONTEXT:
- * Grabs and releases bdev_lock.
- */
-static void bd_abort_claiming(struct block_device *whole, void *holder)
-{
-	spin_lock(&bdev_lock);
-	__bd_abort_claiming(whole, holder);		/* releases bdev_lock */
-}
-
-/* increment holders when we have a legitimate claim. requires bdev_lock */
-static void __bd_claim(struct block_device *bdev, struct block_device *whole,
-					void *holder)
-{
-	/* note that for a whole device bd_holders
-	 * will be incremented twice, and bd_holder will
-	 * be set to bd_may_claim before being set to holder
-	 */
-	whole->bd_holders++;
-	whole->bd_holder = bd_may_claim;
-	bdev->bd_holders++;
-	bdev->bd_holder = holder;
-}
-
-/**
- * bd_finish_claiming - finish claiming a block device
- * @bdev: block device of interest (passed to bd_start_claiming())
- * @whole: whole block device returned by bd_start_claiming()
- * @holder: holder trying to claim @bdev
- *
- * Finish a claiming block started by bd_start_claiming().
- *
- * CONTEXT:
- * Grabs and releases bdev_lock.
- */
-static void bd_finish_claiming(struct block_device *bdev,
-				struct block_device *whole, void *holder)
-{
-	spin_lock(&bdev_lock);
-	BUG_ON(!bd_may_claim(bdev, whole, holder));
-	__bd_claim(bdev, whole, holder);
-	__bd_abort_claiming(whole, holder); /* not actually an abort */
-}
-
-static void bd_release(struct block_device *bdev)
-{
-	spin_lock(&bdev_lock);
-	if (!--bdev->bd_contains->bd_holders)
-		bdev->bd_contains->bd_holder = NULL;
-	if (!--bdev->bd_holders)
-		bdev->bd_holder = NULL;
-	spin_unlock(&bdev_lock);
-}
-
 #ifdef CONFIG_SYSFS
 static int add_symlink(struct kobject *from, struct kobject *to)
 {
@@ -1223,10 +1150,30 @@ int blkdev_get(struct block_device *bdev, fmode_t mode, void *holder)
 	res = __blkdev_get(bdev, mode, 0);
 
 	if (whole) {
-		if (res == 0)
-			bd_finish_claiming(bdev, whole, holder);
-		else
-			bd_abort_claiming(whole, holder);
+		/* finish claiming */
+		spin_lock(&bdev_lock);
+
+		if (res == 0) {
+			BUG_ON(!bd_may_claim(bdev, whole, holder));
+			/*
+			 * Note that for a whole device bd_holders
+			 * will be incremented twice, and bd_holder
+			 * will be set to bd_may_claim before being
+			 * set to holder
+			 */
+			whole->bd_holders++;
+			whole->bd_holder = bd_may_claim;
+			bdev->bd_holders++;
+			bdev->bd_holder = holder;
+		}
+
+		/* tell others that we're done */
+		BUG_ON(whole->bd_claiming != holder);
+		whole->bd_claiming = NULL;
+		wake_up_bit(&whole->bd_claiming, 0);
+
+		spin_unlock(&bdev_lock);
+		bdput(whole);
 	}
 
 	return res;
@@ -1272,6 +1219,7 @@ static int __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part)
 		bdev->bd_part_count--;
 
 	if (!--bdev->bd_openers) {
+		WARN_ON_ONCE(bdev->bd_holders);
 		sync_blockdev(bdev);
 		kill_bdev(bdev);
 	}
@@ -1303,10 +1251,31 @@ static int __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part)
 int blkdev_put(struct block_device *bdev, fmode_t mode)
 {
 	if (mode & FMODE_EXCL) {
+		bool bdev_free;
+
+		/*
+		 * Release a claim on the device.  The holder fields
+		 * are protected with bdev_lock.  bd_mutex is to
+		 * synchronize disk_holder unlinking.
+		 */
 		mutex_lock(&bdev->bd_mutex);
-		bd_release(bdev);
-		if (!bdev->bd_holders)
+		spin_lock(&bdev_lock);
+
+		WARN_ON_ONCE(--bdev->bd_holders < 0);
+		WARN_ON_ONCE(--bdev->bd_contains->bd_holders < 0);
+
+		/* bd_contains might point to self, check in a separate step */
+		if ((bdev_free = !bdev->bd_holders))
+			bdev->bd_holder = NULL;
+		if (!bdev->bd_contains->bd_holders)
+			bdev->bd_contains->bd_holder = NULL;
+
+		spin_unlock(&bdev_lock);
+
+		/* if this was the last claim, holder link should go too */
+		if (bdev_free)
 			bd_unlink_disk_holder(bdev);
+
 		mutex_unlock(&bdev->bd_mutex);
 	}
 	return __blkdev_put(bdev, mode, 0);
-- 
1.7.1


  parent reply	other threads:[~2010-11-01 16:17 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-01 16:15 [PATCHSET] block: clean up bdev claim/release handling Tejun Heo
2010-11-01 16:15 ` [PATCH 1/5] mtd: fix bdev exclusive open bugs in block2mtd::add_device() Tejun Heo
2010-11-13 10:38   ` Artem Bityutskiy
2010-11-13 10:42     ` Tejun Heo
2010-11-13 11:10       ` Artem Bityutskiy
2010-11-13 10:59   ` [PATCH UPDATED " Tejun Heo
2010-11-13 11:14     ` Artem Bityutskiy
2010-11-13 11:18       ` Tejun Heo
2010-11-01 16:15 ` [PATCH 2/5] btrfs: close_bdev_exclusive() should use the same @flags as the matching open_bdev_exclusive() Tejun Heo
2010-11-01 16:15 ` [PATCH 3/5] block: simplify holder symlink handling Tejun Heo
2010-11-04 15:06   ` Mike Snitzer
2010-11-01 16:15 ` [PATCH 4/5] block: make blkdev_get/put() handle exclusive access Tejun Heo
2010-11-03 15:06   ` Jan Kara
2010-11-09 10:18     ` Tejun Heo
2010-11-03 16:10   ` Christoph Hellwig
2010-11-04 15:04     ` Mike Snitzer
2010-11-09 10:34     ` Tejun Heo
2010-11-09 10:36       ` Christoph Hellwig
2010-11-01 16:15 ` Tejun Heo [this message]
2010-11-01 19:49 ` [PATCHSET] block: clean up bdev claim/release handling Neil Brown
2010-11-03 15:53 ` [Drbd-dev] " Philipp Reisner
2010-11-11 17:10 ` [PATCH 6/5] block: check bdev_read_only() from blkdev_get() Tejun Heo
2010-11-11 17:11 ` [PATCH 7/5] block: clean up blkdev_get() wrappers and their users Tejun Heo
2010-11-11 19:19   ` Steven Whitehouse
2010-11-11 17:14 ` [PATCHSET] block: clean up bdev claim/release handling Tejun Heo
2010-11-27 16:01   ` Tejun Heo
2010-11-27 18:47     ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1288628129-12811-6-git-send-email-tj@kernel.org \
    --to=tj@kernel.org \
    --cc=adilger.kernel@dilger.ca \
    --cc=aelder@sgi.com \
    --cc=akpm@linux-foundation.org \
    --cc=axboe@kernel.dk \
    --cc=chris.mason@oracle.com \
    --cc=dm-devel@redhat.com \
    --cc=drbd-dev@lists.linbit.com \
    --cc=hch@infradead.org \
    --cc=heiko.carstens@de.ibm.com \
    --cc=jack@suse.cz \
    --cc=joel.becker@oracle.com \
    --cc=joern@logfs.org \
    --cc=konishi.ryusuke@lab.ntt.co.jp \
    --cc=leochen@broadcom.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mfasheh@suse.com \
    --cc=neilb@suse.de \
    --cc=petero2@telia.com \
    --cc=reiserfs-devel@vger.kernel.org \
    --cc=sbranden@broadcom.com \
    --cc=schwidefsky@de.ibm.com \
    --cc=shaggy@linux.vnet.ibm.com \
    --cc=swhiteho@redhat.com \
    --cc=tytso@mit.edu \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox