Btrfs: fix wrong the mount information in /proc

PATCH In review Miao Xie 3 participants 4 messages May 31, 2012

Subsystems (2)

VFS Btrfs file system

lore

Share & tools

lore

Miao Xie

author

2012-05-31 10:27 UTC #0 lore raw

PATCH 2/2 Btrfs: fix wrong the mount information in /proc

If we remove the disk that is specified when mounting, the mount information in /proc can not be updated. It will make us can not add that disk back into the filesystem. This patch fix this problem by implement the show_devname() interface and choose the name of the device with minimum device id to show.

Signed-off-by Miao Xie <[email protected]>

---
 fs/btrfs/super.c |   31 +++++++++++++++++++++++++++++++
 1 files changed, 31 insertions(+), 0 deletions(-)

fs/btrfs/super.c +31 -0

index c5f8fca..99e5671 100644
@@ -41,6 +41,7 @@
 #include <linux/slab.h>
 #include <linux/cleancache.h>
 #include <linux/ratelimit.h>
+#include "../mount.h"
 #include "compat.h"
 #include "delayed-inode.h"
 #include "ctree.h"

@@ -892,6 +893,35 @@ static int btrfs_show_options(struct seq_file *seq, struct dentry *dentry)
 	return 0;
 }
 
+static int btrfs_show_devname(struct seq_file *m, struct vfsmount *mnt)
+{
+	struct btrfs_fs_info *fs_info = btrfs_sb(mnt->mnt_sb);
+	struct mount *r = real_mount(mnt);
+	struct btrfs_fs_devices *cur_devices;
+	struct btrfs_device *dev;
+	struct list_head *head;
+	struct btrfs_device *first_dev = NULL;
+
+	mutex_lock(&fs_info->fs_devices->device_list_mutex);
+	cur_devices = fs_info->fs_devices;
+	while (cur_devices) {
+		head = &cur_devices->devices;
+		list_for_each_entry(dev, head, dev_list) {
+			if (!strcmp(r->mnt_devname, dev->name)) {
+				seq_escape(m, r->mnt_devname, " \t\n\\");
+				goto out;
+			}
+			if (!first_dev || dev->devid < first_dev->devid)
+				first_dev = dev;
+		}
+		cur_devices = cur_devices->seed;
+	}
+	seq_escape(m, first_dev->name, " \t\n\\");
+out:
+	mutex_unlock(&fs_info->fs_devices->device_list_mutex);
+	return 0;
+}
+
 static int btrfs_test_super(struct super_block *s, void *data)
 {
 	struct btrfs_fs_info *p = data;

@@ -1467,6 +1497,7 @@ static const struct super_operations btrfs_super_ops = {
 	.put_super	= btrfs_put_super,
 	.sync_fs	= btrfs_sync_fs,
 	.show_options	= btrfs_show_options,
+	.show_devname	= btrfs_show_devname,
 	.write_inode	= btrfs_write_inode,
 	.dirty_inode	= btrfs_fs_dirty_inode,
 	.alloc_inode	= btrfs_alloc_inode,

Signature · 1 line

1.7.6.5

Al Viro

maintainer

2012-05-31 14:37 UTC #1 lore raw

Quoted 1 line · Miao Xie

+#include "../mount.h"

No.

fs/btrfs/super.c static int btrfs_show_options(struct seq_file *seq, struct dentry *dentry) · line 899

Quoted 1 line

fs/btrfs/super.c +1 -0

+	struct mount *r = real_mount(mnt);

And even more so. Find a way to do that without layering violations or live with the current behaviour.

Both patches NAKed with extreme prejudice. Don't bring them back, you are *not* going to get them in. Any filesystem code that wants to do that kind of thing is seriously out of luck.

Al Viro

maintainer

2012-05-31 15:53 UTC #2 lore raw

Quoted 5 lines · Al Viro

Quoted 1 line · Miao Xie
+#include "../mount.h"
No.

Quoted 8 lines

fs/btrfs/super.c static int btrfs_show_options(struct seq_file *seq, struct dentry *dentry) · line 899
Quoted 1 line
fs/btrfs/super.c +1 -0
+	struct mount *r = real_mount(mnt);
And even more so. Find a way to do that without layering violations or live with the current behaviour.
Both patches NAKed with extreme prejudice. Don't bring them back, you are *not* going to get them in. Any filesystem code that wants to do that kind of thing is seriously out of luck.

BTW, resulting behaviour is bogus, regardless of layering violations - note that device_list_add() renames existing btrfs_device. *And* does so without holding ->device_list_mutex, so your patch would've cheerfully walked right into strcmp() vs. kfree() races. And while the order of kfree vs. reassignment in device_list_add() is clearly wrong, flipping it won't fix that problem.

Try to formulate the rules for names you are generating - when do they change, what can we say about two mounts by comparing these names, when can two that used to give different names start giving the same and vice versa. You won't be happy with the result.

Josef Bacik

2012-05-31 18:15 UTC #3 lore raw

Quoted 22 lines · Al Viro

Quoted 5 lines · Al Viro
Quoted 1 line · Miao Xie
+#include "../mount.h"
No.
Quoted 8 lines
fs/btrfs/super.c static int btrfs_show_options(struct seq_file *seq, struct dentry *dentry) · line 899
Quoted 1 line
fs/btrfs/super.c +1 -0
+	struct mount *r = real_mount(mnt);
And even more so. Find a way to do that without layering violations or live with the current behaviour.
Both patches NAKed with extreme prejudice. Don't bring them back, you are *not* going to get them in. Any filesystem code that wants to do that kind of thing is seriously out of luck.
BTW, resulting behaviour is bogus, regardless of layering violations - note that device_list_add() renames existing btrfs_device. *And* does so without holding ->device_list_mutex, so your patch would've cheerfully walked right into strcmp() vs. kfree() races. And while the order of kfree vs. reassignment in device_list_add() is clearly wrong, flipping it won't fix that problem.

Quoted 4 lines

Try to formulate the rules for names you are generating - when do they change, what can we say about two mounts by comparing these names, when can two that used to give different names start giving the same and vice versa. You won't be happy with the result.

I'm fixing this up right now and then I'll also fix the show_devname thing, we can just keep track of which device mount() was called with so that we can print that out and then if that device is ever removed we can just print out something different. Thanks,

Josef