On Sat, Sep 03, 2022 at 11:40:44AM +1000, NeilBrown wrote: > I don't think that is a good idea. Once you call d_lookup_done() > (without having first called d_add() or similar) the dentry becomes > invisible to normal path lookup, so another might be created. But the > dentry will still be used for the 'create' or 'rename' and may then be > added to the dcache - at which point you could have two dentries with the > same name. > > When ->lookup() returns success without d_add()ing the dentry, that > means that something else will complete the d_add() if/when necessary. > For NFS, it specifically means that the lookup is effectively being > combined with the following CREATE or RENAME. In this case there is no > d_lookup_done() until the full operation is complete. > > For autofs (thanks for pointing me to that) the operation is completed > when d_automount() signals the daemon to create the directory or > symlink. In that case there IS a d_lookup_done() call and autofs needs > some extra magic (the internal 'active' list) to make sure subsequent > ->lookup requests can see that dentry which is still in the process of > being set up. > > It might be nice if the dentry passed to autofs_lookup() could remain > "d_inlookup()" until after d_automount has completed. Then autofs > wouldn't need that active list. However I haven't yet looked at how > disruptive such a change might be. Very much so. You are starting to invent new rules for ->lookup() that just never had been there, basing on nothing better than a couple of examples. They are nowhere near everything there is. And you can't rely upon d_add() done by a method, for very obvious reasons. They are out of your control, they might very well decide that object creation has failed and drop the damn thing. Which is not allowed for in-lookup dentries without d_lookup_done(). Neil, *IF* you are introducing new rules like that, the absolutely minimal requirement is having them in Documentation/filesystems/porting.rst. And that includes "such-and-such method might be called with parent locked only shared; in that case it's guaranteed such-and-such things about its arguments (bitlocks held, etc.)". One thing we really need to avoid is that thing coming undocumented, with "NFS copes, nobody else has it enabled, whoever does it for other filesystems will just have to RTFS". I hope it's obvious that this is not an option. Because I can bloody guarantee that it will be cargo-culted over to other filesystems, with nobody (you and me included) understanding the resulting code.
