On Wed, Jan 15, 2014 at 6:57 PM, J. Bruce Fields <bfields@xxxxxxxxxxxx> wrote: > On Wed, Jan 15, 2014 at 06:34:56PM +0100, Miklos Szeredi wrote: >> On Wed, 2014-01-15 at 10:17 -0500, J. Bruce Fields wrote: >> > From: "J. Bruce Fields" <bfields@xxxxxxxxxx> >> > >> > d_splice_alias can create duplicate directory aliases (in the !new >> > case), or (in the new case) d_move without holding appropriate locks. >> >> It can d_move, because the dentry is known to be disconnected, i.e. it >> doesn't have a parent for which we could obtain the lock. > > DCACHE_DISCONNECTED doesn't mean that. You're right, but I'm also right, because __d_find_alias() will check IS_ROOT() too. So only "root" disconnected dentries will be moved. > > When you lookup a dentry by filehandle that dentry is initially marked > DCACHE_DISCONNECTED. It is cleared only after reconnect_path() has > verified that the dentry is reachable all the way from the root. > > So !DCACHE_DISCONNECTED implies that the dentry is connected all the way > up to the root, but the converse is not true. > > This has been a source of confusion, but it is explained in > Documentation/filesystems/nfs/Exporting. Recently I've cleaned up a few > odd uses of DCACHE_DISCONNECTED and rewritten reconnect_path(), partly > as an attempt to clarify the situation. > > Let me know if any of that doesn't look right to you.... > >> > d_materialise_unique deals with both of these problems. (The latter >> > seems to be dealt by trylocks (see __d_unalias), which look like they >> > could cause spurious lookup failures--but that's at least better tha >> > corrupting the dcache.) >> > >> > Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxx> >> > --- >> > fs/dcache.c | 25 +------------------------ >> > 1 file changed, 1 insertion(+), 24 deletions(-) >> > >> > Only lightly tested.... If this is right, then we can also just ditch >> > d_splice_alias completely, and clean up the various d_find_alias's. >> > >> > I think the only reason we have both d_splice_alias and >> > d_materialise_unique is that the former was written for exportable >> > filesystems and the latter for distributed filesystems. >> > >> > But we have at least one exportable filesystem (fuse) using >> > d_materialise_unique. And I doubt d_splice_alias was ever completely >> > correct even for on-disk filesystems. >> > >> > Am I missing some subtlety? >> >> One subtle difference is that for a non-directory d_splice_alias() will >> reconnect a DCACHE_DISCONNECTED dentry if one exists, while >> d_materialise_unique() will not. > > Actually until f80de2cde10350b8d146e375ff8b634e72e6a827 "dcache: don't > clear DCACHE_DISCONNECTED too early", it was the reverse: > d_materialise_unique cleared DISCONNECTED and d_splice_alias (correctly) > did not. > > The only place where it should be cleared is reconnect_path(). > >> Does this matter in practice? The small number of extra dentries >> probably does not matter. > > Directories are assumed to have unique aliases. When they don't, the > kernel can deadlock or crash. What I meant is that d_materialise_unique() will currently not reuse disconnected *nondirectory* dentries, hence there may be more aliases than necessary. This could easily be fixed, though. Thanks, Miklos > > --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
