On Tue, Apr 02, 2019 at 05:48:24PM +0100, Al Viro wrote: > 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. Are you able to extrapolate on this comment please? Is this something someone new to the VFS (me) can do with a little nudge in the right direction or is this something that needs thorough knowledge of the VFS? > 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. This patch includes documentation for d_prune() very similar to Jeff's patch (taken from a comment somewhere in filesystems code IIRC). I can have a go at working all the comments below into better documentation if that is useful (dependant on answer to question above). thanks, Tobin. (leaving text below for reference) > 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).
