All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC 0/5] Scan all devices to build fs device list
@ 2014-09-03 13:36 Miao Xie
  2014-09-03 13:36 ` [PATCH 1/5] block: export disk_class and disk_type for btrfs Miao Xie
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Miao Xie @ 2014-09-03 13:36 UTC (permalink / raw)
  To: linux-btrfs

This patchset implements device list automatic building function. As we
know, currently we need scan the devices to build device list by a user tool
before mounting the filesystem, especially mount the filesystem after
we re-install btrfs module. It is not convenient. This patchset can improve
that problem. With this patchset, we will scan all the devices in the
system to build the device list if we find the number of the devices
is not right when we mount the filesystem. By this way, we needn't scan
the device by the user tool and reduce the mount failure probability due
to the incomplete device list.

---
Miao Xie (5):
  block: export disk_class and disk_type for btrfs
  Btrfs: don't return btrfs_fs_devices if the caller doesn't want it
  Btrfs: restructure btrfs_scan_one_device
  Btrfs: restructure btrfs_get_bdev_and_sb and pick up some code used
    later
  Btrfs: scan all the devices and build the fs device list by btrfs's
    self

 block/genhd.c         |   7 +-
 fs/btrfs/super.c      |   3 +
 fs/btrfs/volumes.c    | 227 ++++++++++++++++++++++++++++++++++++--------------
 fs/btrfs/volumes.h    |   5 +-
 include/linux/genhd.h |   1 +
 5 files changed, 177 insertions(+), 66 deletions(-)

-- 
1.9.3


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/5] block: export disk_class and disk_type for btrfs
  2014-09-03 13:36 [PATCH RFC 0/5] Scan all devices to build fs device list Miao Xie
@ 2014-09-03 13:36 ` Miao Xie
  2014-09-03 13:36 ` [PATCH 2/5] Btrfs: don't return btrfs_fs_devices if the caller doesn't want it Miao Xie
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Miao Xie @ 2014-09-03 13:36 UTC (permalink / raw)
  To: linux-btrfs

Btrfs can make filesystem cross several disks/partitions, in order to
load all the disks/partitions which belong to the same filesystem, we
need scan the system and find all the devices, and then register them
into the kernel. Currently, we do it by user tool. But if we forget to
do it, we can not mount the filesystem. So I want btrfs scan the system
and find all the devices by itself in the kernel. In order to implement
it, we need disk_class and disk_type, so export them.

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 block/genhd.c         | 7 +++++--
 include/linux/genhd.h | 1 +
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 791f419..8371c09 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -34,7 +34,7 @@ struct kobject *block_depr;
 static DEFINE_MUTEX(ext_devt_mutex);
 static DEFINE_IDR(ext_devt_idr);
 
-static struct device_type disk_type;
+struct device_type disk_type;
 
 static void disk_check_events(struct disk_events *ev,
 			      unsigned int *clearing_ptr);
@@ -1107,9 +1107,11 @@ static void disk_release(struct device *dev)
 		blk_put_queue(disk->queue);
 	kfree(disk);
 }
+
 struct class block_class = {
 	.name		= "block",
 };
+EXPORT_SYMBOL(block_class);
 
 static char *block_devnode(struct device *dev, umode_t *mode,
 			   kuid_t *uid, kgid_t *gid)
@@ -1121,12 +1123,13 @@ static char *block_devnode(struct device *dev, umode_t *mode,
 	return NULL;
 }
 
-static struct device_type disk_type = {
+struct device_type disk_type = {
 	.name		= "disk",
 	.groups		= disk_attr_groups,
 	.release	= disk_release,
 	.devnode	= block_devnode,
 };
+EXPORT_SYMBOL(disk_type);
 
 #ifdef CONFIG_PROC_FS
 /*
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index ec274e0..a701ace 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -22,6 +22,7 @@
 #define part_to_dev(part)	(&((part)->__dev))
 
 extern struct device_type part_type;
+extern struct device_type disk_type;
 extern struct kobject *block_depr;
 extern struct class block_class;
 
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/5] Btrfs: don't return btrfs_fs_devices if the caller doesn't want it
  2014-09-03 13:36 [PATCH RFC 0/5] Scan all devices to build fs device list Miao Xie
  2014-09-03 13:36 ` [PATCH 1/5] block: export disk_class and disk_type for btrfs Miao Xie
@ 2014-09-03 13:36 ` Miao Xie
  2014-09-03 13:36 ` [PATCH 3/5] Btrfs: restructure btrfs_scan_one_device Miao Xie
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Miao Xie @ 2014-09-03 13:36 UTC (permalink / raw)
  To: linux-btrfs

We will implement the function that the filesystem scan all the devices
in the system and build the device set for btrfs. In this case, we needn't
get btrfs_fs_devices when adding a device into list. This patch changes
device_add_list and implement this feature.

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/volumes.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 1aacf5f..740a4f9 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -568,7 +568,8 @@ static noinline int device_list_add(const char *path,
 	if (!fs_devices->opened)
 		device->generation = found_transid;
 
-	*fs_devices_ret = fs_devices;
+	if (fs_devices_ret)
+		*fs_devices_ret = fs_devices;
 
 	return ret;
 }
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/5] Btrfs: restructure btrfs_scan_one_device
  2014-09-03 13:36 [PATCH RFC 0/5] Scan all devices to build fs device list Miao Xie
  2014-09-03 13:36 ` [PATCH 1/5] block: export disk_class and disk_type for btrfs Miao Xie
  2014-09-03 13:36 ` [PATCH 2/5] Btrfs: don't return btrfs_fs_devices if the caller doesn't want it Miao Xie
@ 2014-09-03 13:36 ` Miao Xie
  2014-09-03 13:36 ` [PATCH 4/5] Btrfs: restructure btrfs_get_bdev_and_sb and pick up some code used later Miao Xie
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Miao Xie @ 2014-09-03 13:36 UTC (permalink / raw)
  To: linux-btrfs

Some code in btrfs_scan_one_device will be re-used by the other function later,
so restructure btrfs_scan_one_device and pick up those code to make a new
function.

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/volumes.c | 57 +++++++++++++++++++++++++++++++-----------------------
 1 file changed, 33 insertions(+), 24 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 740a4f9..bcb19d5 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -885,24 +885,18 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
 	return ret;
 }
 
-/*
- * Look for a btrfs signature on a device. This may be called out of the mount path
- * and we are not allowed to call set_blocksize during the scan. The superblock
- * is read via pagecache
- */
-int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
-			  struct btrfs_fs_devices **fs_devices_ret)
+static int __scan_device(struct block_device *bdev, const char *path,
+			 struct btrfs_fs_devices **fs_devices_ret)
 {
 	struct btrfs_super_block *disk_super;
-	struct block_device *bdev;
 	struct page *page;
 	void *p;
-	int ret = -EINVAL;
 	u64 devid;
 	u64 transid;
 	u64 total_devices;
 	u64 bytenr;
 	pgoff_t index;
+	int ret;
 
 	/*
 	 * we would like to check all the supers, but that would make
@@ -911,38 +905,30 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
 	 * later supers, using BTRFS_SUPER_MIRROR_MAX instead
 	 */
 	bytenr = btrfs_sb_offset(0);
-	flags |= FMODE_EXCL;
-	mutex_lock(&uuid_mutex);
-
-	bdev = blkdev_get_by_path(path, flags, holder);
-
-	if (IS_ERR(bdev)) {
-		ret = PTR_ERR(bdev);
-		goto error;
-	}
 
 	/* make sure our super fits in the device */
 	if (bytenr + PAGE_CACHE_SIZE >= i_size_read(bdev->bd_inode))
-		goto error_bdev_put;
+		return -EINVAL;
 
 	/* make sure our super fits in the page */
 	if (sizeof(*disk_super) > PAGE_CACHE_SIZE)
-		goto error_bdev_put;
+		return -EINVAL;
 
 	/* make sure our super doesn't straddle pages on disk */
 	index = bytenr >> PAGE_CACHE_SHIFT;
 	if ((bytenr + sizeof(*disk_super) - 1) >> PAGE_CACHE_SHIFT != index)
-		goto error_bdev_put;
+		return -EINVAL;
 
 	/* pull in the page with our super */
 	page = read_cache_page_gfp(bdev->bd_inode->i_mapping,
 				   index, GFP_NOFS);
 
 	if (IS_ERR_OR_NULL(page))
-		goto error_bdev_put;
+		return -ENOMEM;
 
-	p = kmap(page);
+	ret = -EINVAL;
 
+	p = kmap(page);
 	/* align our pointer to the offset of the super block */
 	disk_super = p + (bytenr & ~PAGE_CACHE_MASK);
 
@@ -974,7 +960,30 @@ error_unmap:
 	kunmap(page);
 	page_cache_release(page);
 
-error_bdev_put:
+	return ret;
+}
+
+/*
+ * Look for a btrfs signature on a device. This may be called out of the mount path
+ * and we are not allowed to call set_blocksize during the scan. The superblock
+ * is read via pagecache
+ */
+int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
+			  struct btrfs_fs_devices **fs_devices_ret)
+{
+	struct block_device *bdev;
+	int ret;
+
+	flags |= FMODE_EXCL;
+
+	mutex_lock(&uuid_mutex);
+	bdev = blkdev_get_by_path(path, flags, holder);
+	if (IS_ERR(bdev)) {
+		ret = PTR_ERR(bdev);
+		goto error;
+	}
+
+	ret = __scan_device(bdev, path, fs_devices_ret);
 	blkdev_put(bdev, flags);
 error:
 	mutex_unlock(&uuid_mutex);
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/5] Btrfs: restructure btrfs_get_bdev_and_sb and pick up some code used later
  2014-09-03 13:36 [PATCH RFC 0/5] Scan all devices to build fs device list Miao Xie
                   ` (2 preceding siblings ...)
  2014-09-03 13:36 ` [PATCH 3/5] Btrfs: restructure btrfs_scan_one_device Miao Xie
@ 2014-09-03 13:36 ` Miao Xie
  2014-09-03 13:36 ` [PATCH 5/5] Btrfs: scan all the devices and build the fs device list by btrfs's self Miao Xie
  2014-09-06 20:05 ` [PATCH RFC 0/5] Scan all devices to build fs device list Chris Mason
  5 siblings, 0 replies; 16+ messages in thread
From: Miao Xie @ 2014-09-03 13:36 UTC (permalink / raw)
  To: linux-btrfs

Some code in btrfs_get_bdev_and_sb will be re-used by the other function later,
so restructure btrfs_get_bdev_and_sb and pick up those code to make a new
function.

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/volumes.c | 66 +++++++++++++++++++++++++++++-------------------------
 1 file changed, 36 insertions(+), 30 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index bcb19d5..9d52fd8 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -193,42 +193,47 @@ static noinline struct btrfs_fs_devices *find_fsid(u8 *fsid)
 	return NULL;
 }
 
+static int __btrfs_get_sb(struct block_device *bdev, int flush,
+			  struct buffer_head **bh)
+{
+	int ret;
+
+	if (flush)
+		filemap_write_and_wait(bdev->bd_inode->i_mapping);
+
+	ret = set_blocksize(bdev, 4096);
+	if (ret)
+		return ret;
+
+	invalidate_bdev(bdev);
+	*bh = btrfs_read_dev_super(bdev);
+	if (!*bh)
+		return -EINVAL;
+
+	return 0;
+}
+
 static int
-btrfs_get_bdev_and_sb(const char *device_path, fmode_t flags, void *holder,
-		      int flush, struct block_device **bdev,
-		      struct buffer_head **bh)
+btrfs_get_bdev_and_sb_by_path(const char *device_path, fmode_t flags,
+			      void *holder, int flush,
+			      struct block_device **bdev,
+			      struct buffer_head **bh)
 {
 	int ret;
 
 	*bdev = blkdev_get_by_path(device_path, flags, holder);
-
 	if (IS_ERR(*bdev)) {
-		ret = PTR_ERR(*bdev);
 		printk(KERN_INFO "BTRFS: open %s failed\n", device_path);
-		goto error;
+		return PTR_ERR(*bdev);
 	}
 
-	if (flush)
-		filemap_write_and_wait((*bdev)->bd_inode->i_mapping);
-	ret = set_blocksize(*bdev, 4096);
+	ret = __btrfs_get_sb(*bdev, flush, bh);
 	if (ret) {
 		blkdev_put(*bdev, flags);
-		goto error;
-	}
-	invalidate_bdev(*bdev);
-	*bh = btrfs_read_dev_super(*bdev);
-	if (!*bh) {
-		ret = -EINVAL;
-		blkdev_put(*bdev, flags);
-		goto error;
+		return ret;
 	}
 
 	return 0;
-
-error:
-	*bdev = NULL;
-	*bh = NULL;
-	return ret;
 }
 
 static void requeue_list(struct btrfs_pending_bios *pending_bios,
@@ -806,8 +811,8 @@ static int __btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
 			continue;
 
 		/* Just open everything we can; ignore failures here */
-		if (btrfs_get_bdev_and_sb(device->name->str, flags, holder, 1,
-					    &bdev, &bh))
+		if (btrfs_get_bdev_and_sb_by_path(device->name->str, flags,
+						  holder, 1, &bdev, &bh))
 			continue;
 
 		disk_super = (struct btrfs_super_block *)bh->b_data;
@@ -1629,10 +1634,10 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path)
 			goto out;
 		}
 	} else {
-		ret = btrfs_get_bdev_and_sb(device_path,
-					    FMODE_WRITE | FMODE_EXCL,
-					    root->fs_info->bdev_holder, 0,
-					    &bdev, &bh);
+		ret = btrfs_get_bdev_and_sb_by_path(device_path,
+						    FMODE_WRITE | FMODE_EXCL,
+						    root->fs_info->bdev_holder,
+						    0, &bdev, &bh);
 		if (ret)
 			goto out;
 		disk_super = (struct btrfs_super_block *)bh->b_data;
@@ -1906,8 +1911,9 @@ static int btrfs_find_device_by_path(struct btrfs_root *root, char *device_path,
 	struct buffer_head *bh;
 
 	*device = NULL;
-	ret = btrfs_get_bdev_and_sb(device_path, FMODE_READ,
-				    root->fs_info->bdev_holder, 0, &bdev, &bh);
+	ret = btrfs_get_bdev_and_sb_by_path(device_path, FMODE_READ,
+					    root->fs_info->bdev_holder, 0,
+					    &bdev, &bh);
 	if (ret)
 		return ret;
 	disk_super = (struct btrfs_super_block *)bh->b_data;
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/5] Btrfs: scan all the devices and build the fs device list by btrfs's self
  2014-09-03 13:36 [PATCH RFC 0/5] Scan all devices to build fs device list Miao Xie
                   ` (3 preceding siblings ...)
  2014-09-03 13:36 ` [PATCH 4/5] Btrfs: restructure btrfs_get_bdev_and_sb and pick up some code used later Miao Xie
@ 2014-09-03 13:36 ` Miao Xie
  2014-09-06 11:48   ` Goffredo Baroncelli
  2014-09-06 20:05 ` [PATCH RFC 0/5] Scan all devices to build fs device list Chris Mason
  5 siblings, 1 reply; 16+ messages in thread
From: Miao Xie @ 2014-09-03 13:36 UTC (permalink / raw)
  To: linux-btrfs

The original code need scan the devices and build the fs device list by the user
tool by udev or users' selves. It is flexible. But if someone re-install the
filesystem module, and forget to scan the devices by himself, or we plug some
devices with btrfs, but udev thread is blocked and doesn't register the disk
into btrfs in time, the filesystem would report that "can not open some device"
when mounting the filesystem, it was uncomfortable, this patch fixes this problem
by scanning all the devices if we find the number of devices is not right when
we mount the filesystem.

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/super.c   |   3 ++
 fs/btrfs/volumes.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++------
 fs/btrfs/volumes.h |   5 ++-
 3 files changed, 103 insertions(+), 12 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 6b98358..2a8c664 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1264,6 +1264,9 @@ static struct dentry *btrfs_mount(struct file_system_type *fs_type, int flags,
 	if (error)
 		return ERR_PTR(error);
 
+	if (fs_devices->num_devices != fs_devices->total_devices)
+		btrfs_scan_all_devices(fs_type);
+
 	/*
 	 * Setup a dummy root and fs_info for test/set super.  This is because
 	 * we don't actually fill this stuff out until open_ctree, but we need
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 9d52fd8..aa4665e 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -27,6 +27,7 @@
 #include <linux/kthread.h>
 #include <linux/raid/pq.h>
 #include <linux/semaphore.h>
+#include <linux/genhd.h>
 #include <asm/div64.h>
 #include "ctree.h"
 #include "extent_map.h"
@@ -236,6 +237,29 @@ btrfs_get_bdev_and_sb_by_path(const char *device_path, fmode_t flags,
 	return 0;
 }
 
+static int
+btrfs_get_bdev_and_sb_by_dev(dev_t dev, fmode_t flags, void *holder, int flush,
+			     struct block_device **bdev,
+			     struct buffer_head **bh)
+{
+	int ret;
+
+	*bdev = blkdev_get_by_dev(dev, flags, holder);
+	if (IS_ERR(*bdev)) {
+		printk(KERN_INFO "BTRFS: open device %d:%d failed\n",
+		       MAJOR(dev), MINOR(dev));
+		return PTR_ERR(*bdev);
+	}
+
+	ret = __btrfs_get_sb(*bdev, flush, bh);
+	if (ret) {
+		blkdev_put(*bdev, flags);
+		return ret;
+	}
+
+	return 0;
+}
+
 static void requeue_list(struct btrfs_pending_bios *pending_bios,
 			struct bio *head, struct bio *tail)
 {
@@ -466,8 +490,9 @@ static void pending_bios_fn(struct btrfs_work *work)
  * < 0 - error
  */
 static noinline int device_list_add(const char *path,
-			   struct btrfs_super_block *disk_super,
-			   u64 devid, struct btrfs_fs_devices **fs_devices_ret)
+				    struct btrfs_super_block *disk_super,
+				    u64 devid, dev_t devnum,
+				    struct btrfs_fs_devices **fs_devices_ret)
 {
 	struct btrfs_device *device;
 	struct btrfs_fs_devices *fs_devices;
@@ -493,7 +518,7 @@ static noinline int device_list_add(const char *path,
 		if (fs_devices->opened)
 			return -EBUSY;
 
-		device = btrfs_alloc_device(NULL, &devid,
+		device = btrfs_alloc_device(NULL, &devid, devnum,
 					    disk_super->dev_item.uuid);
 		if (IS_ERR(device)) {
 			/* we can safely leave the fs_devices entry around */
@@ -561,6 +586,7 @@ static noinline int device_list_add(const char *path,
 		if (device->missing) {
 			fs_devices->missing_devices--;
 			device->missing = 0;
+			device->devnum = devnum;
 		}
 	}
 
@@ -597,7 +623,7 @@ static struct btrfs_fs_devices *clone_fs_devices(struct btrfs_fs_devices *orig)
 		struct rcu_string *name;
 
 		device = btrfs_alloc_device(NULL, &orig_dev->devid,
-					    orig_dev->uuid);
+					    orig_dev->devnum, orig_dev->uuid);
 		if (IS_ERR(device))
 			goto error;
 
@@ -735,7 +761,7 @@ static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
 			fs_devices->missing_devices--;
 
 		new_device = btrfs_alloc_device(NULL, &device->devid,
-						device->uuid);
+						device->devnum, device->uuid);
 		BUG_ON(IS_ERR(new_device)); /* -ENOMEM */
 
 		/* Safe because we are under uuid_mutex */
@@ -811,7 +837,7 @@ static int __btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
 			continue;
 
 		/* Just open everything we can; ignore failures here */
-		if (btrfs_get_bdev_and_sb_by_path(device->name->str, flags,
+		if (btrfs_get_bdev_and_sb_by_dev(device->devnum, flags,
 						  holder, 1, &bdev, &bh))
 			continue;
 
@@ -945,7 +971,8 @@ static int __scan_device(struct block_device *bdev, const char *path,
 	transid = btrfs_super_generation(disk_super);
 	total_devices = btrfs_super_num_devices(disk_super);
 
-	ret = device_list_add(path, disk_super, devid, fs_devices_ret);
+	ret = device_list_add(path, disk_super, devid, bdev->bd_dev,
+			      fs_devices_ret);
 	if (ret > 0) {
 		if (disk_super->label[0]) {
 			if (disk_super->label[BTRFS_LABEL_SIZE - 1])
@@ -995,6 +1022,63 @@ error:
 	return ret;
 }
 
+static void btrfs_scan_partitions_on_disk(struct gendisk *disk, void *holder)
+{
+	struct disk_part_iter piter;
+	struct hd_struct *part;
+	struct block_device *bdev;
+	char buf[BDEVNAME_SIZE];
+	fmode_t mode = FMODE_READ | FMODE_EXCL;
+	int count = 0;
+	int ret;
+
+	disk_part_iter_init(&piter, disk, DISK_PITER_INCL_PART0 |
+					  DISK_PITER_REVERSE);
+	while ((part = disk_part_iter_next(&piter))) {
+		if (count && !part->partno)
+			continue;
+
+		count++;
+		bdev = bdget(part_devt(part));
+		if (!bdev)
+			continue;
+
+		if (blkdev_get(bdev, mode, holder))
+			continue;
+
+		ret = __scan_device(bdev, bdevname(bdev, buf), NULL);
+		if (!ret)
+			btrfs_kobject_uevent(bdev, KOBJ_CHANGE);
+		blkdev_put(bdev, mode);
+	}
+	disk_part_iter_exit(&piter);
+}
+
+void btrfs_scan_all_devices(void *holder)
+{
+	struct class_dev_iter iter;
+	struct device *dev;
+	struct gendisk *disk;
+
+	mutex_lock(&uuid_mutex);
+	class_dev_iter_init(&iter, &block_class, NULL, &disk_type);
+	while ((dev = class_dev_iter_next(&iter))) {
+		disk = dev_to_disk(dev);
+
+		if (!get_capacity(disk) ||
+		    (!disk_max_parts(disk) &&
+		     (disk->flags & GENHD_FL_REMOVABLE)))
+			continue;
+
+		if (disk->flags & GENHD_FL_SUPPRESS_PARTITION_INFO)
+			continue;
+
+		btrfs_scan_partitions_on_disk(disk, holder);
+	}
+	class_dev_iter_exit(&iter);
+	mutex_unlock(&uuid_mutex);
+}
+
 /* helper to account the used device space in the range */
 int btrfs_account_dev_extents_size(struct btrfs_device *device, u64 start,
 				   u64 end, u64 *length)
@@ -2140,7 +2224,7 @@ int btrfs_init_new_device(struct btrfs_root *root, char *device_path)
 	}
 	mutex_unlock(&root->fs_info->fs_devices->device_list_mutex);
 
-	device = btrfs_alloc_device(root->fs_info, NULL, NULL);
+	device = btrfs_alloc_device(root->fs_info, NULL, bdev->bd_dev, NULL);
 	if (IS_ERR(device)) {
 		/* we can safely leave the fs_devices entry around */
 		ret = PTR_ERR(device);
@@ -2352,7 +2436,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_root *root, char *device_path,
 	}
 
 
-	device = btrfs_alloc_device(NULL, &devid, NULL);
+	device = btrfs_alloc_device(NULL, &devid, bdev->bd_dev, NULL);
 	if (IS_ERR(device)) {
 		ret = PTR_ERR(device);
 		goto error;
@@ -5867,7 +5951,7 @@ static struct btrfs_device *add_missing_dev(struct btrfs_root *root,
 {
 	struct btrfs_device *device;
 
-	device = btrfs_alloc_device(NULL, &devid, dev_uuid);
+	device = btrfs_alloc_device(NULL, &devid, 0, dev_uuid);
 	if (IS_ERR(device))
 		return NULL;
 
@@ -5895,7 +5979,7 @@ static struct btrfs_device *add_missing_dev(struct btrfs_root *root,
  * destroyed with kfree() right away.
  */
 struct btrfs_device *btrfs_alloc_device(struct btrfs_fs_info *fs_info,
-					const u64 *devid,
+					const u64 *devid, dev_t devnum,
 					const u8 *uuid)
 {
 	struct btrfs_device *dev;
@@ -5920,6 +6004,7 @@ struct btrfs_device *btrfs_alloc_device(struct btrfs_fs_info *fs_info,
 		}
 	}
 	dev->devid = tmp;
+	dev->devnum = devnum;
 
 	if (uuid)
 		memcpy(dev->uuid, uuid, BTRFS_UUID_SIZE);
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 2b37da3..cfb6539 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -69,6 +69,7 @@ struct btrfs_device {
 
 	/* the mode sent to blkdev_get */
 	fmode_t mode;
+	dev_t devnum;
 
 	int writeable;
 	int in_fs_metadata;
@@ -288,6 +289,8 @@ struct btrfs_bio_stripe {
 	u64 length; /* only used for discard mappings */
 };
 
+void btrfs_scan_all_devices(void *holder);
+
 struct btrfs_bio;
 typedef void (btrfs_bio_end_io_t) (struct btrfs_bio *bio, int err);
 
@@ -414,7 +417,7 @@ int btrfs_find_device_missing_or_by_path(struct btrfs_root *root,
 					 char *device_path,
 					 struct btrfs_device **device);
 struct btrfs_device *btrfs_alloc_device(struct btrfs_fs_info *fs_info,
-					const u64 *devid,
+					const u64 *devid, dev_t devnum,
 					const u8 *uuid);
 int btrfs_rm_device(struct btrfs_root *root, char *device_path);
 void btrfs_cleanup_fs_uuids(void);
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/5] Btrfs: scan all the devices and build the fs device list by btrfs's self
  2014-09-03 13:36 ` [PATCH 5/5] Btrfs: scan all the devices and build the fs device list by btrfs's self Miao Xie
@ 2014-09-06 11:48   ` Goffredo Baroncelli
  2014-09-09  4:06     ` Miao Xie
  0 siblings, 1 reply; 16+ messages in thread
From: Goffredo Baroncelli @ 2014-09-06 11:48 UTC (permalink / raw)
  To: Miao Xie, linux-btrfs

On 09/03/2014 03:36 PM, Miao Xie wrote:
> The original code need scan the devices and build the fs device list by the user
> tool by udev or users' selves. It is flexible. But if someone re-install the
> filesystem module, and forget to scan the devices by himself, or we plug some
> devices with btrfs, but udev thread is blocked and doesn't register the disk
> into btrfs in time, the filesystem would report that "can not open some device"
> when mounting the filesystem, it was uncomfortable, this patch fixes this problem
> by scanning all the devices if we find the number of devices is not right when
> we mount the filesystem.
> 
> Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
[....]
> +
> +void btrfs_scan_all_devices(void *holder)
> +{
> +	struct class_dev_iter iter;
> +	struct device *dev;
> +	struct gendisk *disk;
> +
> +	mutex_lock(&uuid_mutex);
> +	class_dev_iter_init(&iter, &block_class, NULL, &disk_type);
> +	while ((dev = class_dev_iter_next(&iter))) {
> +		disk = dev_to_disk(dev);
> +
> +		if (!get_capacity(disk) ||
> +		    (!disk_max_parts(disk) &&
> +		     (disk->flags & GENHD_FL_REMOVABLE)))
                                    ^^^^^^^^^^^^^^^^^^
> +			continue;
> +
> +		if (disk->flags & GENHD_FL_SUPPRESS_PARTITION_INFO)
> +			continue;


Hi, could you elaborate why a removable disk should be not scan-ned ? How
a removble usb disk is classified ?

Thanks.
G.Baroncelli

[...]


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RFC 0/5] Scan all devices to build fs device list
  2014-09-03 13:36 [PATCH RFC 0/5] Scan all devices to build fs device list Miao Xie
                   ` (4 preceding siblings ...)
  2014-09-03 13:36 ` [PATCH 5/5] Btrfs: scan all the devices and build the fs device list by btrfs's self Miao Xie
@ 2014-09-06 20:05 ` Chris Mason
  2014-09-08  9:09   ` David Sterba
  5 siblings, 1 reply; 16+ messages in thread
From: Chris Mason @ 2014-09-06 20:05 UTC (permalink / raw)
  To: Miao Xie, linux-btrfs

On 09/03/2014 09:36 AM, Miao Xie wrote:
> This patchset implements device list automatic building function. As we
> know, currently we need scan the devices to build device list by a user tool
> before mounting the filesystem, especially mount the filesystem after
> we re-install btrfs module. It is not convenient. This patchset can improve
> that problem. With this patchset, we will scan all the devices in the
> system to build the device list if we find the number of the devices
> is not right when we mount the filesystem. By this way, we needn't scan
> the device by the user tool and reduce the mount failure probability due
> to the incomplete device list.

Thanks for working on these patches, but I really prefer that we do
these scans from userspace.  I know it introduces a few problems, but it
also solves a lot of complexity at the same time.  And udev already has
most of the info we need, so I think our time is best spent fixing any
problems with the udev integration.

-chris

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RFC 0/5] Scan all devices to build fs device list
  2014-09-06 20:05 ` [PATCH RFC 0/5] Scan all devices to build fs device list Chris Mason
@ 2014-09-08  9:09   ` David Sterba
  2014-09-08 11:04     ` Anand Jain
  2014-09-08 16:59     ` Goffredo Baroncelli
  0 siblings, 2 replies; 16+ messages in thread
From: David Sterba @ 2014-09-08  9:09 UTC (permalink / raw)
  To: Chris Mason; +Cc: Miao Xie, linux-btrfs

On Sat, Sep 06, 2014 at 04:05:20PM -0400, Chris Mason wrote:
> On 09/03/2014 09:36 AM, Miao Xie wrote:
> > This patchset implements device list automatic building function. As we
> > know, currently we need scan the devices to build device list by a user tool
> > before mounting the filesystem, especially mount the filesystem after
> > we re-install btrfs module. It is not convenient. This patchset can improve
> > that problem. With this patchset, we will scan all the devices in the
> > system to build the device list if we find the number of the devices
> > is not right when we mount the filesystem. By this way, we needn't scan
> > the device by the user tool and reduce the mount failure probability due
> > to the incomplete device list.
> 
> Thanks for working on these patches, but I really prefer that we do
> these scans from userspace.

I agree. The userspace approach gives more control of when and how the
scanning is done. Yes the scan has to be done after the module is
inserted, but distros have that in initrd. Reinstalling module is not
something a normal user does and developers know how to workaround.

Automatic scanning in usperspace can be done via the mount helper and
this is IMO the way to go. There's a patch for that from Goffredo, not
merged yet, the patch backlog is still too long.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RFC 0/5] Scan all devices to build fs device list
  2014-09-08  9:09   ` David Sterba
@ 2014-09-08 11:04     ` Anand Jain
  2014-09-08 17:15       ` Goffredo Baroncelli
  2014-09-08 16:59     ` Goffredo Baroncelli
  1 sibling, 1 reply; 16+ messages in thread
From: Anand Jain @ 2014-09-08 11:04 UTC (permalink / raw)
  To: dsterba, Chris Mason, Miao Xie, linux-btrfs



On 08/09/2014 17:09, David Sterba wrote:
> On Sat, Sep 06, 2014 at 04:05:20PM -0400, Chris Mason wrote:
>> On 09/03/2014 09:36 AM, Miao Xie wrote:
>>> This patchset implements device list automatic building function. As we
>>> know, currently we need scan the devices to build device list by a user tool
>>> before mounting the filesystem, especially mount the filesystem after
>>> we re-install btrfs module. It is not convenient. This patchset can improve
>>> that problem. With this patchset, we will scan all the devices in the
>>> system to build the device list if we find the number of the devices
>>> is not right when we mount the filesystem. By this way, we needn't scan
>>> the device by the user tool and reduce the mount failure probability due
>>> to the incomplete device list.
>>
>> Thanks for working on these patches, but I really prefer that we do
>> these scans from userspace.
>
> I agree. The userspace approach gives more control of when and how the
> scanning is done. Yes the scan has to be done after the module is
> inserted, but distros have that in initrd. Reinstalling module is not
> something a normal user does and developers know how to workaround.
>
> Automatic scanning in usperspace can be done via the mount helper and
> this is IMO the way to go. There's a patch for that from Goffredo, not
> merged yet, the patch backlog is still too long.


  Since we are on the topic of scanning. A point on improving the
  btrfs-progs scan method which I am working on...

    - Scan of all system devices is an expensive task. But btrfs-progs
      do it very liberally. Due to this there are some serious problem
      like - btrfs fi show is too slow when scrub is running.

    - The worst is Single btrfs-progs thread scans for btrfs
      devices multiple times. Mainly because most of the functions
      uses check_mounted() which in turn calls scan, think of
      multi device btrfs config.

    - The problem would be more prominent in larger server with 1000's
      of LUNs / devices.

    - lblkid can be the only method to scan for devices. (The other
      method we have as of now is of canning /proc/partitions, which
      lblkid also does).

   what I am planning -
     Use btrfs control ioctl to help btrfs-progs check_mounted() to know
       if a (multi) device is mounted.
     Build kernel's fs_devices list in the user-space using btrfs
       control ioctl.
     Do device scan only once per btrfs-progs thread.



Any comments ?

Thanks, Anand



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RFC 0/5] Scan all devices to build fs device list
  2014-09-08  9:09   ` David Sterba
  2014-09-08 11:04     ` Anand Jain
@ 2014-09-08 16:59     ` Goffredo Baroncelli
  1 sibling, 0 replies; 16+ messages in thread
From: Goffredo Baroncelli @ 2014-09-08 16:59 UTC (permalink / raw)
  To: dsterba; +Cc: Chris Mason, Miao Xie, linux-btrfs

Hi David,

On 09/08/2014 11:09 AM, David Sterba wrote:
> Automatic scanning in usperspace can be done via the mount helper and
> this is IMO the way to go. There's a patch for that from Goffredo, not
> merged yet, the patch backlog is still too long.

There is something that I can do to help you regarding this patch ?


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RFC 0/5] Scan all devices to build fs device list
  2014-09-08 11:04     ` Anand Jain
@ 2014-09-08 17:15       ` Goffredo Baroncelli
  2014-09-09  3:26         ` Anand Jain
  0 siblings, 1 reply; 16+ messages in thread
From: Goffredo Baroncelli @ 2014-09-08 17:15 UTC (permalink / raw)
  To: Anand Jain; +Cc: dsterba, Chris Mason, Miao Xie, linux-btrfs

On 09/08/2014 01:04 PM, Anand Jain wrote:
> 
> 
> On 08/09/2014 17:09, David Sterba wrote:
>> On Sat, Sep 06, 2014 at 04:05:20PM -0400, Chris Mason wrote:
>>> On 09/03/2014 09:36 AM, Miao Xie wrote:
>>>> This patchset implements device list automatic building function. As we
>>>> know, currently we need scan the devices to build device list by a user tool
>>>> before mounting the filesystem, especially mount the filesystem after
>>>> we re-install btrfs module. It is not convenient. This patchset can improve
>>>> that problem. With this patchset, we will scan all the devices in the
>>>> system to build the device list if we find the number of the devices
>>>> is not right when we mount the filesystem. By this way, we needn't scan
>>>> the device by the user tool and reduce the mount failure probability due
>>>> to the incomplete device list.
>>>
>>> Thanks for working on these patches, but I really prefer that we do
>>> these scans from userspace.
>>
>> I agree. The userspace approach gives more control of when and how the
>> scanning is done. Yes the scan has to be done after the module is
>> inserted, but distros have that in initrd. Reinstalling module is not
>> something a normal user does and developers know how to workaround.
>>
>> Automatic scanning in usperspace can be done via the mount helper and
>> this is IMO the way to go. There's a patch for that from Goffredo, not
>> merged yet, the patch backlog is still too long.
> 
> 
>  Since we are on the topic of scanning. A point on improving the
>  btrfs-progs scan method which I am working on...
> 
>    - Scan of all system devices is an expensive task. But btrfs-progs
>      do it very liberally. Due to this there are some serious problem
>      like - btrfs fi show is too slow when scrub is running.
> 
>    - The worst is Single btrfs-progs thread scans for btrfs
>      devices multiple times. Mainly because most of the functions
>      uses check_mounted() which in turn calls scan, think of
>      multi device btrfs config.

Using libblkid (see your proposal below) would help in this case, 
because the values would be cached.
> 
>    - The problem would be more prominent in larger server with 1000's
>      of LUNs / devices.
> 
>    - lblkid can be the only method to scan for devices. (The other
>      method we have as of now is of canning /proc/partitions, which
>      lblkid also does).
> 
>   what I am planning -
>     Use btrfs control ioctl to help btrfs-progs check_mounted() to know
>       if a (multi) device is mounted.
>     Build kernel's fs_devices list in the user-space using btrfs
>       control ioctl.
>     Do device scan only once per btrfs-progs thread.

You are talking about a ioctl: why not use (extending them when needed) 
the sysfs information ?

> 
> Any comments ?


> 
> Thanks, Anand
> 
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RFC 0/5] Scan all devices to build fs device list
  2014-09-08 17:15       ` Goffredo Baroncelli
@ 2014-09-09  3:26         ` Anand Jain
  2014-09-09 20:31           ` Goffredo Baroncelli
  0 siblings, 1 reply; 16+ messages in thread
From: Anand Jain @ 2014-09-09  3:26 UTC (permalink / raw)
  To: kreijack, dsterba, Chris Mason; +Cc: Miao Xie, linux-btrfs



>>   Since we are on the topic of scanning. A point on improving the
>>   btrfs-progs scan method which I am working on...
>>
>>     - Scan of all system devices is an expensive task. But btrfs-progs
>>       do it very liberally. Due to this there are some serious problem
>>       like - btrfs fi show is too slow when scrub is running.
>>
>>     - The worst is Single btrfs-progs thread scans for btrfs
>>       devices multiple times. Mainly because most of the functions
>>       uses check_mounted() which in turn calls scan, think of
>>       multi device btrfs config.
>
> Using libblkid (see your proposal below) would help in this case,
> because the values would be cached.

   may be, let me try / report. but first I need to search for a system
   with at least 100+  LUNs.


>>     - The problem would be more prominent in larger server with 1000's
>>       of LUNs / devices.
>>
>>     - lblkid can be the only method to scan for devices. (The other
>>       method we have as of now is of canning /proc/partitions, which
>>       lblkid also does).
>>
>>    what I am planning -
>>      Use btrfs control ioctl to help btrfs-progs check_mounted() to know
>>        if a (multi) device is mounted.
>>      Build kernel's fs_devices list in the user-space using btrfs
>>        control ioctl.
>>      Do device scan only once per btrfs-progs thread.
>
> You are talking about a ioctl: why not use (extending them when needed)
> the sysfs information ?

  yeah, I understand I am the only person talking about ioctl,
  and all others recommend sysfs. I understand I am wrong, but
  just by mass. and I still have no idea why not ioctl ?
  Problem with sysfs:
   - Note that sysfs supports only one parameter value with max length
   u64, to rebuilt entire kernel's fs_uuid list in the user space
   that would be quite a lot of sysfs files. should that be fine ?
   Further we would need another traverser tool (python ?) to read
   all these sysfs files. ? so that we could create fs_uuid list
   in the user-space.

   - we would need all info about the kernel fs_uuid, even when the
  device is not mounted.

   - thinking of nested seed with sysfs is more complicated, as we would
  have same btrfs_device at multiple fs_devices. still we must represent
  them in the sysfs.

   - as of now fs_uuid can grow infinite with just "a" device.
    ref: [PATCH 1/2] btrfs: device list could grow infinite
    can't imagine anybody traversing through /sysfs to understand
    what btrfs-kernel think of devices. user would prefer a cli
    to know that instead.

   - sysfs layout has to be dynamic based on fs_uuids list changes,
   devices added, removed, mounted replaced etc.. isn't it making
   more unstable ?


appreciate your comments

Anand

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/5] Btrfs: scan all the devices and build the fs device list by btrfs's self
  2014-09-06 11:48   ` Goffredo Baroncelli
@ 2014-09-09  4:06     ` Miao Xie
  0 siblings, 0 replies; 16+ messages in thread
From: Miao Xie @ 2014-09-09  4:06 UTC (permalink / raw)
  To: kreijack, linux-btrfs

On Sat, 6 Sep 2014 13:48:09 +0200, Goffredo Baroncelli wrote:
> On 09/03/2014 03:36 PM, Miao Xie wrote:
>> The original code need scan the devices and build the fs device list by the user
>> tool by udev or users' selves. It is flexible. But if someone re-install the
>> filesystem module, and forget to scan the devices by himself, or we plug some
>> devices with btrfs, but udev thread is blocked and doesn't register the disk
>> into btrfs in time, the filesystem would report that "can not open some device"
>> when mounting the filesystem, it was uncomfortable, this patch fixes this problem
>> by scanning all the devices if we find the number of devices is not right when
>> we mount the filesystem.
>>
>> Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
> [....]
>> +
>> +void btrfs_scan_all_devices(void *holder)
>> +{
>> +	struct class_dev_iter iter;
>> +	struct device *dev;
>> +	struct gendisk *disk;
>> +
>> +	mutex_lock(&uuid_mutex);
>> +	class_dev_iter_init(&iter, &block_class, NULL, &disk_type);
>> +	while ((dev = class_dev_iter_next(&iter))) {
>> +		disk = dev_to_disk(dev);
>> +
>> +		if (!get_capacity(disk) ||
>> +		    (!disk_max_parts(disk) &&
>> +		     (disk->flags & GENHD_FL_REMOVABLE)))
>                                     ^^^^^^^^^^^^^^^^^^
>> +			continue;
>> +
>> +		if (disk->flags & GENHD_FL_SUPPRESS_PARTITION_INFO)
>> +			continue;
> 
> 
> Hi, could you elaborate why a removable disk should be not scan-ned ? How
> a removble usb disk is classified ?

This is used to filter the non-partitionable removeable device such as cdrom,
if it is a usb disk, it should be partitionable.

Thanks
Miao

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RFC 0/5] Scan all devices to build fs device list
  2014-09-09  3:26         ` Anand Jain
@ 2014-09-09 20:31           ` Goffredo Baroncelli
  2014-09-10  6:06             ` Anand Jain
  0 siblings, 1 reply; 16+ messages in thread
From: Goffredo Baroncelli @ 2014-09-09 20:31 UTC (permalink / raw)
  To: Anand Jain, dsterba, Chris Mason; +Cc: Miao Xie, linux-btrfs

On 09/09/2014 05:26 AM, Anand Jain wrote:

Anand,
first thanks to discuss this with me; below my comments

>> You are talking about a ioctl: why not use (extending them when needed)
>> the sysfs information ?
> 
>  yeah, I understand I am the only person talking about ioctl,
>  and all others recommend sysfs. I understand I am wrong, but
>  just by mass. and I still have no idea why not ioctl ?

Because it is an interface difficult to extend.

>  Problem with sysfs:
>   - Note that sysfs supports only one parameter value with max length
>   u64, to rebuilt entire kernel's fs_uuid list in the user space
>   that would be quite a lot of sysfs files. should that be fine ?
>   Further we would need another traverser tool (python ?) to read
>   all these sysfs files. ? so that we could create fs_uuid list
>   in the user-space.
To me it seems that other fs interface scale well even if the number 
of items is big (think /proc)
 
>   - we would need all info about the kernel fs_uuid, even when the
>  device is not mounted.
Could you elaborate ? 
We have two kind of objects: filesystems and devices. A filesystem is
created when its first device is registered. When the filesystem
is mounted, further information are available.

Our hypothetical sysfs interface should export information about
filesystem and/or devices until they exist. If a filesystem is
mounted more information are exported. When it is umounted
less are exported.

This would not be different if we use a ioctl() interface.

> 
>   - thinking of nested seed with sysfs is more complicated, as we would
>  have same btrfs_device at multiple fs_devices. still we must represent
>  them in the sysfs.
I am not used to use seed device; so I can't comment.
 
>   - as of now fs_uuid can grow infinite with just "a" device.
>    ref: [PATCH 1/2] btrfs: device list could grow infinite
This is a btrfs problem. Doesn't make sense to store these information
if there aren't any device. When the last device disappear (is replaced),
the filesystem object should disappear too.
If a sysfs approach point out this problem... to me it seems good ! :-)

>    can't imagine anybody traversing through /sysfs to understand
>    what btrfs-kernel think of devices. user would prefer a cli
>    to know that instead.
sysfs is an interface, that doesn't means that is THE user interface.
Of course a CLI or a GUI will need.

> 
>   - sysfs layout has to be dynamic based on fs_uuids list changes,
>   devices added, removed, mounted replaced etc.. isn't it making
>   more unstable ?

$ find /sys | wc -l
27362
On my PC sysfs has more already than 20 thousands entries, and it is 
a quite simple machine. But this doesn't cause instability.
May be more difficult, but I don't think that sysfs is not capable 
to sustain that.


Already linux export a lot of sysfs files for each device. I.e.

$ find /sys/block/sda/ | wc -l
	190

these are hundreds entries for each disks. So I don't think that btrfs sysfs
interface could cause more problem even if a filesystem has
a lot of disks.

> 
> 
> appreciate your comments
The major problem of an ioctl() interface is that is very difficult 
to extend. Typically when we need to extend it , the current is 
marked as old/deprecated and a new one is generated.
However an ioctl() is an ABI so even the old interface have to be 
maintained.

One point about I have to agree with you is that a sysfs interface
is not good, is when:
- we need to export huge quantity of data
- we have an high rate of data

But these to me don't seem to apply to a sysfs btrfs interface.

> 
> Anand
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RFC 0/5] Scan all devices to build fs device list
  2014-09-09 20:31           ` Goffredo Baroncelli
@ 2014-09-10  6:06             ` Anand Jain
  0 siblings, 0 replies; 16+ messages in thread
From: Anand Jain @ 2014-09-10  6:06 UTC (permalink / raw)
  To: kreijack, dsterba, Chris Mason; +Cc: Miao Xie, linux-btrfs



Goffredo,


Thanks for comments, more in line below.

On 10/09/2014 04:31, Goffredo Baroncelli wrote:
> On 09/09/2014 05:26 AM, Anand Jain wrote:
>
> Anand,
> first thanks to discuss this with me; below my comments
>
>>> You are talking about a ioctl: why not use (extending them when needed)
>>> the sysfs information ?
>>
>>   yeah, I understand I am the only person talking about ioctl,
>>   and all others recommend sysfs. I understand I am wrong, but
>>   just by mass. and I still have no idea why not ioctl ?
>
> Because it is an interface difficult to extend.

  Its quite common to make ioctl auto adaptable by using the
  structure size/version.  btrfs-devlist tool does it too.


>>   Problem with sysfs:
>>    - Note that sysfs supports only one parameter value with max length
>>    u64, to rebuilt entire kernel's fs_uuid list in the user space
>>    that would be quite a lot of sysfs files. should that be fine ?
>>    Further we would need another traverser tool (python ?) to read
>>    all these sysfs files. ? so that we could create fs_uuid list
>>    in the user-space.
> To me it seems that other fs interface scale well even if the number
> of items is big (think /proc)

  /proc supports larger content transfer using seq_file.  /sysfs doesn't.

>>    - we would need all info about the kernel fs_uuid, even when the
>>   device is not mounted.

> Could you elaborate ?
> We have two kind of objects: filesystems and devices. A filesystem is
> created when its first device is registered. When the filesystem
> is mounted, further information are available.
>
> Our hypothetical sysfs interface should export information about
> filesystem and/or devices until they exist. If a filesystem is
> mounted more information are exported. When it is umounted
> less are exported.
>
> This would not be different if we use a ioctl() interface.

  (Its not an on disk structure)
  The purpose of this interface is to read entire of volume.c
  static LIST_HEAD(fs_uuids) list from the btrfs memory to the
  user space. Sorry if it wasn't clear before.

  We desperately need this to solve some critical structural
  problem in btrs-progs.

>>    - thinking of nested seed with sysfs is more complicated, as we would
>>   have same btrfs_device at multiple fs_devices. still we must represent
>>   them in the sysfs.
> I am not used to use seed device; so I can't comment.
>
>>    - as of now fs_uuid can grow infinite with just "a" device.
>>     ref: [PATCH 1/2] btrfs: device list could grow infinite
> This is a btrfs problem. Doesn't make sense to store these information
> if there aren't any device. When the last device disappear (is replaced),
> the filesystem object should disappear too.
> If a sysfs approach point out this problem... to me it seems good ! :-)


>>     can't imagine anybody traversing through /sysfs to understand
>>     what btrfs-kernel think of devices. user would prefer a cli
>>     to know that instead.
> sysfs is an interface, that doesn't means that is THE user interface.
> Of course a CLI or a GUI will need.


>>
>>    - sysfs layout has to be dynamic based on fs_uuids list changes,
>>    devices added, removed, mounted replaced etc.. isn't it making
>>    more unstable ?
>
> $ find /sys | wc -l
> 27362
> On my PC sysfs has more already than 20 thousands entries, and it is
> a quite simple machine. But this doesn't cause instability.
> May be more difficult, but I don't think that sysfs is not capable
> to sustain that.
>
>
> Already linux export a lot of sysfs files for each device. I.e.
>
> $ find /sys/block/sda/ | wc -l
> 	190
>
> these are hundreds entries for each disks. So I don't think that btrfs sysfs
> interface could cause more problem even if a filesystem has
> a lot of disks.


>>
>> appreciate your comments
> The major problem of an ioctl() interface is that is very difficult
> to extend. Typically when we need to extend it , the current is
> marked as old/deprecated and a new one is generated.
> However an ioctl() is an ABI so even the old interface have to be
> maintained.
>
> One point about I have to agree with you is that a sysfs interface
> is not good, is when:
> - we need to export huge quantity of data
> - we have an high rate of data
>
> But these to me don't seem to apply to a sysfs btrfs interface.


  I thought you will say sysfs doesn't have to depend on any
  custom made user space tool to read/write. :-)

  The challenge is to provide sysfs interface without cluttering
  it.  There are a lot talks on sysfs being cluttered.
  We definitely need sysfs interface for btrfs. But I am not
  too sure if we can get all the info without looking btrfs sysfs
  interface too cluttered, typically we might need all info as
  in btrfs-devlist.

Thanks, Anand


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2014-09-10  6:07 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-03 13:36 [PATCH RFC 0/5] Scan all devices to build fs device list Miao Xie
2014-09-03 13:36 ` [PATCH 1/5] block: export disk_class and disk_type for btrfs Miao Xie
2014-09-03 13:36 ` [PATCH 2/5] Btrfs: don't return btrfs_fs_devices if the caller doesn't want it Miao Xie
2014-09-03 13:36 ` [PATCH 3/5] Btrfs: restructure btrfs_scan_one_device Miao Xie
2014-09-03 13:36 ` [PATCH 4/5] Btrfs: restructure btrfs_get_bdev_and_sb and pick up some code used later Miao Xie
2014-09-03 13:36 ` [PATCH 5/5] Btrfs: scan all the devices and build the fs device list by btrfs's self Miao Xie
2014-09-06 11:48   ` Goffredo Baroncelli
2014-09-09  4:06     ` Miao Xie
2014-09-06 20:05 ` [PATCH RFC 0/5] Scan all devices to build fs device list Chris Mason
2014-09-08  9:09   ` David Sterba
2014-09-08 11:04     ` Anand Jain
2014-09-08 17:15       ` Goffredo Baroncelli
2014-09-09  3:26         ` Anand Jain
2014-09-09 20:31           ` Goffredo Baroncelli
2014-09-10  6:06             ` Anand Jain
2014-09-08 16:59     ` Goffredo Baroncelli

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.