On Fri, 14 Feb 2025, Al Viro wrote: > On Fri, Feb 14, 2025 at 04:16:40PM +1100, NeilBrown wrote: > > 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 operation 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 NFS in particular > > to return the correct dentry (I believe it is the only filesystem to > > possibly not use the given dentry), and changes vfs_mkdir() to return > > that dentry, removing the look that a few callers currently need. > > > > I have not Cc: the developers of all the individual filesystems - only > > NFS. I have build-tested all the changes except hostfs. I can email > > them explicitly if/when this is otherwise acceptable. 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. > > 1) please, don't sprinkle the PTR_ERR_OR_ZERO() shite all over the place. > Almost always the same thing can be done without it and it ends up > being cleaner. Seriously. I've removed several PTR_ERR_OR_ZERO() calls. Some times that could be seen as a slight improvement, other times possibly a slight negative (depending on how one feels about PTR_ERR_OR_ZERO of course). I have left three as I cannot see how to remove them without making the code significant more clumsy. If you find the remaining few to still be objectionable I would be happy to see what alternate you would propose. Your other feedback has been quite helpful - thanks. NeilBrown > > 2) I suspect that having method instances return NULL for "just use the > argument" would would be harder to fuck up; basically, the same as for > ->lookup() instances. I'll try to tweak it and see what falls out... > > 3) I'm pretty sure that NFS is *not* the only filesystem that returns > unhashed negative in some success cases; will need to go over the instances > to verify that, though. >
