Re: [PATCH 2/2] smb: client: add support for O_TMPFILE

[Al Viro <viro@zeniv.linux.org.uk>]

On Sun, Apr 05, 2026 at 06:18:19PM -0300, Paulo Alcantara wrote:

> @@ -436,17 +457,13 @@ static int cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned
>  		goto out_err;
>  	}
>  
> -	if (newinode)
> -		if (S_ISDIR(newinode->i_mode)) {
> -			rc = -EISDIR;
> -			goto out_err;
> -		}
> +	if (newinode && S_ISDIR(newinode->i_mode)) {
> +		rc = -EISDIR;
> +		goto out_err;
> +	}
>  
>  	d_drop(direntry);
> -	d_add(direntry, newinode);

> +		rc = __cifs_do_create(dir, direntry, full_path, xid,
> +				      tlink, oflags, mode, oplock,
> +				      fid, buf, &inode);
> +		if (!rc)
> +			d_add(direntry, inode);


> +		rc = __cifs_do_create(dir, dentry, path, xid, tlink,
> +				      file->f_flags, mode, &oplock,
> +				      &fid, NULL, &inode);
> +		if (!rc) {
> +			set_nlink(inode, 0);
> +			mark_inode_dirty(inode);
> +			d_mark_tmpfile_name(file, &QSTR_LEN(name, size - 1));
> +			d_instantiate(dentry, inode);

I really don't like this "not sure what the state is, d_drop() to
get it unhashed" pattern, _especially_ when d_drop() and d_add() or
d_instantiate() get separated.

Note, BTW, this d_add() call site is one of the two in the entire kernel
that are neither d_splice_alias() in disguise nor pass NULL as inode.
The other one is nfs_link(), where we also have this kind of "d_drop()
first, then use d_add()" pattern.

Folks, what state can dentry be in cifs_do_create()?  I'd rather see
that sorted out, not obfuscated even more.

FWIW, that's a major headache for Neil's stuff around directory locking
changes - any place where we play with unhash-and-rehash needs separate
analysis.  d_drop() is not something to be used lightly; it obfuscates the
dentry state and we'll need to translate those to the new locking scheme.