All of lore.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: [Drbd-dev] [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


WARNING: multiple messages have this Message-ID (diff)
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-02 10:38 UTC|newest]

Thread overview: 80+ 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 ` Tejun Heo
2010-11-01 16:15 ` [Drbd-dev] " Tejun Heo
2010-11-01 16:15 ` [PATCH 1/5] mtd: fix bdev exclusive open bugs in block2mtd::add_device() Tejun Heo
2010-11-01 16:15   ` [Drbd-dev] " Tejun Heo
2010-11-13 10:38   ` Artem Bityutskiy
2010-11-13 10:38     ` Artem Bityutskiy
2010-11-13 10:38     ` Artem Bityutskiy
2010-11-13 10:42     ` Tejun Heo
2010-11-13 10:42       ` [Drbd-dev] " Tejun Heo
2010-11-13 10:42       ` Tejun Heo
2010-11-13 11:10       ` Artem Bityutskiy
2010-11-13 11:10         ` Artem Bityutskiy
2010-11-13 11:10         ` Artem Bityutskiy
2010-11-13 10:59   ` [PATCH UPDATED " Tejun Heo
2010-11-13 10:59     ` [Drbd-dev] " Tejun Heo
2010-11-13 10:59     ` Tejun Heo
2010-11-13 11:14     ` Artem Bityutskiy
2010-11-13 11:14       ` Artem Bityutskiy
2010-11-13 11:14       ` Artem Bityutskiy
2010-11-13 11:18       ` Tejun Heo
2010-11-13 11:18         ` [Drbd-dev] " Tejun Heo
2010-11-13 11:18         ` Tejun Heo
2010-11-01 16:15 ` [PATCH " Tejun Heo
2010-11-01 16:15 ` Tejun Heo
2010-11-01 16:15 ` [Drbd-dev] [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   ` Tejun Heo
2010-11-01 16:15 ` Tejun Heo
2010-11-01 16:15 ` Tejun Heo
2010-11-01 16:15 ` [Drbd-dev] [PATCH 3/5] block: simplify holder symlink handling Tejun Heo
2010-11-01 16:15   ` Tejun Heo
2010-11-04 15:06   ` Mike Snitzer
2010-11-04 15:06     ` Mike Snitzer
2010-11-04 15:06     ` [Drbd-dev] " Mike Snitzer
2010-11-01 16:15 ` Tejun Heo
2010-11-01 16:15 ` Tejun Heo
2010-11-01 16:15 ` [PATCH 4/5] block: make blkdev_get/put() handle exclusive access Tejun Heo
2010-11-01 16:15 ` Tejun Heo
2010-11-01 16:15   ` Tejun Heo
2010-11-01 16:15   ` [Drbd-dev] " Tejun Heo
2010-11-03 15:06   ` Jan Kara
2010-11-03 15:06     ` [Drbd-dev] " Jan Kara
2010-11-09 10:18     ` Tejun Heo
2010-11-09 10:18       ` [Drbd-dev] " Tejun Heo
2010-11-03 16:10   ` Christoph Hellwig
2010-11-03 16:10     ` [Drbd-dev] " Christoph Hellwig
2010-11-04 15:04     ` Mike Snitzer
2010-11-04 15:04       ` Mike Snitzer
2010-11-04 15:04       ` [Drbd-dev] " Mike Snitzer
2010-11-09 10:34     ` Tejun Heo
2010-11-09 10:34       ` [Drbd-dev] " Tejun Heo
2010-11-09 10:36       ` Christoph Hellwig
2010-11-09 10:36         ` [Drbd-dev] " Christoph Hellwig
2010-11-01 16:15 ` Tejun Heo [this message]
2010-11-01 16:15   ` [PATCH 5/5] block: reorganize claim/release implementation Tejun Heo
2010-11-01 16:15 ` Tejun Heo
2010-11-01 16:15 ` Tejun Heo
2010-11-01 19:49 ` [PATCHSET] block: clean up bdev claim/release handling Neil Brown
2010-11-01 19:49   ` [Drbd-dev] " Neil Brown
2010-11-03 15:53 ` Philipp Reisner
2010-11-03 15:53   ` Philipp Reisner
2010-11-11 17:10 ` [PATCH 6/5] block: check bdev_read_only() from blkdev_get() Tejun Heo
2010-11-11 17:10   ` Tejun Heo
2010-11-11 17:10   ` [Drbd-dev] " Tejun Heo
2010-11-11 17:10 ` Tejun Heo
2010-11-11 17:11 ` [PATCH 7/5] block: clean up blkdev_get() wrappers and their users Tejun Heo
2010-11-11 17:11   ` Tejun Heo
2010-11-11 17:11   ` [Drbd-dev] " Tejun Heo
2010-11-11 19:19   ` Steven Whitehouse
2010-11-11 17:11 ` Tejun Heo
2010-11-11 17:14 ` [PATCHSET] block: clean up bdev claim/release handling Tejun Heo
2010-11-11 17:14   ` Tejun Heo
2010-11-11 17:14   ` [Drbd-dev] " Tejun Heo
2010-11-27 16:01   ` Tejun Heo
2010-11-27 16:01     ` Tejun Heo
2010-11-27 16:01     ` [Drbd-dev] " Tejun Heo
2010-11-27 18:47     ` Jens Axboe
2010-11-27 18:47       ` [Drbd-dev] " Jens Axboe
2010-11-27 16:01   ` Tejun Heo
2010-11-11 17:14 ` Tejun Heo

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.