On Wed, Sep 10, 2025 at 08:24:23AM +0100, Al Viro wrote:
> Note that these unwrap_dentry() are very likely to move into helpers - if some
> function is always called with unwrapped_dentry(something) as an argument,
> great, that's probably a candidate for struct stable_dentry.
>
> I'll hold onto the current variant for now...
BTW, fun fallout from that experiment once I've got to ->atomic_open() - things
get nicer if we teach finish_no_open() to accept ERR_PTR() for dentry:
int finish_no_open(struct file *file, struct dentry *dentry)
{
if (IS_ERR(dentry))
return PTR_ERR(dentry);
file->f_path.dentry = dentry;
return 0;
}
For example, we get
int nfs_atomic_open_v23(struct inode *dir, struct dentry *dentry,
struct file *file, unsigned int open_flags,
umode_t mode)
{
struct dentry *res;
/* Same as look+open from lookup_open(), but with different O_TRUNC
* handling.
*/
int error = 0;
if (dentry->d_name.len > NFS_SERVER(dir)->namelen)
return -ENAMETOOLONG;
if (open_flags & O_CREAT) {
file->f_mode |= FMODE_CREATED;
error = nfs_do_create(dir, dentry, mode, open_flags);
if (error)
return error;
return finish_open(file, dentry, NULL);
}
if (d_in_lookup(dentry)) {
/* The only flags nfs_lookup considers are
* LOOKUP_EXCL and LOOKUP_RENAME_TARGET, and
* we want those to be zero so the lookup isn't skipped.
*/
res = nfs_lookup(dir, dentry, 0);
}
return finish_no_open(file, res);
}
and in cifs !O_CREAT case folds into
if (!(oflags & O_CREAT)) {
/*
* Check for hashed negative dentry. We have already revalidated
* the dentry and it is fine. No need to perform another lookup.
*/
if (!d_in_lookup(direntry))
return -ENOENT;
return finish_no_open(cifs_lookup(inode, direntry, 0));
}
In vboxsf, and similar in 9p and fuse:
if (d_in_lookup(dentry)) {
struct dentry *res = vboxsf_dir_lookup(parent, dentry, 0);
if (res || d_really_is_positive(dentry))
return finish_no_open(file, res);
}
/* Only creates */
if (!(flags & O_CREAT))
return finish_no_open(file, NULL);
...
The thing is, in that form it's really clear that dentry stays stable if
we proceed past those chunks; basically, d_splice_alias() only returns
non-NULL if it's got an error or a preexisting directory alias, in which
case result will be positive. And it's more compact and easier to follow
in that form...
While we are at it, Miklos mentioned some plans for changing ->atomic_open()
calling conventions. Might be a good time to revisit that... Miklos,
could you give a braindump on those plans?
