On Tue, Apr 02, 2019 at 09:49:34AM -0600, Jonathan Corbet wrote: > On Wed, 27 Mar 2019 16:16:53 +1100 > "Tobin C. Harding" <tobin@xxxxxxxxxx> wrote: > > > Hi Al, > > > > This series converts the VFS file Documentation/filesystems/vfs.txt to > > reStructuredText format. Please consider taking this series through > > your tree as apposed to Jon's tree because this set makes a fair amount > > of changes to VFS files (and also the VFS tree and docs tree are out of > > sync right now with the recent work by Mauro and Neil). > > Al, do you have any thoughts on how you want to handle this? I was about > to apply Jeff Layton's vfs.txt update, but would rather not create > conflicts unnecessarily. Let me know if you'd like me to pick this work > up. Frankly, I would rather see that file be eventually replaced by something saner, and I'm not talking about the format. Re Jeff's patch... + d_prune: called prior to pruning (i.e. unhashing and killing) a hashed + dentry from the dcache. is flat-out misguiding. First of all, it *is* called for unhashed dentries, TYVM. Furthermore, "prior to" is far too vague. What really happens: there's a point in state diagram for dentries where we commit to destroying a dentry and start taking it apart. That transition happens with ->d_lock of dentry, ->i_lock of its inode (if any) and ->d_lock of the parent (again, if any) held; ->d_prune() is the last chance for filesystem to see the (now doomed) dentry still intact. It doesn't matter whether it's hashed or not, etc. The locks held are sufficient to stabilize pretty much everything[1] in dentry and nothing is destroyed yet. The only apparent exception is ->d_count, but that's not real - we are guaranteed that there had been no other counted references to dentry at the decision point and that none could've been added. So this "oh, it's not 0 now, it's gone negative after lockref_mark_dead() the caller has just done" is a red herring. ->d_prune() must not drop/regain any of the locks held by caller. It must _not_ free anything attached to dentry - that belongs later in the shutdown sequence. If anything, I'm tempted to make it take const struct dentry * as argument, just to make that clear. No new (counted) references can be acquired by that point; lockless dcache lookup might find our dentry a match, but result of such lookup is not going to be legitimized - it's doomed to be thrown out as stale. It really makes more sense as part of struct dentry lifecycle description... [1] in theory, ->d_time might be changed by overlapping lockless call of ->d_revalidate(). Up to filesystem - VFS doesn't touch that field (and AFAICS only NFS uses it these days).
