linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "J. Bruce Fields" <bfields@fieldses.org>
To: "J. Bruce Fields" <bfields@redhat.com>
Cc: Al Viro <viro@ZenIV.linux.org.uk>,
	Christoph Hellwig <hch@infradead.org>,
	linux-fsdevel@vger.kernel.org, linux-nfs@vger.kernel.org,
	sandeen@redhat.com
Subject: Re: [PATCH 2/2] exportfs: fix 32-bit nfsd handling of 64-bit inode numbers
Date: Fri, 4 Oct 2013 18:12:16 -0400	[thread overview]
Message-ID: <20131004221216.GC18051@fieldses.org> (raw)
In-Reply-To: <1380749295-20854-2-git-send-email-bfields@redhat.com>

On Wed, Oct 02, 2013 at 05:28:14PM -0400, J. Bruce Fields wrote:
> @@ -268,6 +268,16 @@ static int get_name(const struct path *path, char *name, struct dentry *child)
>  	if (!dir->i_fop)
>  		goto out;
>  	/*
> +	 * inode->i_ino is unsigned long, kstat->ino is u64, so the
> +	 * former would be insufficient on 32-bit hosts when the
> +	 * filesystem supports 64-bit inode numbers.  So we need to
> +	 * actually call ->getattr, not just read i_ino:
> +	 */
> +	error = vfs_getattr_nosec(path, &stat);

Doh, "path" here is for the parent....  The following works better!

--b.

commit 231d38b6f775c4677a61b4d7dc15e0a0d6bbb709
Author: J. Bruce Fields <bfields@redhat.com>
Date:   Tue Sep 10 11:41:12 2013 -0400

    exportfs: fix 32-bit nfsd handling of 64-bit inode numbers
    
    Symptoms were spurious -ENOENTs on stat of an NFS filesystem from a
    32-bit NFS server exporting a very large XFS filesystem, when the
    server's cache is cold (so the inodes in question are not in cache).
    
    Reviewed-by: Christoph Hellwig <hch@lst.de>
    Reported-by: Trevor Cordes <trevor@tecnopolis.ca>
    Signed-off-by: J. Bruce Fields <bfields@redhat.com>

diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
index a235f00..c43fe9b 100644
--- a/fs/exportfs/expfs.c
+++ b/fs/exportfs/expfs.c
@@ -215,7 +215,7 @@ struct getdents_callback {
 	struct dir_context ctx;
 	char *name;		/* name that was found. It already points to a
 				   buffer NAME_MAX+1 is size */
-	unsigned long ino;	/* the inum we are looking for */
+	u64 ino;		/* the inum we are looking for */
 	int found;		/* inode matched? */
 	int sequence;		/* sequence counter */
 };
@@ -255,10 +255,14 @@ static int get_name(const struct path *path, char *name, struct dentry *child)
 	struct inode *dir = path->dentry->d_inode;
 	int error;
 	struct file *file;
+	struct kstat stat;
+	struct path child_path = {
+		.mnt = path->mnt,
+		.dentry = child,
+	};
 	struct getdents_callback buffer = {
 		.ctx.actor = filldir_one,
 		.name = name,
-		.ino = child->d_inode->i_ino
 	};
 
 	error = -ENOTDIR;
@@ -268,6 +272,16 @@ static int get_name(const struct path *path, char *name, struct dentry *child)
 	if (!dir->i_fop)
 		goto out;
 	/*
+	 * inode->i_ino is unsigned long, kstat->ino is u64, so the
+	 * former would be insufficient on 32-bit hosts when the
+	 * filesystem supports 64-bit inode numbers.  So we need to
+	 * actually call ->getattr, not just read i_ino:
+	 */
+	error = vfs_getattr_nosec(&child_path, &stat);
+	if (error)
+		return error;
+	buffer.ino = stat.ino;
+	/*
 	 * Open the directory ...
 	 */
 	file = dentry_open(path, O_RDONLY, cred);

  reply	other threads:[~2013-10-04 22:12 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-12 16:03 why is i_ino unsigned long, anyway? J. Bruce Fields
2013-09-12 19:33 ` Al Viro
2013-09-29 11:54   ` Christoph Hellwig
2013-10-02 14:25     ` J. Bruce Fields
2013-10-02 15:43       ` J. Bruce Fields
2013-10-02 16:04         ` Christoph Hellwig
2013-10-02 18:14           ` J. Bruce Fields
2013-10-02 16:05       ` Christoph Hellwig
2013-10-02 17:53         ` J. Bruce Fields
2013-10-02 17:57           ` Christoph Hellwig
2013-10-02 21:07             ` J. Bruce Fields
2013-10-02 21:28               ` [PATCH 1/2] vfs: split out vfs_getattr_nosec J. Bruce Fields
2013-10-02 21:28                 ` [PATCH 2/2] exportfs: fix 32-bit nfsd handling of 64-bit inode numbers J. Bruce Fields
2013-10-04 22:12                   ` J. Bruce Fields [this message]
2013-10-04 22:15                     ` J. Bruce Fields
2013-10-08 21:56                       ` J. Bruce Fields
2013-10-09  0:16                         ` Dave Chinner
2013-10-09 14:53                           ` J. Bruce Fields
2013-10-10 22:28                             ` Dave Chinner
2013-10-11 21:53                               ` J. Bruce Fields
2013-10-13 22:52                                 ` Dave Chinner
2013-10-02 18:47           ` why is i_ino unsigned long, anyway? Sage Weil
2013-10-02 19:00             ` J. Bruce Fields
2013-10-02 19:04               ` Sage Weil

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=20131004221216.GC18051@fieldses.org \
    --to=bfields@fieldses.org \
    --cc=bfields@redhat.com \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=sandeen@redhat.com \
    --cc=viro@ZenIV.linux.org.uk \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).