On Thu, Dec 01, 2011 at 12:49:22PM +1100, NeilBrown wrote:
> If the path was "/some/long/path/.", then the final component ("path" in
> this case) has already been revalidated and there is no particular
> need to do it again.
>
> If we change nd->last_type to refer to "the last component looked at"
> rather than just "the last component", then these cases can be
> detected by "nd->last_type != LAST_NORM".
This is just plain wrong. Let's *not* bring more dependencies on
nameidata into ->d_revalidate(). The goal is to get rid of it there...
FWIW, if you want a really nasty bug in that area, consider this:
mkdir /tmp/a
mkdir /tmp/b
echo "local file" >/tmp/x
mount -t nfs4 $SOMETHING /tmp/a
mount -t nfs4 $SOMETHING /tmp/b
echo "NFS file" >/tmp/a/x
mount --bind /tmp/x /tmp/a/x
now try opening /tmp/b/x. And watch the NFS traffic; there won't be OPEN
request for x on server. Why? Because NFS sees that x is a mountpoint in
*some* instance of that filesystem. And decides that opening it would be
wrong. And so it would, if we were asked to open /tmp/a/x. Alas, in this
case, while dentry is the same, it does *not* have anything mounted on it.
What we get is ->d_revalidate() returning without issuing OPEN and ->open()
being called - again, without issuing OPEN, since it assumes that ->lookup()
or ->d_revalidate() had done it for us.
Plain IO on resulting descriptor will work and work correcly (you'll get
"NFS file\n" read from it), but try to do F_SETLK on it and it'll fail
since that requires the server to have seen an OPEN.
As far as I can tell, the idea of open done in ->d_revalidate() is
unsalvagable. It's simply the wrong place for that. Note that NFS
is the only filesystem trying to do atomic open stuff in its ->d_revalidate()
and it's not succeeding.
--
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
