block: restore multiple bd_link_disk_holder() support

Commit e09b457b (block: simplify holder symlink handling) incorrectly assumed that there is only one link at maximum. dm may use multiple links and expects block layer to track reference count for each link, which is different from and unrelated to the exclusive device holder identified by @holder when the device is opened. Remove the single holder assumption and automatic removal of the link and revive the per-link reference count tracking. The code essentially behaves the same as before commit e09b457b sans the unnecessary kobject reference count dancing. While at it, note that this facility should not be used by anyone else than the current ones. Sysfs symlinks shouldn't be abused like this and the whole thing doesn't belong in the block layer at all. Signed-off-by: Tejun Heo <tj@kernel.org> Reported-by: Milan Broz <mbroz@redhat.com> Cc: Jun'ichi Nomura <j-nomura@ce.jp.nec.com> Cc: Neil Brown <neilb@suse.de> Cc: linux-raid@vger.kernel.org Cc: Kay Sievers <kay.sievers@vrfy.org> Signed-off-by: Jens Axboe <jaxboe@fusionio.com>

block: restore multiple bd_link_disk_holder() support
Commit e09b457b (block: simplify holder symlink handling) incorrectly assumed that there is only one link at maximum. dm may use multiple links and expects block layer to track reference count for each link, which is different from and unrelated to the exclusive device holder identified by @holder when the device is opened. Remove the single holder assumption and automatic removal of the link and revive the per-link reference count tracking. The code essentially behaves the same as before commit e09b457b sans the unnecessary kobject reference count dancing. While at it, note that this facility should not be used by anyone else than the current ones. Sysfs symlinks shouldn't be abused like this and the whole thing doesn't belong in the block layer at all. Signed-off-by: Tejun Heo <tj@kernel.org> Reported-by: Milan Broz <mbroz@redhat.com> Cc: Jun'ichi Nomura <j-nomura@ce.jp.nec.com> Cc: Neil Brown <neilb@suse.de> Cc: linux-raid@vger.kernel.org Cc: Kay Sievers <kay.sievers@vrfy.org> Signed-off-by: Jens Axboe <jaxboe@fusionio.com>
Tejun Heo · Jens Axboe
1 parent c553f8e335
Showing 4 changed files with 84 additions and 19 deletions Side-by-side Diff
drivers/md/dm-table.c
drivers/md/md.c
fs/block_dev.c
include/linux/fs.h
@@ -350,6 +350,7 @@
 	if (!d->dm_dev.bdev)
 		return;
  
+	bd_unlink_disk_holder(d->dm_dev.bdev, dm_disk(md));
 	blkdev_put(d->dm_dev.bdev, d->dm_dev.mode | FMODE_EXCL);
 	d->dm_dev.bdev = NULL;
 }
@@ -1912,6 +1912,7 @@
 		MD_BUG();
 		return;
 	}
+	bd_unlink_disk_holder(rdev->bdev, rdev->mddev->gendisk);
 	list_del_rcu(&rdev->same_set);
 	printk(KERN_INFO "md: unbind<%s>\n", bdevname(rdev->bdev,b));
 	rdev->mddev = NULL;
@@ -432,6 +432,9 @@
 	mutex_init(&bdev->bd_mutex);
 	INIT_LIST_HEAD(&bdev->bd_inodes);
 	INIT_LIST_HEAD(&bdev->bd_list);
+#ifdef CONFIG_SYSFS
+	INIT_LIST_HEAD(&bdev->bd_holder_disks);
+#endif
 	inode_init_once(&ei->vfs_inode);
 	/* Initialize mutex for freeze. */
 	mutex_init(&bdev->bd_fsfreeze_mutex);
@@ -779,6 +782,23 @@
 }
  
 #ifdef CONFIG_SYSFS
+struct bd_holder_disk {
+	struct list_head	list;
+	struct gendisk		*disk;
+	int			refcnt;
+};
+
+static struct bd_holder_disk *bd_find_holder_disk(struct block_device *bdev,
+						  struct gendisk *disk)
+{
+	struct bd_holder_disk *holder;
+
+	list_for_each_entry(holder, &bdev->bd_holder_disks, list)
+		if (holder->disk == disk)
+			return holder;
+	return NULL;
+}
+
 static int add_symlink(struct kobject *from, struct kobject *to)
 {
 	return sysfs_create_link(from, to, kobject_name(to));
@@ -794,6 +814,8 @@
  * @bdev: the claimed slave bdev
  * @disk: the holding disk
  *
+ * DON'T USE THIS UNLESS YOU'RE ALREADY USING IT.
+ *
  * This functions creates the following sysfs symlinks.
  *
  * - from "slaves" directory of the holder @disk to the claimed @bdev
  
  
  
  
  
  
  
  
  
  
@@ -817,47 +839,83 @@
  */
 int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk)
 {
+	struct bd_holder_disk *holder;
 	int ret = 0;
  
 	mutex_lock(&bdev->bd_mutex);
  
-	WARN_ON_ONCE(!bdev->bd_holder || bdev->bd_holder_disk);
+	WARN_ON_ONCE(!bdev->bd_holder);
  
 	/* FIXME: remove the following once add_disk() handles errors */
 	if (WARN_ON(!disk->slave_dir || !bdev->bd_part->holder_dir))
 		goto out_unlock;
  
-	ret = add_symlink(disk->slave_dir, &part_to_dev(bdev->bd_part)->kobj);
-	if (ret)
+	holder = bd_find_holder_disk(bdev, disk);
+	if (holder) {
+		holder->refcnt++;
 		goto out_unlock;
+	}
  
-	ret = add_symlink(bdev->bd_part->holder_dir, &disk_to_dev(disk)->kobj);
-	if (ret) {
-		del_symlink(disk->slave_dir, &part_to_dev(bdev->bd_part)->kobj);
+	holder = kzalloc(sizeof(*holder), GFP_KERNEL);
+	if (!holder) {
+		ret = -ENOMEM;
 		goto out_unlock;
 	}
  
-	bdev->bd_holder_disk = disk;
+	INIT_LIST_HEAD(&holder->list);
+	holder->disk = disk;
+	holder->refcnt = 1;
+
+	ret = add_symlink(disk->slave_dir, &part_to_dev(bdev->bd_part)->kobj);
+	if (ret)
+		goto out_free;
+
+	ret = add_symlink(bdev->bd_part->holder_dir, &disk_to_dev(disk)->kobj);
+	if (ret)
+		goto out_del;
+
+	list_add(&holder->list, &bdev->bd_holder_disks);
+	goto out_unlock;
+
+out_del:
+	del_symlink(disk->slave_dir, &part_to_dev(bdev->bd_part)->kobj);
+out_free:
+	kfree(holder);
 out_unlock:
 	mutex_unlock(&bdev->bd_mutex);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(bd_link_disk_holder);
  
-static void bd_unlink_disk_holder(struct block_device *bdev)
+/**
+ * bd_unlink_disk_holder - destroy symlinks created by bd_link_disk_holder()
+ * @bdev: the calimed slave bdev
+ * @disk: the holding disk
+ *
+ * DON'T USE THIS UNLESS YOU'RE ALREADY USING IT.
+ *
+ * CONTEXT:
+ * Might sleep.
+ */
+void bd_unlink_disk_holder(struct block_device *bdev, struct gendisk *disk)
 {
-	struct gendisk *disk = bdev->bd_holder_disk;
+	struct bd_holder_disk *holder;
  
-	bdev->bd_holder_disk = NULL;
-	if (!disk)
-		return;
+	mutex_lock(&bdev->bd_mutex);
  
-	del_symlink(disk->slave_dir, &part_to_dev(bdev->bd_part)->kobj);
-	del_symlink(bdev->bd_part->holder_dir, &disk_to_dev(disk)->kobj);
+	holder = bd_find_holder_disk(bdev, disk);
+
+	if (!WARN_ON_ONCE(holder == NULL) && !--holder->refcnt) {
+		del_symlink(disk->slave_dir, &part_to_dev(bdev->bd_part)->kobj);
+		del_symlink(bdev->bd_part->holder_dir,
+			    &disk_to_dev(disk)->kobj);
+		list_del_init(&holder->list);
+		kfree(holder);
+	}
+
+	mutex_unlock(&bdev->bd_mutex);
 }
-#else
-static inline void bd_unlink_disk_holder(struct block_device *bdev)
-{ }
+EXPORT_SYMBOL_GPL(bd_unlink_disk_holder);
 #endif
  
 /**
@@ -1380,7 +1438,6 @@
 		 * unblock evpoll if it was a write holder.
 		 */
 		if (bdev_free) {
-			bd_unlink_disk_holder(bdev);
 			if (bdev->bd_write_holder) {
 				disk_unblock_events(bdev->bd_disk);
 				bdev->bd_write_holder = false;
@@ -666,7 +666,7 @@
 	int			bd_holders;
 	bool			bd_write_holder;
 #ifdef CONFIG_SYSFS
-	struct gendisk *	bd_holder_disk;	/* for sysfs slave linkng */
+	struct list_head	bd_holder_disks;
 #endif
 	struct block_device *	bd_contains;
 	unsigned		bd_block_size;
  
@@ -2058,11 +2058,17 @@
 extern int blkdev_put(struct block_device *bdev, fmode_t mode);
 #ifdef CONFIG_SYSFS
 extern int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk);
+extern void bd_unlink_disk_holder(struct block_device *bdev,
+				  struct gendisk *disk);
 #else
 static inline int bd_link_disk_holder(struct block_device *bdev,
 				      struct gendisk *disk)
 {
 	return 0;
+}
+static inline void bd_unlink_disk_holder(struct block_device *bdev,
+					 struct gendisk *disk)
+{
 }
 #endif
 #endif
...	...	@@ -350,6 +350,7 @@
350	350	if (!d->dm_dev.bdev)
351	351	return;
352	352
	353	+ bd_unlink_disk_holder(d->dm_dev.bdev, dm_disk(md));
353	354	blkdev_put(d->dm_dev.bdev, d->dm_dev.mode \| FMODE_EXCL);
354	355	d->dm_dev.bdev = NULL;
355	356	}
...	...	@@ -1912,6 +1912,7 @@
1912	1912	MD_BUG();
1913	1913	return;
1914	1914	}
	1915	+ bd_unlink_disk_holder(rdev->bdev, rdev->mddev->gendisk);
1915	1916	list_del_rcu(&rdev->same_set);
1916	1917	printk(KERN_INFO "md: unbind<%s>\n", bdevname(rdev->bdev,b));
1917	1918	rdev->mddev = NULL;
...	...	@@ -432,6 +432,9 @@
432	432	mutex_init(&bdev->bd_mutex);
433	433	INIT_LIST_HEAD(&bdev->bd_inodes);
434	434	INIT_LIST_HEAD(&bdev->bd_list);
	435	+#ifdef CONFIG_SYSFS
	436	+ INIT_LIST_HEAD(&bdev->bd_holder_disks);
	437	+#endif
435	438	inode_init_once(&ei->vfs_inode);
436	439	/* Initialize mutex for freeze. */
437	440	mutex_init(&bdev->bd_fsfreeze_mutex);
...	...	@@ -779,6 +782,23 @@
779	782	}
780	783
781	784	#ifdef CONFIG_SYSFS
	785	+struct bd_holder_disk {
	786	+ struct list_head list;
	787	+ struct gendisk *disk;
	788	+ int refcnt;
	789	+};
	790	+
	791	+static struct bd_holder_disk bd_find_holder_disk(struct block_device bdev,
	792	+ struct gendisk *disk)
	793	+{
	794	+ struct bd_holder_disk *holder;
	795	+
	796	+ list_for_each_entry(holder, &bdev->bd_holder_disks, list)
	797	+ if (holder->disk == disk)
	798	+ return holder;
	799	+ return NULL;
	800	+}
	801	+
782	802	static int add_symlink(struct kobject from, struct kobject to)
783	803	{
784	804	return sysfs_create_link(from, to, kobject_name(to));
...	...	@@ -794,6 +814,8 @@
794	814	* @bdev: the claimed slave bdev
795	815	* @disk: the holding disk
796	816	*
	817	+ * DON'T USE THIS UNLESS YOU'RE ALREADY USING IT.
	818	+ *
797	819	* This functions creates the following sysfs symlinks.
798	820	*
799	821	* - from "slaves" directory of the holder @disk to the claimed @bdev
800	822
801	823
802	824
803	825
804	826
805	827
806	828
807	829
808	830
809	831
...	...	@@ -817,47 +839,83 @@
817	839	*/
818	840	int bd_link_disk_holder(struct block_device bdev, struct gendisk disk)
819	841	{
	842	+ struct bd_holder_disk *holder;
820	843	int ret = 0;
821	844
822	845	mutex_lock(&bdev->bd_mutex);
823	846
824		- WARN_ON_ONCE(!bdev->bd_holder \|\| bdev->bd_holder_disk);
	847	+ WARN_ON_ONCE(!bdev->bd_holder);
825	848
826	849	/* FIXME: remove the following once add_disk() handles errors */
827	850	if (WARN_ON(!disk->slave_dir \|\| !bdev->bd_part->holder_dir))
828	851	goto out_unlock;
829	852
830		- ret = add_symlink(disk->slave_dir, &part_to_dev(bdev->bd_part)->kobj);
831		- if (ret)
	853	+ holder = bd_find_holder_disk(bdev, disk);
	854	+ if (holder) {
	855	+ holder->refcnt++;
832	856	goto out_unlock;
	857	+ }
833	858
834		- ret = add_symlink(bdev->bd_part->holder_dir, &disk_to_dev(disk)->kobj);
835		- if (ret) {
836		- del_symlink(disk->slave_dir, &part_to_dev(bdev->bd_part)->kobj);
	859	+ holder = kzalloc(sizeof(*holder), GFP_KERNEL);
	860	+ if (!holder) {
	861	+ ret = -ENOMEM;
837	862	goto out_unlock;
838	863	}
839	864
840		- bdev->bd_holder_disk = disk;
	865	+ INIT_LIST_HEAD(&holder->list);
	866	+ holder->disk = disk;
	867	+ holder->refcnt = 1;
	868	+
	869	+ ret = add_symlink(disk->slave_dir, &part_to_dev(bdev->bd_part)->kobj);
	870	+ if (ret)
	871	+ goto out_free;
	872	+
	873	+ ret = add_symlink(bdev->bd_part->holder_dir, &disk_to_dev(disk)->kobj);
	874	+ if (ret)
	875	+ goto out_del;
	876	+
	877	+ list_add(&holder->list, &bdev->bd_holder_disks);
	878	+ goto out_unlock;
	879	+
	880	+out_del:
	881	+ del_symlink(disk->slave_dir, &part_to_dev(bdev->bd_part)->kobj);
	882	+out_free:
	883	+ kfree(holder);
841	884	out_unlock:
842	885	mutex_unlock(&bdev->bd_mutex);
843	886	return ret;
844	887	}
845	888	EXPORT_SYMBOL_GPL(bd_link_disk_holder);
846	889
847		-static void bd_unlink_disk_holder(struct block_device *bdev)
	890	+/**
	891	+ * bd_unlink_disk_holder - destroy symlinks created by bd_link_disk_holder()
	892	+ * @bdev: the calimed slave bdev
	893	+ * @disk: the holding disk
	894	+ *
	895	+ * DON'T USE THIS UNLESS YOU'RE ALREADY USING IT.
	896	+ *
	897	+ * CONTEXT:
	898	+ * Might sleep.
	899	+ */
	900	+void bd_unlink_disk_holder(struct block_device bdev, struct gendisk disk)
848	901	{
849		- struct gendisk *disk = bdev->bd_holder_disk;
	902	+ struct bd_holder_disk *holder;
850	903
851		- bdev->bd_holder_disk = NULL;
852		- if (!disk)
853		- return;
	904	+ mutex_lock(&bdev->bd_mutex);
854	905
855		- del_symlink(disk->slave_dir, &part_to_dev(bdev->bd_part)->kobj);
856		- del_symlink(bdev->bd_part->holder_dir, &disk_to_dev(disk)->kobj);
	906	+ holder = bd_find_holder_disk(bdev, disk);
	907	+
	908	+ if (!WARN_ON_ONCE(holder == NULL) && !--holder->refcnt) {
	909	+ del_symlink(disk->slave_dir, &part_to_dev(bdev->bd_part)->kobj);
	910	+ del_symlink(bdev->bd_part->holder_dir,
	911	+ &disk_to_dev(disk)->kobj);
	912	+ list_del_init(&holder->list);
	913	+ kfree(holder);
	914	+ }
	915	+
	916	+ mutex_unlock(&bdev->bd_mutex);
857	917	}
858		-#else
859		-static inline void bd_unlink_disk_holder(struct block_device *bdev)
860		-{ }
	918	+EXPORT_SYMBOL_GPL(bd_unlink_disk_holder);
861	919	#endif
862	920
863	921	/**
...	...	@@ -1380,7 +1438,6 @@
1380	1438	* unblock evpoll if it was a write holder.
1381	1439	*/
1382	1440	if (bdev_free) {
1383		- bd_unlink_disk_holder(bdev);
1384	1441	if (bdev->bd_write_holder) {
1385	1442	disk_unblock_events(bdev->bd_disk);
1386	1443	bdev->bd_write_holder = false;
...	...	@@ -666,7 +666,7 @@
666	666	int bd_holders;
667	667	bool bd_write_holder;
668	668	#ifdef CONFIG_SYSFS
669		- struct gendisk * bd_holder_disk; /* for sysfs slave linkng */
	669	+ struct list_head bd_holder_disks;
670	670	#endif
671	671	struct block_device * bd_contains;
672	672	unsigned bd_block_size;
673	673
...	...	@@ -2058,11 +2058,17 @@
2058	2058	extern int blkdev_put(struct block_device *bdev, fmode_t mode);
2059	2059	#ifdef CONFIG_SYSFS
2060	2060	extern int bd_link_disk_holder(struct block_device bdev, struct gendisk disk);
	2061	+extern void bd_unlink_disk_holder(struct block_device *bdev,
	2062	+ struct gendisk *disk);
2061	2063	#else
2062	2064	static inline int bd_link_disk_holder(struct block_device *bdev,
2063	2065	struct gendisk *disk)
2064	2066	{
2065	2067	return 0;
	2068	+}
	2069	+static inline void bd_unlink_disk_holder(struct block_device *bdev,
	2070	+ struct gendisk *disk)
	2071	+{
2066	2072	}
2067	2073	#endif
2068	2074	#endif