From: Peter Zijlstra <a.p.zijlstra@chello.nl>
To: Neil Brown <neilb@suse.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>,
linux-kernel@vger.kernel.org, Ingo Molnar <mingo@elte.hu>,
Arjan van de Ven <arjan@linux.intel.com>,
Andrew Morton <akpm@osdl.org>, Jason Baron <jbaron@redhat.com>
Subject: Re: [PATCH 2/2] new bd_mutex lockdep annotation
Date: Fri, 29 Sep 2006 20:30:55 +0200 [thread overview]
Message-ID: <1159554656.13651.15.camel@lappy> (raw)
In-Reply-To: <1158223815.30737.86.camel@taijtu>
Andrew, please hold on this, Al Viro doesn't agree with it.
Calling get_gendisk() from bdget() changes the bdget() semantics too
much. For one it enables bdget() to load modules.
Al proposed the following approach. Neil can you agree with this too?
---
Avoid the nesting of bd_mutex by serializing the locks. This is made
easier by changing the ->bd_part_count rules, its now only changed for
the first openers/closers.
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 92de28d..0ffc4f0 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -898,12 +889,15 @@ void bd_set_size(struct block_device *bd
}
EXPORT_SYMBOL(bd_set_size);
+static int __blkdev_put(struct block_device *bdev, int part);
+
static int do_open(struct block_device *bdev, struct file *file)
{
struct module *owner = NULL;
struct gendisk *disk;
int ret = -ENXIO;
int part;
+ struct block_device *whole = NULL;
file->f_mapping = bdev->bd_inode->i_mapping;
lock_kernel();
@@ -937,30 +931,42 @@ static int do_open(struct block_device *
rescan_partitions(disk, bdev);
} else {
struct hd_struct *p;
- struct block_device *whole;
+
+ mutex_unlock(&bdev->bd_mutex);
+
whole = bdget_disk(disk, 0);
ret = -ENOMEM;
if (!whole)
- goto out_first;
+ goto out_first_lock;
ret = blkdev_get(whole, file->f_mode, file->f_flags);
if (ret)
- goto out_first;
- bdev->bd_contains = whole;
+ goto out_first_lock;
+
mutex_lock(&whole->bd_mutex);
whole->bd_part_count++;
p = disk->part[part - 1];
- bdev->bd_inode->i_data.backing_dev_info =
- whole->bd_inode->i_data.backing_dev_info;
if (!(disk->flags & GENHD_FL_UP) || !p || !p->nr_sects) {
- whole->bd_part_count--;
mutex_unlock(&whole->bd_mutex);
ret = -ENXIO;
- goto out_first;
+ goto out_first_lock;
}
kobject_get(&p->kobj);
- bdev->bd_part = p;
- bd_set_size(bdev, (loff_t) p->nr_sects << 9);
mutex_unlock(&whole->bd_mutex);
+
+ mutex_lock(&bdev->bd_mutex);
+ if (bdev->bd_contains != whole) {
+ bdev->bd_contains = whole;
+ bdev->bd_inode->i_data.backing_dev_info =
+ whole->bd_inode->i_data.backing_dev_info;
+ bdev->bd_part = p;
+ bd_set_size(bdev, (loff_t) p->nr_sects << 9);
+ whole = NULL;
+ } else {
+ mutex_unlock(&bdev->bd_mutex);
+ kobject_put(&p->kobj);
+ __blkdev_put(whole, 1);
+ mutex_lock(&bdev->bd_mutex);
+ }
}
} else {
put_disk(disk);
@@ -973,10 +979,6 @@ static int do_open(struct block_device *
}
if (bdev->bd_invalidated)
rescan_partitions(bdev->bd_disk, bdev);
- } else {
- mutex_lock(&bdev->bd_contains->bd_mutex);
- bdev->bd_contains->bd_part_count++;
- mutex_unlock(&bdev->bd_contains->bd_mutex);
}
}
bdev->bd_openers++;
@@ -984,11 +986,12 @@ static int do_open(struct block_device *
unlock_kernel();
return 0;
+out_first_lock:
+ if (whole)
+ __blkdev_put(whole, 1);
+ mutex_lock(&bdev->bd_mutex);
out_first:
bdev->bd_disk = NULL;
- bdev->bd_inode->i_data.backing_dev_info = &default_backing_dev_info;
- if (bdev != bdev->bd_contains)
- blkdev_put(bdev->bd_contains);
bdev->bd_contains = NULL;
put_disk(disk);
module_put(owner);
@@ -1049,14 +1052,17 @@ static int blkdev_open(struct inode * in
return res;
}
-int blkdev_put(struct block_device *bdev)
+static int __blkdev_put(struct block_device *bdev, int part)
{
int ret = 0;
struct inode *bd_inode = bdev->bd_inode;
struct gendisk *disk = bdev->bd_disk;
+ struct block_device *victim = NULL;
mutex_lock(&bdev->bd_mutex);
lock_kernel();
+ if (part)
+ bdev->bd_part_count--;
if (!--bdev->bd_openers) {
sync_blockdev(bdev);
kill_bdev(bdev);
@@ -1064,10 +1070,6 @@ int blkdev_put(struct block_device *bdev
if (bdev->bd_contains == bdev) {
if (disk->fops->release)
ret = disk->fops->release(bd_inode, NULL);
- } else {
- mutex_lock(&bdev->bd_contains->bd_mutex);
- bdev->bd_contains->bd_part_count--;
- mutex_unlock(&bdev->bd_contains->bd_mutex);
}
if (!bdev->bd_openers) {
struct module *owner = disk->fops->owner;
@@ -1081,17 +1083,23 @@ int blkdev_put(struct block_device *bdev
}
bdev->bd_disk = NULL;
bdev->bd_inode->i_data.backing_dev_info = &default_backing_dev_info;
- if (bdev != bdev->bd_contains) {
- blkdev_put(bdev->bd_contains);
- }
+ if (bdev != bdev->bd_contains)
+ victim = bdev->bd_contains;
bdev->bd_contains = NULL;
}
unlock_kernel();
mutex_unlock(&bdev->bd_mutex);
+ if (victim)
+ __blkdev_put(victim, 1);
bdput(bdev);
return ret;
}
+int blkdev_put(struct block_device *bdev)
+{
+ return __blkdev_put(bdev, 0);
+}
+
EXPORT_SYMBOL(blkdev_put);
static int blkdev_close(struct inode * inode, struct file * filp)
prev parent reply other threads:[~2006-09-29 18:31 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-09-13 17:43 [PATCH 0/2] new bd_mutex lockdep annotation Peter Zijlstra
2006-09-13 17:43 ` [PATCH 1/2] remove the old " Peter Zijlstra
2006-09-13 17:43 ` [PATCH 2/2] new " Peter Zijlstra
2006-09-13 18:15 ` Arjan van de Ven
2006-09-14 7:11 ` Neil Brown
2006-09-14 8:50 ` Peter Zijlstra
2006-09-29 18:30 ` Peter Zijlstra [this message]
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=1159554656.13651.15.camel@lappy \
--to=a.p.zijlstra@chello.nl \
--cc=akpm@osdl.org \
--cc=arjan@linux.intel.com \
--cc=jbaron@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=neilb@suse.de \
--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