| Message ID | 20240606173912.99442-1-jemmywong512@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4] Improve readability of copy_tree | expand |
On Fri, 07 Jun 2024 01:39:12 +0800, Jemmy wrote: > by employing `copy mount tree from src to dst` concept. > This involves renaming the opaque variables (e.g., p, q, r, s) > to be more descriptive, aiming to make the code easier to understand. > > Changes: > mnt -> src_root (root of the tree to copy) > r -> src_root_child (direct child of the root being cloning) > p -> src_parent (parent of src_mnt) > s -> src_mnt (current mount being copying) > parent -> dst_parent (parent of dst_child) > q -> dst_mnt (freshly cloned mount) > > [...] Applied to the vfs.misc branch of the vfs/vfs.git tree. Patches in the vfs.misc branch should appear in linux-next soon. Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it. It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated. Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch. tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git branch: vfs.misc [1/1] Improve readability of copy_tree https://git.kernel.org/vfs/vfs/c/5692e7579306
On Fri 07-06-24 01:39:12, Jemmy wrote: > by employing `copy mount tree from src to dst` concept. > This involves renaming the opaque variables (e.g., p, q, r, s) > to be more descriptive, aiming to make the code easier to understand. > > Changes: > mnt -> src_root (root of the tree to copy) > r -> src_root_child (direct child of the root being cloning) > p -> src_parent (parent of src_mnt) > s -> src_mnt (current mount being copying) > parent -> dst_parent (parent of dst_child) > q -> dst_mnt (freshly cloned mount) > > Signed-off-by: Jemmy <jemmywong512@gmail.com> Thanks! Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > fs/namespace.c | 59 ++++++++++++++++++++++++++------------------------ > 1 file changed, 31 insertions(+), 28 deletions(-) > > diff --git a/fs/namespace.c b/fs/namespace.c > index 5a51315c6678..b0202e37515e 100644 > --- a/fs/namespace.c > +++ b/fs/namespace.c > @@ -1966,69 +1966,72 @@ static bool mnt_ns_loop(struct dentry *dentry) > return current->nsproxy->mnt_ns->seq >= mnt_ns->seq; > } > > -struct mount *copy_tree(struct mount *mnt, struct dentry *dentry, > +struct mount *copy_tree(struct mount *src_root, struct dentry *dentry, > int flag) > { > - struct mount *res, *p, *q, *r, *parent; > + struct mount *res, *src_parent, *src_root_child, *src_mnt, > + *dst_parent, *dst_mnt; > > - if (!(flag & CL_COPY_UNBINDABLE) && IS_MNT_UNBINDABLE(mnt)) > + if (!(flag & CL_COPY_UNBINDABLE) && IS_MNT_UNBINDABLE(src_root)) > return ERR_PTR(-EINVAL); > > if (!(flag & CL_COPY_MNT_NS_FILE) && is_mnt_ns_file(dentry)) > return ERR_PTR(-EINVAL); > > - res = q = clone_mnt(mnt, dentry, flag); > - if (IS_ERR(q)) > - return q; > + res = dst_mnt = clone_mnt(src_root, dentry, flag); > + if (IS_ERR(dst_mnt)) > + return dst_mnt; > > - q->mnt_mountpoint = mnt->mnt_mountpoint; > + src_parent = src_root; > + dst_mnt->mnt_mountpoint = src_root->mnt_mountpoint; > > - p = mnt; > - list_for_each_entry(r, &mnt->mnt_mounts, mnt_child) { > - struct mount *s; > - if (!is_subdir(r->mnt_mountpoint, dentry)) > + list_for_each_entry(src_root_child, &src_root->mnt_mounts, mnt_child) { > + if (!is_subdir(src_root_child->mnt_mountpoint, dentry)) > continue; > > - for (s = r; s; s = next_mnt(s, r)) { > + for (src_mnt = src_root_child; src_mnt; > + src_mnt = next_mnt(src_mnt, src_root_child)) { > if (!(flag & CL_COPY_UNBINDABLE) && > - IS_MNT_UNBINDABLE(s)) { > - if (s->mnt.mnt_flags & MNT_LOCKED) { > + IS_MNT_UNBINDABLE(src_mnt)) { > + if (src_mnt->mnt.mnt_flags & MNT_LOCKED) { > /* Both unbindable and locked. */ > - q = ERR_PTR(-EPERM); > + dst_mnt = ERR_PTR(-EPERM); > goto out; > } else { > - s = skip_mnt_tree(s); > + src_mnt = skip_mnt_tree(src_mnt); > continue; > } > } > if (!(flag & CL_COPY_MNT_NS_FILE) && > - is_mnt_ns_file(s->mnt.mnt_root)) { > - s = skip_mnt_tree(s); > + is_mnt_ns_file(src_mnt->mnt.mnt_root)) { > + src_mnt = skip_mnt_tree(src_mnt); > continue; > } > - while (p != s->mnt_parent) { > - p = p->mnt_parent; > - q = q->mnt_parent; > + while (src_parent != src_mnt->mnt_parent) { > + src_parent = src_parent->mnt_parent; > + dst_mnt = dst_mnt->mnt_parent; > } > - p = s; > - parent = q; > - q = clone_mnt(p, p->mnt.mnt_root, flag); > - if (IS_ERR(q)) > + > + src_parent = src_mnt; > + dst_parent = dst_mnt; > + dst_mnt = clone_mnt(src_mnt, src_mnt->mnt.mnt_root, flag); > + if (IS_ERR(dst_mnt)) > goto out; > lock_mount_hash(); > - list_add_tail(&q->mnt_list, &res->mnt_list); > - attach_mnt(q, parent, p->mnt_mp, false); > + list_add_tail(&dst_mnt->mnt_list, &res->mnt_list); > + attach_mnt(dst_mnt, dst_parent, src_parent->mnt_mp, false); > unlock_mount_hash(); > } > } > return res; > + > out: > if (res) { > lock_mount_hash(); > umount_tree(res, UMOUNT_SYNC); > unlock_mount_hash(); > } > - return q; > + return dst_mnt; > } > > /* Caller should check returned pointer for errors */ > -- > 2.34.1 >
diff --git a/fs/namespace.c b/fs/namespace.c index 5a51315c6678..b0202e37515e 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -1966,69 +1966,72 @@ static bool mnt_ns_loop(struct dentry *dentry) return current->nsproxy->mnt_ns->seq >= mnt_ns->seq; } -struct mount *copy_tree(struct mount *mnt, struct dentry *dentry, +struct mount *copy_tree(struct mount *src_root, struct dentry *dentry, int flag) { - struct mount *res, *p, *q, *r, *parent; + struct mount *res, *src_parent, *src_root_child, *src_mnt, + *dst_parent, *dst_mnt; - if (!(flag & CL_COPY_UNBINDABLE) && IS_MNT_UNBINDABLE(mnt)) + if (!(flag & CL_COPY_UNBINDABLE) && IS_MNT_UNBINDABLE(src_root)) return ERR_PTR(-EINVAL); if (!(flag & CL_COPY_MNT_NS_FILE) && is_mnt_ns_file(dentry)) return ERR_PTR(-EINVAL); - res = q = clone_mnt(mnt, dentry, flag); - if (IS_ERR(q)) - return q; + res = dst_mnt = clone_mnt(src_root, dentry, flag); + if (IS_ERR(dst_mnt)) + return dst_mnt; - q->mnt_mountpoint = mnt->mnt_mountpoint; + src_parent = src_root; + dst_mnt->mnt_mountpoint = src_root->mnt_mountpoint; - p = mnt; - list_for_each_entry(r, &mnt->mnt_mounts, mnt_child) { - struct mount *s; - if (!is_subdir(r->mnt_mountpoint, dentry)) + list_for_each_entry(src_root_child, &src_root->mnt_mounts, mnt_child) { + if (!is_subdir(src_root_child->mnt_mountpoint, dentry)) continue; - for (s = r; s; s = next_mnt(s, r)) { + for (src_mnt = src_root_child; src_mnt; + src_mnt = next_mnt(src_mnt, src_root_child)) { if (!(flag & CL_COPY_UNBINDABLE) && - IS_MNT_UNBINDABLE(s)) { - if (s->mnt.mnt_flags & MNT_LOCKED) { + IS_MNT_UNBINDABLE(src_mnt)) { + if (src_mnt->mnt.mnt_flags & MNT_LOCKED) { /* Both unbindable and locked. */ - q = ERR_PTR(-EPERM); + dst_mnt = ERR_PTR(-EPERM); goto out; } else { - s = skip_mnt_tree(s); + src_mnt = skip_mnt_tree(src_mnt); continue; } } if (!(flag & CL_COPY_MNT_NS_FILE) && - is_mnt_ns_file(s->mnt.mnt_root)) { - s = skip_mnt_tree(s); + is_mnt_ns_file(src_mnt->mnt.mnt_root)) { + src_mnt = skip_mnt_tree(src_mnt); continue; } - while (p != s->mnt_parent) { - p = p->mnt_parent; - q = q->mnt_parent; + while (src_parent != src_mnt->mnt_parent) { + src_parent = src_parent->mnt_parent; + dst_mnt = dst_mnt->mnt_parent; } - p = s; - parent = q; - q = clone_mnt(p, p->mnt.mnt_root, flag); - if (IS_ERR(q)) + + src_parent = src_mnt; + dst_parent = dst_mnt; + dst_mnt = clone_mnt(src_mnt, src_mnt->mnt.mnt_root, flag); + if (IS_ERR(dst_mnt)) goto out; lock_mount_hash(); - list_add_tail(&q->mnt_list, &res->mnt_list); - attach_mnt(q, parent, p->mnt_mp, false); + list_add_tail(&dst_mnt->mnt_list, &res->mnt_list); + attach_mnt(dst_mnt, dst_parent, src_parent->mnt_mp, false); unlock_mount_hash(); } } return res; + out: if (res) { lock_mount_hash(); umount_tree(res, UMOUNT_SYNC); unlock_mount_hash(); } - return q; + return dst_mnt; } /* Caller should check returned pointer for errors */
by employing `copy mount tree from src to dst` concept. This involves renaming the opaque variables (e.g., p, q, r, s) to be more descriptive, aiming to make the code easier to understand. Changes: mnt -> src_root (root of the tree to copy) r -> src_root_child (direct child of the root being cloning) p -> src_parent (parent of src_mnt) s -> src_mnt (current mount being copying) parent -> dst_parent (parent of dst_child) q -> dst_mnt (freshly cloned mount) Signed-off-by: Jemmy <jemmywong512@gmail.com> --- fs/namespace.c | 59 ++++++++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 28 deletions(-)