| Message ID | 1405933349-6215-1-git-send-email-quwenruo@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Mon, Jul 21, 2014 at 05:02:29PM +0800, Qu Wenruo wrote: > show_path() function in struct super_operations is used to output > subtree mount info for mountinfo. > Without the implement of show_path() function, user can not found where > each subvolume is mounted if using 'subvolid=' mount option. > (When mounted with 'subvol=' mount option, vfs is aware of subtree mount > and can to the path resolve by vfs itself) Your previous patches unify both to call mount_subtree, then the default vfs implementation of show_path will do the right thing, ie seq_dentry(...), and the path will be resolved for free. Means this patch is not needed, so I'll skip commenting it. > With this patch, end users will be able to use findmnt(8) or other > programs reading mountinfo to find which btrfs subvolume is mounted. > > Though we use fs_info->subvol_sem to protect show_path() from subvolume > destroying/creating, if user renames/moves the parent non-subvolume > dir of a subvolume, it is still possible that concurrency may happen and > cause btrfs_search_slot() fails to find the desired key. > In that case, we just return -EBUSY and info user to try again since > extra locking like locking the whole subvolume tree is too expensive for > such usage. And the subvolume renames will be handled as well. -- 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
Thanks for the comment. -------- Original Message -------- Subject: Re: [PATCH] btrfs: Add show_path function for btrfs_super_ops. From: David Sterba <dsterba@suse.cz> To: Qu Wenruo <quwenruo@cn.fujitsu.com> Date: 2014?07?24? 21:09 > On Mon, Jul 21, 2014 at 05:02:29PM +0800, Qu Wenruo wrote: >> show_path() function in struct super_operations is used to output >> subtree mount info for mountinfo. >> Without the implement of show_path() function, user can not found where >> each subvolume is mounted if using 'subvolid=' mount option. >> (When mounted with 'subvol=' mount option, vfs is aware of subtree mount >> and can to the path resolve by vfs itself) > Your previous patches unify both to call mount_subtree, then the default > vfs implementation of show_path will do the right thing, ie > seq_dentry(...), and the path will be resolved for free. > > Means this patch is not needed, so I'll skip commenting it. I'm sorry that I forgot to mention this patch is going to replace the previous patch(use mount_subtree method). Since vfs provide the show_path() function to do the fs specific subtree showing things, I would like to use it other than previous mount_subtree() trick. Also. as mentioned by Chandan Rajendra, previous subtree patch can't handle subvolume behind normal directory. This show_path() patch is somewhat v2 version of previous patch. > >> With this patch, end users will be able to use findmnt(8) or other >> programs reading mountinfo to find which btrfs subvolume is mounted. >> >> Though we use fs_info->subvol_sem to protect show_path() from subvolume >> destroying/creating, if user renames/moves the parent non-subvolume >> dir of a subvolume, it is still possible that concurrency may happen and >> cause btrfs_search_slot() fails to find the desired key. >> In that case, we just return -EBUSY and info user to try again since >> extra locking like locking the whole subvolume tree is too expensive for >> such usage. > And the subvolume renames will be handled as well. -- 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
Hi David, No mean to disturb you, but it has been serveral days since last comment. If you have any comment about the new patch, since due to my lack of explain the patch seems not reviewed, I am very glad to hear. If any other ideas occur to you, like the pro and cons about old mount trick vs show_path() implement, it would be also quite nice. Thanks, Qu -------- Original Message -------- Subject: Re: [PATCH] btrfs: Add show_path function for btrfs_super_ops. From: Qu Wenruo <quwenruo@cn.fujitsu.com> To: dsterba@suse.cz, linux-btrfs@vger.kernel.org Date: 2014?07?25? 08:40 > Thanks for the comment. > > -------- Original Message -------- > Subject: Re: [PATCH] btrfs: Add show_path function for btrfs_super_ops. > From: David Sterba <dsterba@suse.cz> > To: Qu Wenruo <quwenruo@cn.fujitsu.com> > Date: 2014?07?24? 21:09 >> On Mon, Jul 21, 2014 at 05:02:29PM +0800, Qu Wenruo wrote: >>> show_path() function in struct super_operations is used to output >>> subtree mount info for mountinfo. >>> Without the implement of show_path() function, user can not found where >>> each subvolume is mounted if using 'subvolid=' mount option. >>> (When mounted with 'subvol=' mount option, vfs is aware of subtree >>> mount >>> and can to the path resolve by vfs itself) >> Your previous patches unify both to call mount_subtree, then the default >> vfs implementation of show_path will do the right thing, ie >> seq_dentry(...), and the path will be resolved for free. >> >> Means this patch is not needed, so I'll skip commenting it. > > I'm sorry that I forgot to mention this patch is going to replace the > previous patch(use mount_subtree method). > > Since vfs provide the show_path() function to do the fs specific > subtree showing things, > I would like to use it other than previous mount_subtree() trick. > > Also. as mentioned by Chandan Rajendra, previous subtree patch can't > handle subvolume behind normal directory. > This show_path() patch is somewhat v2 version of previous patch. >> >>> With this patch, end users will be able to use findmnt(8) or other >>> programs reading mountinfo to find which btrfs subvolume is mounted. >>> >>> Though we use fs_info->subvol_sem to protect show_path() from subvolume >>> destroying/creating, if user renames/moves the parent non-subvolume >>> dir of a subvolume, it is still possible that concurrency may happen >>> and >>> cause btrfs_search_slot() fails to find the desired key. >>> In that case, we just return -EBUSY and info user to try again since >>> extra locking like locking the whole subvolume tree is too expensive >>> for >>> such usage. >> And the subvolume renames will be handled as well. -- 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
Ping? Any new comments? It has been a long time and the patch still not merged.... Thanks, Qu -------- Original Message -------- Subject: Re: [PATCH] btrfs: Add show_path function for btrfs_super_ops. From: Qu Wenruo <quwenruo@cn.fujitsu.com> To: <dsterba@suse.cz>, <linux-btrfs@vger.kernel.org> Date: 2014?07?31? 11:50 > Hi David, > > No mean to disturb you, but it has been serveral days since last comment. > > If you have any comment about the new patch, since due to my lack of > explain the patch seems not > reviewed, I am very glad to hear. > > If any other ideas occur to you, like the pro and cons about old mount > trick vs show_path() implement, > it would be also quite nice. > > Thanks, > Qu > > > -------- Original Message -------- > Subject: Re: [PATCH] btrfs: Add show_path function for btrfs_super_ops. > From: Qu Wenruo <quwenruo@cn.fujitsu.com> > To: dsterba@suse.cz, linux-btrfs@vger.kernel.org > Date: 2014?07?25? 08:40 >> Thanks for the comment. >> >> -------- Original Message -------- >> Subject: Re: [PATCH] btrfs: Add show_path function for btrfs_super_ops. >> From: David Sterba <dsterba@suse.cz> >> To: Qu Wenruo <quwenruo@cn.fujitsu.com> >> Date: 2014?07?24? 21:09 >>> On Mon, Jul 21, 2014 at 05:02:29PM +0800, Qu Wenruo wrote: >>>> show_path() function in struct super_operations is used to output >>>> subtree mount info for mountinfo. >>>> Without the implement of show_path() function, user can not found >>>> where >>>> each subvolume is mounted if using 'subvolid=' mount option. >>>> (When mounted with 'subvol=' mount option, vfs is aware of subtree >>>> mount >>>> and can to the path resolve by vfs itself) >>> Your previous patches unify both to call mount_subtree, then the >>> default >>> vfs implementation of show_path will do the right thing, ie >>> seq_dentry(...), and the path will be resolved for free. >>> >>> Means this patch is not needed, so I'll skip commenting it. >> >> I'm sorry that I forgot to mention this patch is going to replace the >> previous patch(use mount_subtree method). >> >> Since vfs provide the show_path() function to do the fs specific >> subtree showing things, >> I would like to use it other than previous mount_subtree() trick. >> >> Also. as mentioned by Chandan Rajendra, previous subtree patch can't >> handle subvolume behind normal directory. >> This show_path() patch is somewhat v2 version of previous patch. >>> >>>> With this patch, end users will be able to use findmnt(8) or other >>>> programs reading mountinfo to find which btrfs subvolume is mounted. >>>> >>>> Though we use fs_info->subvol_sem to protect show_path() from >>>> subvolume >>>> destroying/creating, if user renames/moves the parent non-subvolume >>>> dir of a subvolume, it is still possible that concurrency may >>>> happen and >>>> cause btrfs_search_slot() fails to find the desired key. >>>> In that case, we just return -EBUSY and info user to try again since >>>> extra locking like locking the whole subvolume tree is too >>>> expensive for >>>> such usage. >>> And the subvolume renames will be handled as well. > > -- > 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 -- 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
Hi, all. Patch from long long ago. Anyone to review/comment/merge it? Thanks Qu -------- Original Message -------- Subject: [PATCH] btrfs: Add show_path function for btrfs_super_ops. From: Qu Wenruo <quwenruo@cn.fujitsu.com> To: linux-btrfs@vger.kernel.org Date: 2014?07?21? 17:02 > show_path() function in struct super_operations is used to output > subtree mount info for mountinfo. > Without the implement of show_path() function, user can not found where > each subvolume is mounted if using 'subvolid=' mount option. > (When mounted with 'subvol=' mount option, vfs is aware of subtree mount > and can to the path resolve by vfs itself) > > With this patch, end users will be able to use findmnt(8) or other > programs reading mountinfo to find which btrfs subvolume is mounted. > > Though we use fs_info->subvol_sem to protect show_path() from subvolume > destroying/creating, if user renames/moves the parent non-subvolume > dir of a subvolume, it is still possible that concurrency may happen and > cause btrfs_search_slot() fails to find the desired key. > In that case, we just return -EBUSY and info user to try again since > extra locking like locking the whole subvolume tree is too expensive for > such usage. > > Reported-by: Stefan G.Weichinger <lists@xunil.at> > Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> > --- > fs/btrfs/ctree.h | 2 + > fs/btrfs/ioctl.c | 4 +- > fs/btrfs/super.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 116 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h > index be91397..63fba05 100644 > --- a/fs/btrfs/ctree.h > +++ b/fs/btrfs/ctree.h > @@ -3881,6 +3881,8 @@ void btrfs_get_block_group_info(struct list_head *groups_list, > struct btrfs_ioctl_space_info *space); > void update_ioctl_balance_args(struct btrfs_fs_info *fs_info, int lock, > struct btrfs_ioctl_balance_args *bargs); > +int btrfs_search_path_in_tree(struct btrfs_fs_info *info, > + u64 tree_id, u64 dirid, char *name); > > > /* file.c */ > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c > index 47aceb4..c2bd6b5 100644 > --- a/fs/btrfs/ioctl.c > +++ b/fs/btrfs/ioctl.c > @@ -2218,8 +2218,8 @@ static noinline int btrfs_ioctl_tree_search_v2(struct file *file, > * Search INODE_REFs to identify path name of 'dirid' directory > * in a 'tree_id' tree. and sets path name to 'name'. > */ > -static noinline int btrfs_search_path_in_tree(struct btrfs_fs_info *info, > - u64 tree_id, u64 dirid, char *name) > +int btrfs_search_path_in_tree(struct btrfs_fs_info *info, > + u64 tree_id, u64 dirid, char *name) > { > struct btrfs_root *root; > struct btrfs_key key; > diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c > index 8e16bca..b5ece81 100644 > --- a/fs/btrfs/super.c > +++ b/fs/btrfs/super.c > @@ -1831,6 +1831,117 @@ static int btrfs_show_devname(struct seq_file *m, struct dentry *root) > return 0; > } > > +static char *str_prepend(char *dest, char *src) > +{ > + memmove(dest + strlen(src), dest, strlen(dest) + 1); > + memcpy(dest, src, strlen(src)); > + return dest; > +} > + > +static int alloc_mem_if_needed(char **dest, char *src, int *len) > +{ > + char *tmp; > + > + if (unlikely(strlen(*dest) + strlen(src) > *len)) { > + *len *= 2; > + tmp = krealloc(*dest, *len, GFP_NOFS); > + if (!tmp) { > + return -ENOMEM; > + } > + *dest = tmp; > + } > + return 0; > +} > + > +static int btrfs_show_path(struct seq_file *m, struct dentry *mount_root) > +{ > + struct inode *inode = mount_root->d_inode; > + struct btrfs_root *subv_root = BTRFS_I(inode)->root; > + struct btrfs_fs_info *fs_info = subv_root->fs_info; > + struct btrfs_root *tree_root = fs_info->tree_root; > + struct btrfs_root_ref *ref; > + struct btrfs_key key; > + struct btrfs_key found_key; > + struct btrfs_path *path = NULL; > + char *name = NULL; > + char *buf = NULL; > + int ret = 0; > + int len; > + u64 dirid = 0; > + u16 namelen; > + > + name = kmalloc(PAGE_SIZE, GFP_NOFS); > + len = PAGE_SIZE; > + buf = kmalloc(BTRFS_INO_LOOKUP_PATH_MAX, GFP_NOFS); > + path = btrfs_alloc_path(); > + if (!name || !buf || !path) { > + ret = -ENOMEM; > + goto out_free; > + } > + *name = '/'; > + *(name + 1) = '\0'; > + > + key.objectid = subv_root->root_key.objectid; > + key.type = BTRFS_ROOT_BACKREF_KEY; > + key.offset = 0; > + down_read(&fs_info->subvol_sem); > + while (key.objectid != BTRFS_FS_TREE_OBJECTID) { > + ret = btrfs_search_slot_for_read(tree_root, &key, path, 1, 1); > + if (ret < 0) > + goto out; > + if (ret) { > + ret = -ENOENT; > + goto out; > + } > + btrfs_item_key_to_cpu(path->nodes[0], &found_key, > + path->slots[0]); > + if (found_key.objectid != key.objectid || > + found_key.type != BTRFS_ROOT_BACKREF_KEY) { > + ret = -ENOENT; > + goto out; > + } > + /* append the subvol name first */ > + ref = btrfs_item_ptr(path->nodes[0], path->slots[0], > + struct btrfs_root_ref); > + dirid = btrfs_root_ref_dirid(path->nodes[0], ref); > + namelen = btrfs_root_ref_name_len(path->nodes[0], ref); > + read_extent_buffer(path->nodes[0], buf, > + (unsigned long)(ref + 1), namelen); > + *(buf + namelen) = '/'; > + *(buf + namelen + 1) = '\0'; > + ret = alloc_mem_if_needed(&name, buf, &len); > + if (ret < 0) > + goto out; > + str_prepend(name + 1, buf); > + > + /* then append path name inside the subvole */ > + ret = btrfs_search_path_in_tree(fs_info, found_key.offset, > + dirid, buf); > + if (ret < 0) { > + if (ret == -ENOENT) > + /* parent dir may be under rename/moving, > + * info user to try again other than > + * "No such file or directory" */ > + ret = -EBUSY; > + goto out; > + } > + btrfs_release_path(path); > + key.objectid = found_key.offset; > + ret = alloc_mem_if_needed(&name, buf, &len); > + if (ret < 0) > + goto out; > + str_prepend(name + 1, buf); > + } > + seq_puts(m, name); > +out: > + up_read(&fs_info->subvol_sem); > +out_free: > + btrfs_free_path(path); > + kfree(buf); > + kfree(name); > + return ret; > +} > + > static const struct super_operations btrfs_super_ops = { > .drop_inode = btrfs_drop_inode, > .evict_inode = btrfs_evict_inode, > @@ -1845,6 +1956,7 @@ static const struct super_operations btrfs_super_ops = { > .remount_fs = btrfs_remount, > .freeze_fs = btrfs_freeze, > .unfreeze_fs = btrfs_unfreeze, > + .show_path = btrfs_show_path, > }; > > static const struct file_operations btrfs_ctl_fops = { > -- 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
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index be91397..63fba05 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -3881,6 +3881,8 @@ void btrfs_get_block_group_info(struct list_head *groups_list, struct btrfs_ioctl_space_info *space); void update_ioctl_balance_args(struct btrfs_fs_info *fs_info, int lock, struct btrfs_ioctl_balance_args *bargs); +int btrfs_search_path_in_tree(struct btrfs_fs_info *info, + u64 tree_id, u64 dirid, char *name); /* file.c */ diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 47aceb4..c2bd6b5 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -2218,8 +2218,8 @@ static noinline int btrfs_ioctl_tree_search_v2(struct file *file, * Search INODE_REFs to identify path name of 'dirid' directory * in a 'tree_id' tree. and sets path name to 'name'. */ -static noinline int btrfs_search_path_in_tree(struct btrfs_fs_info *info, - u64 tree_id, u64 dirid, char *name) +int btrfs_search_path_in_tree(struct btrfs_fs_info *info, + u64 tree_id, u64 dirid, char *name) { struct btrfs_root *root; struct btrfs_key key; diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 8e16bca..b5ece81 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -1831,6 +1831,117 @@ static int btrfs_show_devname(struct seq_file *m, struct dentry *root) return 0; } +static char *str_prepend(char *dest, char *src) +{ + memmove(dest + strlen(src), dest, strlen(dest) + 1); + memcpy(dest, src, strlen(src)); + return dest; +} + +static int alloc_mem_if_needed(char **dest, char *src, int *len) +{ + char *tmp; + + if (unlikely(strlen(*dest) + strlen(src) > *len)) { + *len *= 2; + tmp = krealloc(*dest, *len, GFP_NOFS); + if (!tmp) { + return -ENOMEM; + } + *dest = tmp; + } + return 0; +} + +static int btrfs_show_path(struct seq_file *m, struct dentry *mount_root) +{ + struct inode *inode = mount_root->d_inode; + struct btrfs_root *subv_root = BTRFS_I(inode)->root; + struct btrfs_fs_info *fs_info = subv_root->fs_info; + struct btrfs_root *tree_root = fs_info->tree_root; + struct btrfs_root_ref *ref; + struct btrfs_key key; + struct btrfs_key found_key; + struct btrfs_path *path = NULL; + char *name = NULL; + char *buf = NULL; + int ret = 0; + int len; + u64 dirid = 0; + u16 namelen; + + name = kmalloc(PAGE_SIZE, GFP_NOFS); + len = PAGE_SIZE; + buf = kmalloc(BTRFS_INO_LOOKUP_PATH_MAX, GFP_NOFS); + path = btrfs_alloc_path(); + if (!name || !buf || !path) { + ret = -ENOMEM; + goto out_free; + } + *name = '/'; + *(name + 1) = '\0'; + + key.objectid = subv_root->root_key.objectid; + key.type = BTRFS_ROOT_BACKREF_KEY; + key.offset = 0; + down_read(&fs_info->subvol_sem); + while (key.objectid != BTRFS_FS_TREE_OBJECTID) { + ret = btrfs_search_slot_for_read(tree_root, &key, path, 1, 1); + if (ret < 0) + goto out; + if (ret) { + ret = -ENOENT; + goto out; + } + btrfs_item_key_to_cpu(path->nodes[0], &found_key, + path->slots[0]); + if (found_key.objectid != key.objectid || + found_key.type != BTRFS_ROOT_BACKREF_KEY) { + ret = -ENOENT; + goto out; + } + /* append the subvol name first */ + ref = btrfs_item_ptr(path->nodes[0], path->slots[0], + struct btrfs_root_ref); + dirid = btrfs_root_ref_dirid(path->nodes[0], ref); + namelen = btrfs_root_ref_name_len(path->nodes[0], ref); + read_extent_buffer(path->nodes[0], buf, + (unsigned long)(ref + 1), namelen); + *(buf + namelen) = '/'; + *(buf + namelen + 1) = '\0'; + ret = alloc_mem_if_needed(&name, buf, &len); + if (ret < 0) + goto out; + str_prepend(name + 1, buf); + + /* then append path name inside the subvole */ + ret = btrfs_search_path_in_tree(fs_info, found_key.offset, + dirid, buf); + if (ret < 0) { + if (ret == -ENOENT) + /* parent dir may be under rename/moving, + * info user to try again other than + * "No such file or directory" */ + ret = -EBUSY; + goto out; + } + btrfs_release_path(path); + key.objectid = found_key.offset; + ret = alloc_mem_if_needed(&name, buf, &len); + if (ret < 0) + goto out; + str_prepend(name + 1, buf); + } + seq_puts(m, name); +out: + up_read(&fs_info->subvol_sem); +out_free: + btrfs_free_path(path); + kfree(buf); + kfree(name); + return ret; +} + static const struct super_operations btrfs_super_ops = { .drop_inode = btrfs_drop_inode, .evict_inode = btrfs_evict_inode, @@ -1845,6 +1956,7 @@ static const struct super_operations btrfs_super_ops = { .remount_fs = btrfs_remount, .freeze_fs = btrfs_freeze, .unfreeze_fs = btrfs_unfreeze, + .show_path = btrfs_show_path, }; static const struct file_operations btrfs_ctl_fops = {
show_path() function in struct super_operations is used to output subtree mount info for mountinfo. Without the implement of show_path() function, user can not found where each subvolume is mounted if using 'subvolid=' mount option. (When mounted with 'subvol=' mount option, vfs is aware of subtree mount and can to the path resolve by vfs itself) With this patch, end users will be able to use findmnt(8) or other programs reading mountinfo to find which btrfs subvolume is mounted. Though we use fs_info->subvol_sem to protect show_path() from subvolume destroying/creating, if user renames/moves the parent non-subvolume dir of a subvolume, it is still possible that concurrency may happen and cause btrfs_search_slot() fails to find the desired key. In that case, we just return -EBUSY and info user to try again since extra locking like locking the whole subvolume tree is too expensive for such usage. Reported-by: Stefan G.Weichinger <lists@xunil.at> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> --- fs/btrfs/ctree.h | 2 + fs/btrfs/ioctl.c | 4 +- fs/btrfs/super.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 116 insertions(+), 2 deletions(-)