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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
