All of lore.kernel.org
 help / color / mirror / Atom feed
From: Trond Myklebust <Trond.Myklebust@netapp.com>
To: Simon Kirby <sim@hostway.ca>
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>,
	linux-nfs@vger.kernel.org,
	"J. Bruce Fields" <bfields@fieldses.org>,
	Neil Brown <neilb@suse.de>, Bryan Schumaker <bjschuma@netapp.com>,
	rees@umich.edu
Subject: Re: [REGRESSION] git commit d1bacf9e "NFS: add readdir cache array" is bad
Date: Sat, 27 Nov 2010 13:24:00 -0500	[thread overview]
Message-ID: <1290882240.3415.5.camel@heimdal.trondhjem.org> (raw)
In-Reply-To: <20101127102732.GD12175@hostway.ca>

On Sat, 2010-11-27 at 02:27 -0800, Simon Kirby wrote:
> Ok, so I tracked them down, and they didn't seem to be particularly
> unusual, so I tried a not-particularly-unusual thing that I figured might
> work, and reproduced it:
> 
> server: echo test > a
> client: ls -l
> server: echo test > b ; mv b a
> client: ls -l
> 
> That's it.  The kernel (2.6.37-rc3), on the final "ls -l", says:
> 
> [12814.611197] NFS: server 10.10.52.228 error: fileid changed
> [12814.611200] fsid 0:3f: expected fileid 0x122efbf1, got 0x122efc15
> 
> "ls -li" shows the inode updated, so maybe this isn't even a bug?

Ah! I think I see it now, and yes it is a bug...

Does the following patch fix it?

Cheers
  Trond
-------------------------------------------------------------------------------------
NFS: Fix a readdirplus bug
From: Trond Myklebust <Trond.Myklebust@netapp.com>

When comparing filehandles in the helper nfs_same_file(), we should not be
using 'strncmp()': filehandles are not null terminated strings.

Instead, we should just use the existing helper nfs_compare_fh().

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 fs/nfs/dir.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)


diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 8ea4a41..f0a384e 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -395,13 +395,9 @@ int xdr_decode(nfs_readdir_descriptor_t *desc, struct nfs_entry *entry, struct x
 static
 int nfs_same_file(struct dentry *dentry, struct nfs_entry *entry)
 {
-	struct nfs_inode *node;
 	if (dentry->d_inode == NULL)
 		goto different;
-	node = NFS_I(dentry->d_inode);
-	if (node->fh.size != entry->fh->size)
-		goto different;
-	if (strncmp(node->fh.data, entry->fh->data, node->fh.size) != 0)
+	if (nfs_compare_fh(entry->fh, NFS_FH(dentry->d_inode)) != 0)
 		goto different;
 	return 1;
 different:


-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com


  reply	other threads:[~2010-11-27 18:24 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-26 12:05 [REGRESSION] git commit d1bacf9e "NFS: add readdir cache array" is bad Guennadi Liakhovetski
2010-11-26 18:05 ` Trond Myklebust
2010-11-26 18:34   ` Guennadi Liakhovetski
2010-11-27  1:41     ` Simon Kirby
2010-11-27  0:25   ` Simon Kirby
2010-11-27 10:27     ` Simon Kirby
2010-11-27 18:24       ` Trond Myklebust [this message]
2010-11-30  8:30         ` Simon Kirby

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1290882240.3415.5.camel@heimdal.trondhjem.org \
    --to=trond.myklebust@netapp.com \
    --cc=bfields@fieldses.org \
    --cc=bjschuma@netapp.com \
    --cc=g.liakhovetski@gmx.de \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=rees@umich.edu \
    --cc=sim@hostway.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.