On Wed, 10 Sep 2025, Al Viro wrote: > On Wed, Sep 10, 2025 at 05:12:49AM +0100, Al Viro wrote: > > On Wed, Sep 10, 2025 at 01:01:49PM +1000, NeilBrown wrote: > > > On Tue, 09 Sep 2025, Al Viro wrote: > > > > On Tue, Sep 09, 2025 at 02:43:21PM +1000, NeilBrown wrote: > > > > > > > > > d_instantiate(dentry, inode); > > > > > - dget(dentry); > > > > > -fail: > > > > > - inode_unlock(d_inode(parent)); > > > > > - return dentry; > > > > > + return simple_end_creating(dentry); > > > > > > > > No. This is the wrong model - dget() belongs with d_instantiate() > > > > here; your simple_end_creating() calling conventions are wrong. > > > > > > I can see that I shouldn't have removed the dget() there - thanks. > > > It is not entirely clear why hypfs_create_file() returns with two > > > references held to the dentry.... > > > I see now one is added either to ->update_file or the list at > > > hypfs_last_dentry, and the other is disposed of by kill_litter_super(). > > > > > > But apart from that one error is there something broader wrong with the > > > patch? You say "the wrong model" but I don't see it. > > > > See below for hypfs: > > ... and see viro/vfs.git#work.persistency for the part of the queue that > had order already settled down (I'm reshuffling the tail at the moment; > hypfs commit is still in the leftovers pile - the whole thing used to > have a really messy topology, with most of the prep work that used to > be the cause of that topology already in mainline - e.g. rpc_pipefs > series, securityfs one, etc.) > Thanks. I now see you were saying that you are introducing a new model for these simple_ filesystems and the simple_end_creating() I proposed does not fit with that model. Fair enough - I do agree your model is better. I assume it will eventually remove DCACHE_DENTRY_KILLED as setting that is equivalent to clearing the new DCACHE_PERSISTENT. So I'll leave those filesystems alone for now, though I would like to change "start_creating()" in debugfs to something like "debugfs_start_creating()" so I can use the generic name elsewhere. But are you OK with 1-5 of this series? I've got a comment addition but no other changes suggested in review. May I add the start_creating() rename patch 6? Thanks, NeilBrown
