On Mon, Mar 12, 2018 at 07:13:51PM +0000, Al Viro wrote:
> There's an unpleasant area around the ->s_root vs. NFS. There's
> code that makes assumptions about ->s_root that are simply not true
> for NFS. Is path_connected() correct wrt NFS multiple imports from
> the same server? Ditto for mnt_already_visible() (that one might
> be mitigated at the moment, but probably won't last). Eric, am
> I missing something subtle in there?
BTW, another thing spotted while trying to document the stuff:
Neil's "VFS: don't keep disconnected dentries on d_anon" has a subtle
side effect I hadn't spotted when applying. Namely, for
DCACHE_DISCONNECTED non-directory IS_ROOT dentries we now have
d_unhashed() true. That makes d_find_alias() logics around
discon_alias dead code:
if (S_ISDIR(inode->i_mode) || !d_unhashed(alias)) {
if (IS_ROOT(alias) &&
(alias->d_flags & DCACHE_DISCONNECTED)) {
discon_alias = alias;
} else {
__dget_dlock(alias);
spin_unlock(&alias->d_lock);
return alias;
}
}
Directory case is a red herring - we'll end up picking one and only
alias, whether it's IS_ROOT/disconnected or not. For non-directories,
though, IS_ROOT and disconnected implies d_unhashed() now, so we might
as well turn that into
if directory
return d_find_any_alias
else
return the first hashed alias
Does any of the d_find_alias() callers want those dentries? That is,
non-directories from d_obtain_alias() still not attached to a parent;
note that exportfs_decode_fh() is *not* one of such places - we don't
use d_find_alias() there at all. If there's no such caller, we can
bloody well just drop the discon_alias logics and be done with that;
if there is, that commit has introduced a bug. I might have missed
a part of threads related to that patch, so my apologies if it had
been discussed.
Neil, what's the situation there? A lot of those callers clearly treat
the "only disconnected IS_ROOT alias exist" same as "no aliases at all";
it looks like the change might have been the right thing, but it sure
as hell shouldn't be an undocumented side effect...
