History

[PATCH] lockdep: fix blkdev_open() warning

On Wed, 2006-08-09 at 07:57 +0200, Rolf Eike Beer wrote: > ============================================= > [ INFO: possible recursive locking detected ] > --------------------------------------------- > parted/7929 is trying to acquire lock: > (&bdev->bd_mutex){--..}, at: [<c105eb8d>] __blkdev_put+0x1e/0x13c > > but task is already holding lock: > (&bdev->bd_mutex){--..}, at: [<c105eec6>] do_open+0x72/0x3a8 > > other info that might help us debug this: > 1 lock held by parted/7929: > #0: (&bdev->bd_mutex){--..}, at: [<c105eec6>] do_open+0x72/0x3a8 > stack backtrace: > [<c1003aad>] show_trace_log_lvl+0x58/0x15b > [<c100495f>] show_trace+0xd/0x10 > [<c1004979>] dump_stack+0x17/0x1a > [<c102dee5>] __lock_acquire+0x753/0x99c > [<c102e3b0>] lock_acquire+0x4a/0x6a > [<c1204501>] mutex_lock_nested+0xc8/0x20c > [<c105eb8d>] __blkdev_put+0x1e/0x13c > [<c105ecc4>] blkdev_put+0xa/0xc > [<c105f18a>] do_open+0x336/0x3a8 > [<c105f21b>] blkdev_open+0x1f/0x4c > [<c1057b40>] __dentry_open+0xc7/0x1aa > [<c1057c91>] nameidata_to_filp+0x1c/0x2e > [<c1057cd1>] do_filp_open+0x2e/0x35 > [<c1057dd7>] do_sys_open+0x38/0x68 > [<c1057e33>] sys_open+0x16/0x18 > [<c1002845>] sysenter_past_esp+0x56/0x8d OK, I'm having a look here; its all new to me so bear with me. blkdev_open() calls do_open(bdev, ...,BD_MUTEX_NORMAL) and takes mutex_lock_nested(&bdev->bd_mutex, BD_MUTEX_NORMAL) then something fails, and we're thrown to: out_first: where if (bdev != bdev->bd_contains) blkdev_put(bdev->bd_contains) which is __blkdev_put(bdev->bd_contains, BD_MUTEX_NORMAL) which does mutex_lock_nested(&bdev->bd_contains->bd_mutex, BD_MUTEX_NORMAL) <--- lockdep trigger When going to out_first, dbev->bd_contains is either bdev or whole, and since we take the branch it must be whole. So it seems to me the following patch would be the right one: [akpm@osdl.org: compile fix] Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl> Cc: Arjan van de Ven <arjan@linux.intel.com> Acked-by: NeilBrown <neilb@suse.de> Signed-off-by: Andrew Morton <akpm@osdl.org> Signed-off-by: Linus Torvalds <torvalds@osdl.org>

Peter Zijlstra committed 20 years ago

Commit 6946bd61 parent 7334bb41 file · +56 −58Browse files

Showing the changes to fs/block_dev.c (1 of 1 file). Show all

Unified · Split

fs/block_dev.c+56−58
@@ -884,6 +884,61 @@ void bd_set_size(struct block_device *bdev, loff_t size) · 883 unmodified lines
884884 }
885885 EXPORT_SYMBOL(bd_set_size);
886886
887+static int __blkdev_put(struct block_device *bdev, unsigned int subclass)
888+{
889+ int ret = 0;
890+ struct inode *bd_inode = bdev->bd_inode;
891+ struct gendisk *disk = bdev->bd_disk;
892+
893+ mutex_lock_nested(&bdev->bd_mutex, subclass);
894+ lock_kernel();
895+ if (!--bdev->bd_openers) {
896+ sync_blockdev(bdev);
897+ kill_bdev(bdev);
898+ }
899+ if (bdev->bd_contains == bdev) {
900+ if (disk->fops->release)
901+ ret = disk->fops->release(bd_inode, NULL);
902+ } else {
903+ mutex_lock_nested(&bdev->bd_contains->bd_mutex,
904+ subclass + 1);
905+ bdev->bd_contains->bd_part_count--;
906+ mutex_unlock(&bdev->bd_contains->bd_mutex);
907+ }
908+ if (!bdev->bd_openers) {
909+ struct module *owner = disk->fops->owner;
910+
911+ put_disk(disk);
912+ module_put(owner);
913+
914+ if (bdev->bd_contains != bdev) {
915+ kobject_put(&bdev->bd_part->kobj);
916+ bdev->bd_part = NULL;
917+ }
918+ bdev->bd_disk = NULL;
919+ bdev->bd_inode->i_data.backing_dev_info = &default_backing_dev_info;
920+ if (bdev != bdev->bd_contains)
921+ __blkdev_put(bdev->bd_contains, subclass + 1);
922+ bdev->bd_contains = NULL;
923+ }
924+ unlock_kernel();
925+ mutex_unlock(&bdev->bd_mutex);
926+ bdput(bdev);
927+ return ret;
928+}
929+
930+int blkdev_put(struct block_device *bdev)
931+{
932+ return __blkdev_put(bdev, BD_MUTEX_NORMAL);
933+}
934+EXPORT_SYMBOL(blkdev_put);
935+
936+int blkdev_put_partition(struct block_device *bdev)
937+{
938+ return __blkdev_put(bdev, BD_MUTEX_PARTITION);
939+}
940+EXPORT_SYMBOL(blkdev_put_partition);
941+
887942 static int
888943 blkdev_get_whole(struct block_device *bdev, mode_t mode, unsigned flags);
889944
@@ -980,7 +1035,7 @@ out_first: · 90 unmodified lines
9801035 bdev->bd_disk = NULL;
9811036 bdev->bd_inode->i_data.backing_dev_info = &default_backing_dev_info;
9821037 if (bdev != bdev->bd_contains)
983− blkdev_put(bdev->bd_contains);
1038+ __blkdev_put(bdev->bd_contains, BD_MUTEX_WHOLE);
9841039 bdev->bd_contains = NULL;
9851040 put_disk(disk);
9861041 module_put(owner);
@@ -1079,63 +1134,6 @@ static int blkdev_open(struct inode * inode, struct file * filp) · 92 unmodified lines
10791134 return res;
10801135 }
10811136
1082−static int __blkdev_put(struct block_device *bdev, unsigned int subclass)
1083−{
1084− int ret = 0;
1085− struct inode *bd_inode = bdev->bd_inode;
1086− struct gendisk *disk = bdev->bd_disk;
1087−
1088− mutex_lock_nested(&bdev->bd_mutex, subclass);
1089− lock_kernel();
1090− if (!--bdev->bd_openers) {
1091− sync_blockdev(bdev);
1092− kill_bdev(bdev);
1093− }
1094− if (bdev->bd_contains == bdev) {
1095− if (disk->fops->release)
1096− ret = disk->fops->release(bd_inode, NULL);
1097− } else {
1098− mutex_lock_nested(&bdev->bd_contains->bd_mutex,
1099− subclass + 1);
1100− bdev->bd_contains->bd_part_count--;
1101− mutex_unlock(&bdev->bd_contains->bd_mutex);
1102− }
1103− if (!bdev->bd_openers) {
1104− struct module *owner = disk->fops->owner;
1105−
1106− put_disk(disk);
1107− module_put(owner);
1108−
1109− if (bdev->bd_contains != bdev) {
1110− kobject_put(&bdev->bd_part->kobj);
1111− bdev->bd_part = NULL;
1112− }
1113− bdev->bd_disk = NULL;
1114− bdev->bd_inode->i_data.backing_dev_info = &default_backing_dev_info;
1115− if (bdev != bdev->bd_contains)
1116− __blkdev_put(bdev->bd_contains, subclass + 1);
1117− bdev->bd_contains = NULL;
1118− }
1119− unlock_kernel();
1120− mutex_unlock(&bdev->bd_mutex);
1121− bdput(bdev);
1122− return ret;
1123−}
1124−
1125−int blkdev_put(struct block_device *bdev)
1126−{
1127− return __blkdev_put(bdev, BD_MUTEX_NORMAL);
1128−}
1129−
1130−EXPORT_SYMBOL(blkdev_put);
1131−
1132−int blkdev_put_partition(struct block_device *bdev)
1133−{
1134− return __blkdev_put(bdev, BD_MUTEX_PARTITION);
1135−}
1136−
1137−EXPORT_SYMBOL(blkdev_put_partition);
1138−
11391137 static int blkdev_close(struct inode * inode, struct file * filp)
11401138 {
11411139 struct block_device *bdev = I_BDEV(filp->f_mapping->host);