[RFE] Add a new super operation for finding block devices in a super

[RFE] Add a new super operation for finding block devices in a super

[Josef Bacik]

Btrfs can span multiple devices, so sb->s_bdev will not always point to the
block device that we are looking to freeze.  This isn't a huge issue since the
only thing that freezes per-bdev is device-mapper, but if btrfs spans multiple
dm devices then we could run into corruption issues as we could end up not
actually freezing the super properly.  So enter contains_bdev.  This will be
used by get_active_super, if the sb implements contains_bdev we just ask the fs
if it has the bdev we want and if so we are good to go.  I'm not married to the
name, so if anybody has any better suggestions I'm open to them.  Comments?
Thanks,

Josef

[PATCH 1/2] fs: add super operation contains_bdev

[Josef Bacik]

Using get_active_super will not work properly if the fs (like btrfs) does not
save it's s_bdev with the device it is on.  Also it doesn't provide the entire
picture, since an fs can be contained on multiple disks (again like btrfs).  So
add a super operation that will check to see if the given block device is
contained within the given super.  This will mean that if we try to something
like a snapshot of a lv with btrfs on the lv the fs will actually get properly
frozen.  Thanks,

Signed-off-by: Josef Bacik <josef@redhat.com>
---
 Documentation/filesystems/vfs.txt |    6 ++++++
 fs/super.c                        |   16 ++++++++++++++--
 include/linux/fs.h                |    1 +
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index 94cf97b..ce22988 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -221,6 +221,7 @@ struct super_operations {
 
         ssize_t (*quota_read)(struct super_block *, int, char *, size_t, loff_t);
         ssize_t (*quota_write)(struct super_block *, int, const char *, size_t, loff_t);
+        int (*contains_bdev)(struct super_block *, struct block_device *);
 };
 
 All methods are called without any locks being held, unless otherwise
@@ -293,6 +294,11 @@ or bottom half).
 
   quota_write: called by the VFS to write to filesystem quota file.
 
+  contains_bdev: called by the VFS to see if the filesystem contains the given
+	block_device.  Return 1 if it does or 0 if it does not.  Only intended
+	for filesystems that can have several underlying block devices.
+	Optional.
+
 Whoever sets up the inode is responsible for filling in the "i_op" field. This
 is a pointer to a "struct inode_operations" which describes the methods that
 can be performed on individual inodes.
diff --git a/fs/super.c b/fs/super.c
index 7e9dd4c..43185fa 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -497,7 +497,7 @@ EXPORT_SYMBOL(get_super);
  */
 struct super_block *get_active_super(struct block_device *bdev)
 {
-	struct super_block *sb;
+	struct super_block *sb, *p = NULL;
 
 	if (!bdev)
 		return NULL;
@@ -507,13 +507,25 @@ restart:
 	list_for_each_entry(sb, &super_blocks, s_list) {
 		if (list_empty(&sb->s_instances))
 			continue;
-		if (sb->s_bdev == bdev) {
+
+		if (sb->s_op->contains_bdev) {
+			if (!grab_super(sb))
+				goto restart;
+			if (sb->s_op->contains_bdev(sb, bdev))
+				return sb;
+			spin_lock(&sb_lock);
+			if (p)
+				__put_super(p);
+			p = sb;
+		} else if (sb->s_bdev == bdev) {
 			if (grab_super(sb)) /* drops sb_lock */
 				return sb;
 			else
 				goto restart;
 		}
 	}
+	if (p)
+		__put_super(p);
 	spin_unlock(&sb_lock);
 	return NULL;
 }
diff --git a/include/linux/fs.h b/include/linux/fs.h
index e38b50a..fe12b7b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1627,6 +1627,7 @@ struct super_operations {
 	ssize_t (*quota_write)(struct super_block *, int, const char *, size_t, loff_t);
 #endif
 	int (*bdev_try_to_free_page)(struct super_block*, struct page*, gfp_t);
+	int (*contains_bdev)(struct super_block *, struct block_device *);
 };
 
 /*
-- 
1.7.2.3

[PATCH 2/2] btrfs: implement super op contains_bdev

[Josef Bacik]

This patch implements the super op contains_bdev.  We just loop through the
fs_devices on this super looking for the block device that was requested.
Thanks,

Signed-off-by: Josef Bacik <josef@redhat.com>
---
 fs/btrfs/super.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index d39a989..1487bef 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1095,6 +1095,26 @@ static int btrfs_unfreeze(struct super_block *sb)
 	return 0;
 }
 
+static int btrfs_contains_bdev(struct super_block *sb,
+			       struct block_device *bdev)
+{
+	struct btrfs_root *root = btrfs_sb(sb);
+	struct btrfs_fs_devices *fs_devices = root->fs_info->fs_devices;
+	struct btrfs_device *device;
+	int ret = 0;
+
+	mutex_lock(&fs_devices->device_list_mutex);
+	list_for_each_entry(device, &fs_devices->devices, dev_list) {
+		if (device->bdev == bdev) {
+			ret = 1;
+			break;
+		}
+	}
+	mutex_unlock(&fs_devices->device_list_mutex);
+
+	return ret;
+}
+
 static const struct super_operations btrfs_super_ops = {
 	.drop_inode	= btrfs_drop_inode,
 	.evict_inode	= btrfs_evict_inode,
@@ -1109,6 +1129,7 @@ static const struct super_operations btrfs_super_ops = {
 	.remount_fs	= btrfs_remount,
 	.freeze_fs	= btrfs_freeze,
 	.unfreeze_fs	= btrfs_unfreeze,
+	.contains_bdev	= btrfs_contains_bdev,
 };
 
 static const struct file_operations btrfs_ctl_fops = {
-- 
1.7.2.3

Re: [RFE] Add a new super operation for finding block devices in a super

[Christoph Hellwig]

I think there's a better and more efficient way to archive this.

We already have a bd_super field in struct block_device.  Just
generalize it, and use it from the freeze code instead of doing the
get_active_super loop. 

Re: [RFE] Add a new super operation for finding block devices in a super

[Josef Bacik]

On Mon, Mar 14, 2011 at 12:31:26PM -0400, Christoph Hellwig wrote:
> 
> I think there's a better and more efficient way to archive this.
> 
> We already have a bd_super field in struct block_device.  Just
> generalize it, and use it from the freeze code instead of doing the
> get_active_super loop. 
> 

Great I will do that, thanks,

Josef