On 7/6/25 4:54 PM, Song Liu wrote:
On Jul 4, 2025, at 10:40 AM, Yonghong Song <yonghong.song@xxxxxxxxx> wrote:
[...]
+static struct dentry *__path_walk_parent(struct path *path, const struct path *root, int flags)
{
- struct dentry *parent;
-
- if (path_equal(&nd->path, &nd->root))
+ if (path_equal(path, root))
goto in_root;
- if (unlikely(nd->path.dentry == nd->path.mnt->mnt_root)) {
- struct path path;
+ if (unlikely(path->dentry == path->mnt->mnt_root)) {
+ struct path new_path;
- if (!choose_mountpoint(real_mount(nd->path.mnt),
- &nd->root, &path))
+ if (!choose_mountpoint(real_mount(path->mnt),
+ root, &new_path))
goto in_root;
- path_put(&nd->path);
- nd->path = path;
- nd->inode = path.dentry->d_inode;
- if (unlikely(nd->flags & LOOKUP_NO_XDEV))
+ path_put(path);
+ *path = new_path;
+ if (unlikely(flags & LOOKUP_NO_XDEV))
return ERR_PTR(-EXDEV);
}
/* rare case of legitimate dget_parent()... */
- parent = dget_parent(nd->path.dentry);
+ return dget_parent(path->dentry);
I have some confusion with this patch when crossing mount boundary.
In d_path.c, we have
static int __prepend_path(const struct dentry *dentry, const struct mount *mnt,
const struct path *root, struct prepend_buffer *p)
{
while (dentry != root->dentry || &mnt->mnt != root->mnt) {
const struct dentry *parent = READ_ONCE(dentry->d_parent);
if (dentry == mnt->mnt.mnt_root) {
struct mount *m = READ_ONCE(mnt->mnt_parent);
struct mnt_namespace *mnt_ns;
if (likely(mnt != m)) {
dentry = READ_ONCE(mnt->mnt_mountpoint);
mnt = m;
continue;
}
/* Global root */
mnt_ns = READ_ONCE(mnt->mnt_ns);
/* open-coded is_mounted() to use local mnt_ns */
if (!IS_ERR_OR_NULL(mnt_ns) && !is_anon_ns(mnt_ns))
return 1; // absolute root
else
return 2; // detached or not attached yet
}
if (unlikely(dentry == parent))
/* Escaped? */
return 3;
prefetch(parent);
if (!prepend_name(p, &dentry->d_name))
break;
dentry = parent;
}
return 0;
}
At the mount boundary and not at root mount, the code has
dentry = READ_ONCE(mnt->mnt_mountpoint);
mnt = m; /* 'mnt' will be parent mount */
continue;
After that, we have
const struct dentry *parent = READ_ONCE(dentry->d_parent);
if (dentry == mnt->mnt.mnt_root) {
/* assume this is false */
}
...
prefetch(parent);
if (!prepend_name(p, &dentry->d_name))
break;
dentry = parent;
So the prepend_name(p, &dentry->d_name) is actually from mnt->mnt_mountpoint.
I am not quite following the question. In the code below:
if (dentry == mnt->mnt.mnt_root) {
struct mount *m = READ_ONCE(mnt->mnt_parent);
struct mnt_namespace *mnt_ns;
if (likely(mnt != m)) {
dentry = READ_ONCE(mnt->mnt_mountpoint);
mnt = m;
continue;
/* We either continue, here */
}
/* Global root */
mnt_ns = READ_ONCE(mnt->mnt_ns);
/* open-coded is_mounted() to use local mnt_ns */
if (!IS_ERR_OR_NULL(mnt_ns) && !is_anon_ns(mnt_ns))
return 1; // absolute root
else
return 2; // detached or not attached yet
/* Or return here */
}
So we will not hit prepend_name(). Does this answer the
question?
In your above code, maybe we should return path->dentry in the below if statement?
if (unlikely(path->dentry == path->mnt->mnt_root)) {
struct path new_path;
if (!choose_mountpoint(real_mount(path->mnt),
root, &new_path))
goto in_root;
path_put(path);
*path = new_path;
if (unlikely(flags & LOOKUP_NO_XDEV))
return ERR_PTR(-EXDEV);
+ return path->dentry;
}
/* rare case of legitimate dget_parent()... */
return dget_parent(path->dentry);
Also, could you add some selftests cross mount points? This will
have more coverages with __path_walk_parent().
Looks like __path_walk_parent() works for the root of mounted fs.
If this is the case, the implementation is correct. It could be
good to add some comments to clarify.
Yeah, I will try to add more tests in the next revision.
Thanks,
Song
