[0/3,RFC,v2] change ->mkdir() and vfs_mkdir() to return a dentry

Message

NeilBrown Feb. 17, 2025, 5:30 a.m. UTC

Here is a second attempt at this change.  Guided by Al I have handled
other file systems which don't return a hashed positive dentry on success.

It is not always possible to provide a reliable answer.  One example is
that cifs might find, after successfully creating a directory, that the
name now leads to a non-directory (due to a race with another client).
So callers of vfs_mkdir() need to cope with successful mkdir but no
usable dentry.  There is nothing they can do to recover and must
continue gracefully.  This failre mode is detected by the returned
dentry being unhashed.

ORIGINAL DESCRIPTION - updated to reflect changes.

This is a small set of patches which are needed before we can make the
locking on directory operations more fine grained.  I think they are
useful even if we don't go that direction.

Some callers of vfs_mkdir() need to operate on the resulting directory
but cannot be guaranteed that the dentry will be hashed and positive on
success - another dentry might have been used.

This patch changes ->mkdir to return a dentry, changes several
filesystems to return the correct dentry, and changes vfs_mkdir() to
return that dentry, only performing a lookup as a last resort.

I have not Cc: the developers of all the individual filesystems NFS.  I
or kernel-test-robot have build-tested all the changes.  If anyone sees
this on fs-devel and wants to provide a pre-emptive ack I will collect
those and avoid further posting for those fs.

Thanks,
NeilBrown


 [PATCH 1/3] Change inode_operations.mkdir to return struct dentry *
 [PATCH 2/3] nfs: change mkdir inode_operation to return alternate
 [PATCH 3/3] VFS: Change vfs_mkdir() to return the dentry.

Comments

Christian Brauner Feb. 17, 2025, 10:47 a.m. UTC | #1

On Mon, Feb 17, 2025 at 04:30:02PM +1100, NeilBrown wrote:
> Here is a second attempt at this change.  Guided by Al I have handled
> other file systems which don't return a hashed positive dentry on success.
> 
> It is not always possible to provide a reliable answer.  One example is
> that cifs might find, after successfully creating a directory, that the
> name now leads to a non-directory (due to a race with another client).
> So callers of vfs_mkdir() need to cope with successful mkdir but no
> usable dentry.  There is nothing they can do to recover and must
> continue gracefully.  This failre mode is detected by the returned
> dentry being unhashed.
> 
> ORIGINAL DESCRIPTION - updated to reflect changes.
> 
> This is a small set of patches which are needed before we can make the
> locking on directory operations more fine grained.  I think they are
> useful even if we don't go that direction.
> 
> Some callers of vfs_mkdir() need to operate on the resulting directory
> but cannot be guaranteed that the dentry will be hashed and positive on
> success - another dentry might have been used.
> 
> This patch changes ->mkdir to return a dentry, changes several

Thanks for doing that.

> filesystems to return the correct dentry, and changes vfs_mkdir() to
> return that dentry, only performing a lookup as a last resort.
> 
> I have not Cc: the developers of all the individual filesystems NFS.  I
> or kernel-test-robot have build-tested all the changes.  If anyone sees
> this on fs-devel and wants to provide a pre-emptive ack I will collect
> those and avoid further posting for those fs.

Once I'll merge this into async.dir it'll show up in fs-next which means
it'll get additional xfstest testing.