| Message ID | 1424736897-95767-1-git-send-email-trond.myklebust@primarydata.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Feb 23, 2015 at 7:14 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: > If we're traversing a directory which contains a submounted filesystem, > or one that has a referral, the NFS server that is processing the READDIR > request will often return information for the underlying (mounted-on) > directory. It may, or may not, also return filehandle information. > > If this happens, and the lookup in nfs_prime_dcache() returns the > dentry for the submounted directory, the filehandle comparison will > fail, and we call d_invalidate(). Post-commit 8ed936b5671bf > ("vfs: Lazily remove mounts on unlinked files and directories."), this > means the entire subtree is unmounted. > > The following minimal patch addresses this problem by punting on > the invalidation if there is a submount. > > Kudos to Neil Brown <neilb@suse.de> for having tracked down this > issue (see link). > > Reported-by: Nix <nix@esperi.org.uk> > Link: http://lkml.kernel.org/r/87iofju9ht.fsf@spindle.srvr.nix > Cc: stable@vger.kernel.org # 3.18+ > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > --- > fs/nfs/dir.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c > index 9b0c55cb2a2e..0da617a61c0b 100644 > --- a/fs/nfs/dir.c > +++ b/fs/nfs/dir.c > @@ -479,6 +479,10 @@ void nfs_prime_dcache(struct dentry *parent, struct nfs_entry *entry) > > dentry = d_lookup(parent, &filename); > if (dentry != NULL) { > + /* Is there a mountpoint here? If so, just exit */ > + if (!nfs_fsid_equal(&NFS_SB(dentry->d_sb)->fsid, > + &entry->fattr->fsid)) > + goto out; > if (nfs_same_file(dentry, entry)) { > nfs_set_verifier(dentry, nfs_save_change_attribute(dir)); > status = nfs_refresh_inode(dentry->d_inode, entry->fattr); > -- ...and this of course needs the test for NFS_ATTR_FATTR_FSID from 3/3.... I've updated.
On Mon, 23 Feb 2015 19:14:55 -0500 Trond Myklebust <trond.myklebust@primarydata.com> wrote: > If we're traversing a directory which contains a submounted filesystem, > or one that has a referral, the NFS server that is processing the READDIR > request will often return information for the underlying (mounted-on) > directory. It may, or may not, also return filehandle information. > > If this happens, and the lookup in nfs_prime_dcache() returns the > dentry for the submounted directory, the filehandle comparison will > fail, and we call d_invalidate(). Post-commit 8ed936b5671bf > ("vfs: Lazily remove mounts on unlinked files and directories."), this > means the entire subtree is unmounted. > > The following minimal patch addresses this problem by punting on > the invalidation if there is a submount. > > Kudos to Neil Brown <neilb@suse.de> for having tracked down this > issue (see link). > > Reported-by: Nix <nix@esperi.org.uk> > Link: http://lkml.kernel.org/r/87iofju9ht.fsf@spindle.srvr.nix > Cc: stable@vger.kernel.org # 3.18+ > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > --- > fs/nfs/dir.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c > index 9b0c55cb2a2e..0da617a61c0b 100644 > --- a/fs/nfs/dir.c > +++ b/fs/nfs/dir.c > @@ -479,6 +479,10 @@ void nfs_prime_dcache(struct dentry *parent, struct nfs_entry *entry) > > dentry = d_lookup(parent, &filename); > if (dentry != NULL) { > + /* Is there a mountpoint here? If so, just exit */ > + if (!nfs_fsid_equal(&NFS_SB(dentry->d_sb)->fsid, > + &entry->fattr->fsid)) > + goto out; Surely we should only consider fattr->fsid if NFS_ATTR_FATTR_FSID is set. In the case of the Linux NFSv3 server, if this were a mountpoint on the server, then NFS_ATTR_FATTR_FSID will not be set (as the server returns neither postop attrs nor filehandle). I realise that this issue is addressed in the subsequent patch (3/3), but that isn't tagged for -stable, and this is. NeilBrown > if (nfs_same_file(dentry, entry)) { > nfs_set_verifier(dentry, nfs_save_change_attribute(dir)); > status = nfs_refresh_inode(dentry->d_inode, entry->fattr);
On Mon, 23 Feb 2015 22:09:09 -0500 Trond Myklebust <trond.myklebust@primarydata.com> wrote: > On Mon, Feb 23, 2015 at 7:14 PM, Trond Myklebust > <trond.myklebust@primarydata.com> wrote: > > If we're traversing a directory which contains a submounted filesystem, > > or one that has a referral, the NFS server that is processing the READDIR > > request will often return information for the underlying (mounted-on) > > directory. It may, or may not, also return filehandle information. > > > > If this happens, and the lookup in nfs_prime_dcache() returns the > > dentry for the submounted directory, the filehandle comparison will > > fail, and we call d_invalidate(). Post-commit 8ed936b5671bf > > ("vfs: Lazily remove mounts on unlinked files and directories."), this > > means the entire subtree is unmounted. > > > > The following minimal patch addresses this problem by punting on > > the invalidation if there is a submount. > > > > Kudos to Neil Brown <neilb@suse.de> for having tracked down this > > issue (see link). > > > > Reported-by: Nix <nix@esperi.org.uk> > > Link: http://lkml.kernel.org/r/87iofju9ht.fsf@spindle.srvr.nix > > Cc: stable@vger.kernel.org # 3.18+ > > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > > --- > > fs/nfs/dir.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c > > index 9b0c55cb2a2e..0da617a61c0b 100644 > > --- a/fs/nfs/dir.c > > +++ b/fs/nfs/dir.c > > @@ -479,6 +479,10 @@ void nfs_prime_dcache(struct dentry *parent, struct nfs_entry *entry) > > > > dentry = d_lookup(parent, &filename); > > if (dentry != NULL) { > > + /* Is there a mountpoint here? If so, just exit */ > > + if (!nfs_fsid_equal(&NFS_SB(dentry->d_sb)->fsid, > > + &entry->fattr->fsid)) > > + goto out; > > if (nfs_same_file(dentry, entry)) { > > nfs_set_verifier(dentry, nfs_save_change_attribute(dir)); > > status = nfs_refresh_inode(dentry->d_inode, entry->fattr); > > -- > > ...and this of course needs the test for NFS_ATTR_FATTR_FSID from > 3/3.... I've updated. > > > Sorry ... I didn't see this before my earlier reply... What exactly do you do if NFS_ATTR_FATTR_FSID isn't set? Hopefully you "goto out". Thanks, NeilBrown
On Tue, Feb 24, 2015 at 4:49 PM, NeilBrown <neilb@suse.de> wrote: > On Mon, 23 Feb 2015 22:09:09 -0500 Trond Myklebust > <trond.myklebust@primarydata.com> wrote: > >> On Mon, Feb 23, 2015 at 7:14 PM, Trond Myklebust >> <trond.myklebust@primarydata.com> wrote: >> > If we're traversing a directory which contains a submounted filesystem, >> > or one that has a referral, the NFS server that is processing the READDIR >> > request will often return information for the underlying (mounted-on) >> > directory. It may, or may not, also return filehandle information. >> > >> > If this happens, and the lookup in nfs_prime_dcache() returns the >> > dentry for the submounted directory, the filehandle comparison will >> > fail, and we call d_invalidate(). Post-commit 8ed936b5671bf >> > ("vfs: Lazily remove mounts on unlinked files and directories."), this >> > means the entire subtree is unmounted. >> > >> > The following minimal patch addresses this problem by punting on >> > the invalidation if there is a submount. >> > >> > Kudos to Neil Brown <neilb@suse.de> for having tracked down this >> > issue (see link). >> > >> > Reported-by: Nix <nix@esperi.org.uk> >> > Link: http://lkml.kernel.org/r/87iofju9ht.fsf@spindle.srvr.nix >> > Cc: stable@vger.kernel.org # 3.18+ >> > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> >> > --- >> > fs/nfs/dir.c | 4 ++++ >> > 1 file changed, 4 insertions(+) >> > >> > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c >> > index 9b0c55cb2a2e..0da617a61c0b 100644 >> > --- a/fs/nfs/dir.c >> > +++ b/fs/nfs/dir.c >> > @@ -479,6 +479,10 @@ void nfs_prime_dcache(struct dentry *parent, struct nfs_entry *entry) >> > >> > dentry = d_lookup(parent, &filename); >> > if (dentry != NULL) { >> > + /* Is there a mountpoint here? If so, just exit */ >> > + if (!nfs_fsid_equal(&NFS_SB(dentry->d_sb)->fsid, >> > + &entry->fattr->fsid)) >> > + goto out; >> > if (nfs_same_file(dentry, entry)) { >> > nfs_set_verifier(dentry, nfs_save_change_attribute(dir)); >> > status = nfs_refresh_inode(dentry->d_inode, entry->fattr); >> > -- >> >> ...and this of course needs the test for NFS_ATTR_FATTR_FSID from >> 3/3.... I've updated. >> >> >> > > Sorry ... I didn't see this before my earlier reply... > > What exactly do you do if NFS_ATTR_FATTR_FSID isn't set? > Hopefully you "goto out". > Yes. The test is made immediately upon entering the function, and so a simple 'return' is sufficient.
On 24 Feb 2015, Trond Myklebust stated: > If we're traversing a directory which contains a submounted filesystem, > or one that has a referral, the NFS server that is processing the READDIR > request will often return information for the underlying (mounted-on) > directory. It may, or may not, also return filehandle information. FWIW, this never seems to have made it into 3.19.x, nor is it in the stable queue. Was this intentional? > Cc: stable@vger.kernel.org # 3.18+ (... hm, I guess so...)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index 9b0c55cb2a2e..0da617a61c0b 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -479,6 +479,10 @@ void nfs_prime_dcache(struct dentry *parent, struct nfs_entry *entry) dentry = d_lookup(parent, &filename); if (dentry != NULL) { + /* Is there a mountpoint here? If so, just exit */ + if (!nfs_fsid_equal(&NFS_SB(dentry->d_sb)->fsid, + &entry->fattr->fsid)) + goto out; if (nfs_same_file(dentry, entry)) { nfs_set_verifier(dentry, nfs_save_change_attribute(dir)); status = nfs_refresh_inode(dentry->d_inode, entry->fattr);
If we're traversing a directory which contains a submounted filesystem, or one that has a referral, the NFS server that is processing the READDIR request will often return information for the underlying (mounted-on) directory. It may, or may not, also return filehandle information. If this happens, and the lookup in nfs_prime_dcache() returns the dentry for the submounted directory, the filehandle comparison will fail, and we call d_invalidate(). Post-commit 8ed936b5671bf ("vfs: Lazily remove mounts on unlinked files and directories."), this means the entire subtree is unmounted. The following minimal patch addresses this problem by punting on the invalidation if there is a submount. Kudos to Neil Brown <neilb@suse.de> for having tracked down this issue (see link). Reported-by: Nix <nix@esperi.org.uk> Link: http://lkml.kernel.org/r/87iofju9ht.fsf@spindle.srvr.nix Cc: stable@vger.kernel.org # 3.18+ Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/dir.c | 4 ++++ 1 file changed, 4 insertions(+)