"J. Bruce Fields" <bfields@xxxxxxxxxxxx> writes: > On Fri, Feb 14, 2014 at 07:49:35AM -0800, Eric W. Biederman wrote: >> >> I believe we want both groups of dentries on the s_anon list so that if >> they remain disconnected when the filesystem is unmounted we can >> find them and deal with them. > > Note it's IS_ROOT(), not DCACHE_DISCONNECTED, that determines whether a > hashed dentry is on s_anon or not. (See > 7632e465feb182cadc3c9aa1282a057201818a8c for more detailed > discussion.) Interesting. It remains the case that d_obtain_alias that sets DCACHE_DISCONNECTED is the only way to get on the s_anon list. >From the rest of the conversation below I think what is needed is d_obtain_alias_root. A function like d_obtain_alias but that does not set DCACHE_DISCONNECTED, that places IS_ROOT dentries on the s_anon list and that is usually expected to not find an alias. >> I can see distinguishing between dentries that are supposed to be >> disconnected for a short time, and dentries that are supposed to be >> disconnected indefinitely but we currently (as of 3.14-rc1) don't have >> that distinction. > > I believe we do: DCACHE_DISCONNECTED is for the former, and the latter > should be IS_ROOT(), !DCACHE_DISCONNECTED. > > DCACHE_DISCONNECTED was intended to be used only for dentries created > while performing lookup-by-filehandle which have not yet been confirmed > to be linked back to filesystem root by a chain of ->d_parent > pointers. Given the current usages that was not at all clear from reading through the code. So I suggest d_obtain_alias_root to sort the last of it out. >> But a blanket statement that the long term disconnected dentries are >> doing it wrong seems off base. >> >> If those dentires can tolerate not being on the s_anon list >> d_alloc_pseudo or d_make_root looks like it will serve just as well > > The difference is that d_alloc_pseudo and d_make_root unconditionally > create new dentries, whereas d_materialise_unique lets us reuse an > existing dentry. Aliasing differences aside when dentries are allocated my point is that d_materialise_uniuqe does not care afterwards if DCACHE_DISCONNECTED is set or not, and d_materialise_unique will perform the connection if that is possible later on. > (In the NFS client case, I believe this happens for example when you > mount export A from a server, then export B from the same server, and > then one day you look up A/foo and find out that it's the same directory > as B's root. You don't want to duplicate the inode or give it multiple > dentries, so you instead reuse the existing IS_ROOT dentry for B to > represent A/foo. > > In the btrfs case I guess it's a similar situation but with two > subvolumes instead of two exports?) That is what it sounds like. And d_materialise_unique will connect them in either case. So we just need d_obtain_alias_root to allocate these weird oddball dentries and we should be good. I suspect the code would be much clearer if we were to do a mass s/DCACHE_DISCONNECTED/DCACHE_CONNECTING/ to make it clear that the flag is not intended to cover the general case of when dentries are floating around without parents. >> from >> the perspective of d_materialise_unique, but that leaves me with the >> queasy feeling that we will leak dentries and inodes when we unmount the >> filesystems in question, if those dentries have never been attached. > > In the NFS server (lookup-by-filehandle) case, I believe dentries in the > process of being reconnected are all children of some IS_ROOT dentry > which is on the s_anon list, so everyone's accounted for. My concern there is for all of the other cases. Because so far today only DCACHE_DISCONNECTED dentries land on the s_anon list and that means if we don't use d_obtain_alias and we can have dentry leaks and unmount. > Thanks for looking at this as I've found myself easily confused by it > all.... (And judging by some of the code in the tree I'm not alone.) I am in the final stages of putting together some patches so that filesystems don't need to like to the vfs to preserve vfs invariants about mountpoints. In particular I am implemeting automatic detaching of mountpionts on unlink and d_invalidate. Once that is done and everyone is dropping directory dentries instead of sticking to the silly 2.2 era logic that present resides in d_invalidate some of these other dcache uses should become a little less mysterious. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
