[RFC v2 0/2] vfs / btrfs: add support for ustat()

[RFC v2 0/2] vfs / btrfs: add support for ustat()

[Luis R. Rodriguez]

From: "Luis R. Rodriguez" <mcgrof@suse.com>

This makes the implementation simpler by stuffing the struct on
the driver and just letting the driver iinsert it and remove it
onto the sb list. This avoids the kzalloc() completely.

Luis R. Rodriguez (2):
  fs/super.c: add new super block sub devices super_block_dev
  btrfs: use the new VFS super_block_dev

 fs/btrfs/ctree.h   |  7 ++----
 fs/btrfs/disk-io.c |  6 ++---
 fs/btrfs/inode.c   |  2 +-
 fs/super.c         | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/fs.h | 10 ++++++++
 5 files changed, 81 insertions(+), 12 deletions(-)

-- 
2.0.1

[RFC v2 1/2] fs/super.c: add new super block sub devices super_block_dev

[Luis R. Rodriguez]

From: "Luis R. Rodriguez" <mcgrof@suse.com>

Modern filesystems are using the get_anon_bdev() for internal
notions of volumes, snapshots for a single super block but never
exposing them directly to the VFS layer. While this works its
leaves the VFS layer growing dumb over what filesystems are doing.
This creates a new super block subdevice which we can use to start
stuffing in information about the underlying bdev's and its
associated super block to start off with. This at least now lets
us implement proper support for ustat() once filesystems are
modified to use this data structure and respective helpers.

Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 fs/super.c         | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/fs.h | 10 ++++++++
 2 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/fs/super.c b/fs/super.c
index d20d5b1..d871892 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -133,6 +133,68 @@ static unsigned long super_cache_count(struct shrinker *shrink,
 	return total_objects;
 }
 
+static bool super_dev_match(struct super_block *sb, dev_t dev)
+{
+	struct super_block_dev *sbdev;
+
+	if (sb->s_dev == dev)
+		return true;
+
+	if (list_empty(&sb->s_sbdevs))
+		return false;
+
+	list_for_each_entry(sbdev, &sb->s_sbdevs, entry)
+		if (sbdev->anon_dev ==  dev)
+			return true;
+
+	return false;
+}
+
+int insert_anon_sbdev(struct super_block *sb, struct super_block_dev *sbdev)
+{
+	int ret;
+
+	ret = get_anon_bdev(&sbdev->anon_dev);
+	if (ret)
+		return ret;
+
+	sbdev->sb = sb;
+
+	spin_lock(&sb_lock);
+	list_add_tail(&sbdev->entry, &sb->s_sbdevs);
+	spin_unlock(&sb_lock);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(insert_anon_sbdev);
+
+void remove_anon_sbdev(struct super_block_dev *sbdev)
+{
+	struct super_block *sb;
+	struct super_block_dev *sbdev_i, *tmp;
+
+	if (!sbdev)
+		return;
+
+	sb = sbdev->sb;
+
+	spin_lock(&sb_lock);
+
+	WARN_ON(list_empty(&sb->s_sbdevs));
+
+	list_for_each_entry_safe(sbdev_i, tmp, &sb->s_sbdevs, entry) {
+		if (sbdev == sbdev_i) {
+			list_del_init(&sbdev_i->entry);
+			break;
+		}
+	}
+
+	spin_unlock(&sb_lock);
+
+	free_anon_bdev(sbdev->anon_dev);
+}
+EXPORT_SYMBOL_GPL(remove_anon_sbdev);
+
 /**
  *	destroy_super	-	frees a superblock
  *	@s: superblock to free
@@ -148,6 +210,7 @@ static void destroy_super(struct super_block *s)
 		percpu_counter_destroy(&s->s_writers.counter[i]);
 	security_sb_free(s);
 	WARN_ON(!list_empty(&s->s_mounts));
+	WARN_ON(!list_empty(&s->s_sbdevs));
 	kfree(s->s_subtype);
 	kfree(s->s_options);
 	kfree_rcu(s, rcu);
@@ -188,6 +251,7 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags)
 	INIT_HLIST_NODE(&s->s_instances);
 	INIT_HLIST_BL_HEAD(&s->s_anon);
 	INIT_LIST_HEAD(&s->s_inodes);
+	INIT_LIST_HEAD(&s->s_sbdevs);
 
 	if (list_lru_init(&s->s_dentry_lru))
 		goto fail;
@@ -652,7 +716,7 @@ restart:
 	spin_unlock(&sb_lock);
 	return NULL;
 }
- 
+
 struct super_block *user_get_super(dev_t dev)
 {
 	struct super_block *sb;
@@ -662,7 +726,7 @@ rescan:
 	list_for_each_entry(sb, &super_blocks, s_list) {
 		if (hlist_unhashed(&sb->s_instances))
 			continue;
-		if (sb->s_dev ==  dev) {
+		if (super_dev_match(sb, dev)) {
 			sb->s_count++;
 			spin_unlock(&sb_lock);
 			down_read(&sb->s_umount);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index e11d60c..f97b559 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1172,6 +1172,13 @@ struct sb_writers {
 #endif
 };
 
+/* we can expand this to help the VFS layer with modern filesystems */
+struct super_block_dev {
+	struct super_block	*sb;
+	struct list_head	entry;		/* For struct sb->s_sbdevs */
+	dev_t			anon_dev;
+};
+
 struct super_block {
 	struct list_head	s_list;		/* Keep this first */
 	dev_t			s_dev;		/* search index; _not_ kdev_t */
@@ -1196,6 +1203,7 @@ struct super_block {
 
 	struct list_head	s_inodes;	/* all inodes */
 	struct hlist_bl_head	s_anon;		/* anonymous dentries for (nfs) exporting */
+	struct list_head	s_sbdevs;	/* internal fs dev_t */
 	struct list_head	s_mounts;	/* list of mounts; _not_ for fs use */
 	struct block_device	*s_bdev;
 	struct backing_dev_info *s_bdi;
@@ -1796,6 +1804,8 @@ void deactivate_locked_super(struct super_block *sb);
 int set_anon_super(struct super_block *s, void *data);
 int get_anon_bdev(dev_t *);
 void free_anon_bdev(dev_t);
+int insert_anon_sbdev(struct super_block *sb, struct super_block_dev *sbdev);
+void remove_anon_sbdev(struct super_block_dev *sbdev);
 struct super_block *sget(struct file_system_type *type,
 			int (*test)(struct super_block *,void *),
 			int (*set)(struct super_block *,void *),
-- 
2.0.1

[RFC v2 2/2] btrfs: use the new VFS super_block_dev

[Luis R. Rodriguez]

From: "Luis R. Rodriguez" <mcgrof@suse.com>

Use the new VFS layer struct super_block_dev instead of carrying
the anonymous bdev's on our own. This makes the VFS layer aware of
all of our anonymous dev's on the super block.

Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 fs/btrfs/ctree.h   | 7 ++-----
 fs/btrfs/disk-io.c | 6 ++----
 fs/btrfs/inode.c   | 2 +-
 3 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index be91397..0ece396 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1846,11 +1846,8 @@ struct btrfs_root {
 	 * protected by inode_lock
 	 */
 	struct radix_tree_root delayed_nodes_tree;
-	/*
-	 * right now this just gets used so that a root has its own devid
-	 * for stat.  It may be used for more later
-	 */
-	dev_t anon_dev;
+
+	struct super_block_dev sbdev;
 
 	spinlock_t root_item_lock;
 	atomic_t refs;
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 8bb4aa1..1a6aa8d 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1269,7 +1269,6 @@ static void __setup_root(u32 nodesize, u32 leafsize, u32 sectorsize,
 		root->defrag_trans_start = 0;
 	init_completion(&root->kobj_unregister);
 	root->root_key.objectid = objectid;
-	root->anon_dev = 0;
 
 	spin_lock_init(&root->root_item_lock);
 }
@@ -1572,7 +1571,7 @@ int btrfs_init_fs_root(struct btrfs_root *root)
 	spin_lock_init(&root->cache_lock);
 	init_waitqueue_head(&root->cache_wait);
 
-	ret = get_anon_bdev(&root->anon_dev);
+	ret = insert_anon_sbdev(root->fs_info->sb, &root->sbdev);
 	if (ret)
 		goto free_writers;
 	return 0;
@@ -3529,8 +3528,7 @@ static void free_fs_root(struct btrfs_root *root)
 	WARN_ON(!RB_EMPTY_ROOT(&root->inode_tree));
 	btrfs_free_block_rsv(root, root->orphan_block_rsv);
 	root->orphan_block_rsv = NULL;
-	if (root->anon_dev)
-		free_anon_bdev(root->anon_dev);
+	remove_anon_sbdev(&root->sbdev);
 	if (root->subv_writers)
 		btrfs_free_subvolume_writers(root->subv_writers);
 	free_extent_buffer(root->node);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 3668048..0e8f604 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8277,7 +8277,7 @@ static int btrfs_getattr(struct vfsmount *mnt,
 	u32 blocksize = inode->i_sb->s_blocksize;
 
 	generic_fillattr(inode, stat);
-	stat->dev = BTRFS_I(inode)->root->anon_dev;
+	stat->dev = BTRFS_I(inode)->root->sbdev.anon_dev;
 	stat->blksize = PAGE_CACHE_SIZE;
 
 	spin_lock(&BTRFS_I(inode)->lock);
-- 
2.0.1

Re: [RFC v2 0/2] vfs / btrfs: add support for ustat()

[Christoph Hellwig]

On Wed, Jul 16, 2014 at 02:37:56PM -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
> 
> This makes the implementation simpler by stuffing the struct on
> the driver and just letting the driver iinsert it and remove it
> onto the sb list. This avoids the kzalloc() completely.

Again, NAK.  Make btrfs report the proper anon dev_t in stat and
everything will just work.

Re: [RFC v2 0/2] vfs / btrfs: add support for ustat()

[Luis R. Rodriguez]

On Thu, Jul 17, 2014 at 01:03:01AM -0700, Christoph Hellwig wrote:
> On Wed, Jul 16, 2014 at 02:37:56PM -0700, Luis R. Rodriguez wrote:
> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> > 
> > This makes the implementation simpler by stuffing the struct on
> > the driver and just letting the driver iinsert it and remove it
> > onto the sb list. This avoids the kzalloc() completely.
> 
> Again, NAK.  Make btrfs report the proper anon dev_t in stat and
> everything will just work.

Let's consider this userspace case:

        struct stat buf;                                                        
        struct ustat ubuf;                                                      
                                                                                
        /* Find a valid device number */                                        
        if (stat("/", &buf)) {
                fprintf(stderr, "Stat failed: %s\n", strerror(errno));          
                return 1;                                                       
        }                                                                       
                                                                                
        /* Call ustat on it */                                                  
        if (ustat(buf.st_dev, &ubuf)) {                                         
                fprintf(stderr, "Ustat failed: %s\n", strerror(errno));         
                return 1;                                                       
        } 

In the btrfs case it has an inode op for getattr, that is used and we set
the dev to anonymous dev_t. Later ustat will use user_get_super() which
will only be able to work with a userblock if the super block's only
dev_t is assigned to it. Since we have many anonymous to dev_t mapping
to super block though we can't complete the search for btfs and ustat()
fails with -EINVAL. The series expands the number of dev_t's that a super
block can have and allows this search to complete.

  Luis

Re: [RFC v2 0/2] vfs / btrfs: add support for ustat()

[Luis R. Rodriguez]

On Thu, Jul 17, 2014 at 1:49 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> On Thu, Jul 17, 2014 at 01:03:01AM -0700, Christoph Hellwig wrote:
>> On Wed, Jul 16, 2014 at 02:37:56PM -0700, Luis R. Rodriguez wrote:
>> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
>> >
>> > This makes the implementation simpler by stuffing the struct on
>> > the driver and just letting the driver iinsert it and remove it
>> > onto the sb list. This avoids the kzalloc() completely.
>>
>> Again, NAK.  Make btrfs report the proper anon dev_t in stat and
>> everything will just work.
>
> Let's consider this userspace case:
>
>         struct stat buf;
>         struct ustat ubuf;
>
>         /* Find a valid device number */
>         if (stat("/", &buf)) {
>                 fprintf(stderr, "Stat failed: %s\n", strerror(errno));
>                 return 1;
>         }
>
>         /* Call ustat on it */
>         if (ustat(buf.st_dev, &ubuf)) {
>                 fprintf(stderr, "Ustat failed: %s\n", strerror(errno));
>                 return 1;
>         }
>
> In the btrfs case it has an inode op for getattr, that is used and we set
> the dev to anonymous dev_t. Later ustat will use user_get_super() which
> will only be able to work with a userblock if the super block's only
> dev_t is assigned to it. Since we have many anonymous to dev_t mapping
> to super block though we can't complete the search for btfs and ustat()
> fails with -EINVAL. The series expands the number of dev_t's that a super
> block can have and allows this search to complete.

Any further advice? I'll submit a v3 for RFC with some small change
for a fix for stress testing identified by Filipe Manana.

  Luis

Re: [RFC v2 0/2] vfs / btrfs: add support for ustat()

[Christoph Hellwig]

On Thu, Aug 14, 2014 at 10:48:05PM -0400, Luis R. Rodriguez wrote:
> Any further advice? I'll submit a v3 for RFC with some small change
> for a fix for stress testing identified by Filipe Manana.

The advice is to stop setting different dev_t values for different
files in btrfs.