Re: [PATCH] fs/dcache: allow __d_obtain_alias() to return unhashed dentries

On Tue, Mar 08, 2011 at 01:13:20PM -0500, J. Bruce Fields wrote:

Al, do you have this in your queue to look at?  Need me to resend?  Or
should it take some other route?

It's in queue, but I'd be a lot happier if I understood what's going on
with __d_find_alias() elsewhere.  Namely, in d_splice_alias().  The thing
is, unless I'm missing something we ought to use __d_find_any_alias()
there as well.  Directories really, _really_ should not have more than
one alias.  And what we get is really weird:
	* find (the only) alias
	* if it doesn't exist, create one (OK, no problem)
	* if it does exist and happens to be IS_ROOT and DCACHE_DISCONNECTED,
move it (also fine, modulo rather useless BUG_ON() in there)
	* if it does exist and either isn't IS_ROOT or not DCACHE_DISCONNECTED,
add a new alias and say nothing.

The last part looks very strange.  I'd been staring at the history of this
function and I _think_ it's a rudiment of period when we used that stuff for
non-directories as well.  It used to be directory-only; then Neil had
switched it to non-directories as well (in "Fix disconnected dentries on NFS
exports" back in 2004), adding want_discon argument to __d_find_alias() in
process and using it instead of open-coded equivalent of __d_find_any_alias().
Then, two years later, in "knfsd: close a race-opportunity in d_splice_alias"
he'd made that code directory-only again, at which point we arrived to the
current situation.

Neil, is there some reason I'm missing that makes __d_find_alias() right in
there?  And do we still need the second argument in __d_find_alias()?

Anyway, Bruce's patch goes in tonight's push, but I'd love to see that
mess cleaned up as well or at least explained.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html