lkml.org 
[lkml]   [2006]   [Sep]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH 2/2] new bd_mutex lockdep annotation
From
Date
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)

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-09-29 20:33    [W:0.037 / U:0.504 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site