What's in the NFS queue for 2.6.27

PATCH In review Trond Myklebust 4 participants 42 messages Oct 7, 2008 – Oct 15, 2008 2 messages not in the archive

Subsystems (3)

Networking NFS client NFS server

lore

Share & tools

lore

Trond Myklebust

author

2008-10-07 22:32 UTC #0 lore raw

cover What's in the NFS queue for 2.6.27

Most of the following patches are bugfixes:
There are a couple of spin locking fixes to follow up the BKL removal. There are some attribute update fixups, mainly to make the spin locking more efficient
A fix for the problem of autobind on cloned RPC clients

The one feature that has been added is the lookup revalidation mount option to allow clients to specify that they might not want to cache negative dentries, and that they might want strict dentry revalidation.

Signature · 6 lines

Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
www.netapp.com

A message that is not in the archive (sent off-list or not yet received): 20081007221952.20945.69529.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/[email protected]

Trond Myklebust

author

2008-10-07 22:32 UTC #2 lore raw

PATCH 1/30 NFS: Fix nfs_file_llseek()

After the BKL removal patches were applied to the rest of the NFS code, the BKL protection in nfs_file_llseek() is no longer sufficient to ensure that inode->i_size is read safely in generic_file_llseek_unlocked().

In order to fix the situation, we either have to replace the naked read of inode->i_size in generic_file_llseek_unlocked() with i_size_read(), or the whole thing needs to be executed under the inode->i_lock;
In order to avoid disrupting other filesystems, avoid touching generic_file_llseek_unlocked() for now...

Signed-off-by Trond Myklebust <[email protected]>

---

 fs/nfs/file.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

fs/nfs/file.c +7 -4

index 7846065..3ddb004 100644
@@ -188,13 +188,16 @@ static loff_t nfs_file_llseek(struct file *filp, loff_t offset, int origin)
 	/* origin == SEEK_END => we must revalidate the cached file length */
 	if (origin == SEEK_END) {
 		struct inode *inode = filp->f_mapping->host;
+
 		int retval = nfs_revalidate_file_size(inode, filp);
 		if (retval < 0)
 			return (loff_t)retval;
-	}
-	lock_kernel();	/* BKL needed? */
-	loff = generic_file_llseek_unlocked(filp, offset, origin);
-	unlock_kernel();
+
+		spin_lock(&inode->i_lock);
+		loff = generic_file_llseek_unlocked(filp, offset, origin);
+		spin_unlock(&inode->i_lock);
+	} else
+		loff = generic_file_llseek_unlocked(filp, offset, origin);
 	return loff;
 }

Trond Myklebust

author

2008-10-07 22:32 UTC #3 lore raw

PATCH 2/30 NFS: Clean up nfs_sb_active/nfs_sb_deactive

In reply to a message not in the archive

Instead of causing umount requests to block on server->active_wq while the asynchronous sillyrename deletes are executing, we can use the sb->s_active counter to obtain a reference to the super_block, and then release that reference in nfs_async_unlink_release().

Signed-off-by Trond Myklebust <[email protected]>

---

 fs/nfs/client.c           |    1 -
 fs/nfs/internal.h         |    4 ++--
 fs/nfs/super.c            |   24 ++++++++----------------
 fs/nfs/unlink.c           |    5 +++--
 include/linux/nfs_fs_sb.h |    1 -
 5 files changed, 13 insertions(+), 22 deletions(-)

fs/nfs/client.c +0 -1

index 5ee23e7..2accb67 100644
@@ -850,7 +850,6 @@ static struct nfs_server *nfs_alloc_server(void)
 	INIT_LIST_HEAD(&server->client_link);
 	INIT_LIST_HEAD(&server->master_link);
 
-	init_waitqueue_head(&server->active_wq);
 	atomic_set(&server->active, 0);
 
 	server->io_stats = nfs_alloc_iostats();

fs/nfs/internal.h +2 -2

index 24241fc..7bcf6ec 100644
@@ -163,8 +163,8 @@ extern struct rpc_stat nfs_rpcstat;
 
 extern int __init register_nfs_fs(void);
 extern void __exit unregister_nfs_fs(void);
-extern void nfs_sb_active(struct nfs_server *server);
-extern void nfs_sb_deactive(struct nfs_server *server);
+extern void nfs_sb_active(struct super_block *sb);
+extern void nfs_sb_deactive(struct super_block *sb);
 
 /* namespace.c */
 extern char *nfs_path(const char *base,

fs/nfs/super.c +8 ~~-16~~

index e9b2017..e527fab 100644
@@ -209,7 +209,6 @@ static int nfs_get_sb(struct file_system_type *, int, const char *, void *, stru
 static int nfs_xdev_get_sb(struct file_system_type *fs_type,
 		int flags, const char *dev_name, void *raw_data, struct vfsmount *mnt);
 static void nfs_kill_super(struct super_block *);
-static void nfs_put_super(struct super_block *);
 static int nfs_remount(struct super_block *sb, int *flags, char *raw_data);
 
 static struct file_system_type nfs_fs_type = {

@@ -232,7 +231,6 @@ static const struct super_operations nfs_sops = {
 	.alloc_inode	= nfs_alloc_inode,
 	.destroy_inode	= nfs_destroy_inode,
 	.write_inode	= nfs_write_inode,
-	.put_super	= nfs_put_super,
 	.statfs		= nfs_statfs,
 	.clear_inode	= nfs_clear_inode,
 	.umount_begin	= nfs_umount_begin,

@@ -337,26 +335,20 @@ void __exit unregister_nfs_fs(void)
 	unregister_filesystem(&nfs_fs_type);
 }
 
-void nfs_sb_active(struct nfs_server *server)
+void nfs_sb_active(struct super_block *sb)
 {
-	atomic_inc(&server->active);
-}
+	struct nfs_server *server = NFS_SB(sb);
 
-void nfs_sb_deactive(struct nfs_server *server)
-{
-	if (atomic_dec_and_test(&server->active))
-		wake_up(&server->active_wq);
+	if (atomic_inc_return(&server->active) == 1)
+		atomic_inc(&sb->s_active);
 }
 
-static void nfs_put_super(struct super_block *sb)
+void nfs_sb_deactive(struct super_block *sb)
 {
 	struct nfs_server *server = NFS_SB(sb);
-	/*
-	 * Make sure there are no outstanding ops to this server.
-	 * If so, wait for them to finish before allowing the
-	 * unmount to continue.
-	 */
-	wait_event(server->active_wq, atomic_read(&server->active) == 0);
+
+	if (atomic_dec_and_test(&server->active))
+		deactivate_super(sb);
 }
 
 /*

fs/nfs/unlink.c +3 -2

index f089e58..ecc2953 100644
@@ -99,7 +99,7 @@ static void nfs_async_unlink_release(void *calldata)
 
 	nfs_dec_sillycount(data->dir);
 	nfs_free_unlinkdata(data);
-	nfs_sb_deactive(NFS_SB(sb));
+	nfs_sb_deactive(sb);
 }
 
 static const struct rpc_call_ops nfs_unlink_ops = {

@@ -118,6 +118,7 @@ static int nfs_do_call_unlink(struct dentry *parent, struct inode *dir, struct n
 		.rpc_message = &msg,
 		.callback_ops = &nfs_unlink_ops,
 		.callback_data = data,
+		.workqueue = nfsiod_workqueue,
 		.flags = RPC_TASK_ASYNC,
 	};
 	struct rpc_task *task;

@@ -149,7 +150,7 @@ static int nfs_do_call_unlink(struct dentry *parent, struct inode *dir, struct n
 		nfs_dec_sillycount(dir);
 		return 0;
 	}
-	nfs_sb_active(NFS_SERVER(dir));
+	nfs_sb_active(dir->i_sb);
 	data->args.fh = NFS_FH(dir);
 	nfs_fattr_init(&data->res.dir_attr);

include/linux/nfs_fs_sb.h +0 -1

index c9beacd..4e477ae 100644
@@ -119,7 +119,6 @@ struct nfs_server {
 	void (*destroy)(struct nfs_server *);
 
 	atomic_t active; /* Keep trace of any activity to this server */
-	wait_queue_head_t active_wq;  /* Wait for any activity to stop  */
 
 	/* mountd-related mount options */
 	struct sockaddr_storage	mountd_address;

Trond Myklebust

author

2008-10-07 22:32 UTC #4 lore raw

PATCH 3/30 NFS: Add options for finer control of the lookup cache

In reply to a message not in the archive

Add the flag NFS_MOUNT_LOOKUP_CACHE_NONEG to turn off the caching of negative dentries. In reality what we do is to force nfs_lookup_revalidate() to always discard negative dentries.

Add the flag NFS_MOUNT_LOOKUP_CACHE_NONE for enforcing stricter revalidation of dentries. It forces the revalidate code to always do a lookup instead of just checking the cached mtime of the parent directory.

Signed-off-by Trond Myklebust <[email protected]>

---

 fs/nfs/dir.c              |    4 ++++
 include/linux/nfs_mount.h |    4 ++++
 2 files changed, 8 insertions(+), 0 deletions(-)

fs/nfs/dir.c +4 -0

index 74f92b7..49d5654 100644
@@ -667,6 +667,8 @@ static int nfs_check_verifier(struct inode *dir, struct dentry *dentry)
 {
 	if (IS_ROOT(dentry))
 		return 1;
+	if (NFS_SERVER(dir)->flags & NFS_MOUNT_LOOKUP_CACHE_NONE)
+		return 0;
 	if (!nfs_verify_change_attribute(dir, dentry->d_time))
 		return 0;
 	/* Revalidate nfsi->cache_change_attribute before we declare a match */

@@ -750,6 +752,8 @@ int nfs_neg_need_reval(struct inode *dir, struct dentry *dentry,
 	/* Don't revalidate a negative dentry if we're creating a new file */
 	if (nd != NULL && nfs_lookup_check_intent(nd, LOOKUP_CREATE) != 0)
 		return 0;
+	if (NFS_SERVER(dir)->flags & NFS_MOUNT_LOOKUP_CACHE_NONEG)
+		return 1;
 	return !nfs_check_verifier(dir, dentry);
 }

include/linux/nfs_mount.h +4 -0

index df7c6b7..6549a06 100644
@@ -65,4 +65,8 @@ struct nfs_mount_data {
 #define NFS_MOUNT_UNSHARED	0x8000	/* 5 */
 #define NFS_MOUNT_FLAGMASK	0xFFFF
 
+/* The following are for internal use only */
+#define NFS_MOUNT_LOOKUP_CACHE_NONEG	0x10000
+#define NFS_MOUNT_LOOKUP_CACHE_NONE	0x20000
+
 #endif

Trond Myklebust

author

2008-10-07 22:32 UTC #5 lore raw

PATCH 4/30 NFS: Don't apply NFS_MOUNT_FLAGMASK to text-based mounts

In reply to a message not in the archive

The point of introducing text-based mounts was to allow us to add functionality without having to worry about legacy binary mount formats. The mask should be there in order to ensure that binary formats don't start enabling features that they cannot support. There is no justification for applying it to the text mount path.

Signed-off-by Trond Myklebust <[email protected]>

---

 fs/nfs/client.c |    4 ++--
 fs/nfs/super.c  |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

fs/nfs/client.c +2 -2

index 2accb67..7547600 100644
@@ -675,7 +675,7 @@ static int nfs_init_server(struct nfs_server *server,
 	server->nfs_client = clp;
 
 	/* Initialise the client representation from the mount data */
-	server->flags = data->flags & NFS_MOUNT_FLAGMASK;
+	server->flags = data->flags;
 
 	if (data->rsize)
 		server->rsize = nfs_block_size(data->rsize, NULL);

@@ -1072,7 +1072,7 @@ static int nfs4_init_server(struct nfs_server *server,
 		goto error;
 
 	/* Initialise the client representation from the mount data */
-	server->flags = data->flags & NFS_MOUNT_FLAGMASK;
+	server->flags = data->flags;
 	server->caps |= NFS_CAP_ATOMIC_OPEN;
 
 	if (data->rsize)

fs/nfs/super.c +1 -1

index e527fab..81686ae 100644
@@ -1550,7 +1550,7 @@ static int nfs_validate_mount_data(void *options,
 		 * Translate to nfs_parsed_mount_data, which nfs_fill_super
 		 * can deal with.
 		 */
-		args->flags		= data->flags;
+		args->flags		= data->flags & NFS_MOUNT_FLAGMASK;
 		args->rsize		= data->rsize;
 		args->wsize		= data->wsize;
 		args->timeo		= data->timeo;

Trond Myklebust

author

2008-10-07 22:32 UTC #6 lore raw

PATCH 5/30 NFS: Add mount options for controlling the lookup cache

In reply to a message not in the archive

Add the following NFS-specific mount options to the parser.

-o lookupcache=all /* Default: cache positive & negative
dentries */
-o lookupcache=pos[itive] /* Don't cache negative dentries */
-o lookupcache=none /* Strict revalidation of all dentries */

Signed-off-by Trond Myklebust <[email protected]>

---

 fs/nfs/super.c |   43 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 43 insertions(+), 0 deletions(-)

fs/nfs/super.c +43 -0

index 81686ae..1e35586 100644
@@ -91,6 +91,7 @@ enum {
 	/* Mount options that take string arguments */
 	Opt_sec, Opt_proto, Opt_mountproto, Opt_mounthost,
 	Opt_addr, Opt_mountaddr, Opt_clientaddr,
+	Opt_lookupcache,
 
 	/* Special mount options */
 	Opt_userspace, Opt_deprecated, Opt_sloppy,

@@ -154,6 +155,8 @@ static match_table_t nfs_mount_option_tokens = {
 	{ Opt_mounthost, "mounthost=%s" },
 	{ Opt_mountaddr, "mountaddr=%s" },
 
+	{ Opt_lookupcache, "lookupcache=%s" },
+
 	{ Opt_err, NULL }
 };

@@ -200,6 +203,22 @@ static match_table_t nfs_secflavor_tokens = {
 	{ Opt_sec_err, NULL }
 };
 
+enum {
+	Opt_lookupcache_all, Opt_lookupcache_positive,
+	Opt_lookupcache_none,
+
+	Opt_lookupcache_err
+};
+
+static match_table_t nfs_lookupcache_tokens = {
+	{ Opt_lookupcache_all, "all" },
+	{ Opt_lookupcache_positive, "pos" },
+	{ Opt_lookupcache_positive, "positive" },
+	{ Opt_lookupcache_none, "none" },
+
+	{ Opt_lookupcache_err, NULL }
+};
+
 
 static void nfs_umount_begin(struct super_block *);
 static int  nfs_statfs(struct dentry *, struct kstatfs *);

@@ -1250,6 +1269,30 @@ static int nfs_parse_mount_options(char *raw,
 					     &mnt->mount_server.addrlen);
 			kfree(string);
 			break;
+		case Opt_lookupcache:
+			string = match_strdup(args);
+			if (string == NULL)
+				goto out_nomem;
+			token = match_token(string,
+					nfs_lookupcache_tokens, args);
+			kfree(string);
+			switch (token) {
+				case Opt_lookupcache_all:
+					mnt->flags &= ~(NFS_MOUNT_LOOKUP_CACHE_NONEG|NFS_MOUNT_LOOKUP_CACHE_NONE);
+					break;
+				case Opt_lookupcache_positive:
+					mnt->flags &= ~NFS_MOUNT_LOOKUP_CACHE_NONE;
+					mnt->flags |= NFS_MOUNT_LOOKUP_CACHE_NONEG;
+					break;
+				case Opt_lookupcache_none:
+					mnt->flags |= NFS_MOUNT_LOOKUP_CACHE_NONEG|NFS_MOUNT_LOOKUP_CACHE_NONE;
+					break;
+				default:
+					errors++;
+					dfprintk(MOUNT, "NFS:   invalid "
+							"lookupcache argument\n");
+			};
+			break;
 
 		/*
 		 * Special options

Trond Myklebust

author

2008-10-07 22:32 UTC #7 lore raw

PATCH 6/30 NFS: Clean up nfs_refresh_inode() and nfs_post_op_update_inode()

In reply to a message not in the archive

Try to avoid taking and dropping the inode->i_lock more than once. Do so by moving the code in nfs_refresh_inode() that needs to be done under the spinlock into a function nfs_refresh_inode_locked(), and then having both nfs_refresh_inode() and nfs_post_op_update_inode() call it directly.

Signed-off-by Trond Myklebust <[email protected]>

---

 fs/nfs/inode.c |   21 ++++++++++++++-------
 1 files changed, 14 insertions(+), 7 deletions(-)

fs/nfs/inode.c +14 -7

index 52daefa..f189169 100644
@@ -948,6 +948,15 @@ static int nfs_check_inode_attributes(struct inode *inode, struct nfs_fattr *fat
 	return 0;
 }
 
+static int nfs_refresh_inode_locked(struct inode *inode, struct nfs_fattr *fattr)
+{
+	struct nfs_inode *nfsi = NFS_I(inode);
+
+	if (time_after(fattr->time_start, nfsi->last_updated))
+		return nfs_update_inode(inode, fattr);
+	return nfs_check_inode_attributes(inode, fattr);
+}
+
 /**
  * nfs_refresh_inode - try to update the inode attribute cache
  * @inode - pointer to inode

@@ -960,17 +969,12 @@ static int nfs_check_inode_attributes(struct inode *inode, struct nfs_fattr *fat
  */
 int nfs_refresh_inode(struct inode *inode, struct nfs_fattr *fattr)
 {
-	struct nfs_inode *nfsi = NFS_I(inode);
 	int status;
 
 	if ((fattr->valid & NFS_ATTR_FATTR) == 0)
 		return 0;
 	spin_lock(&inode->i_lock);
-	if (time_after(fattr->time_start, nfsi->last_updated))
-		status = nfs_update_inode(inode, fattr);
-	else
-		status = nfs_check_inode_attributes(inode, fattr);
-
+	status = nfs_refresh_inode_locked(inode, fattr);
 	spin_unlock(&inode->i_lock);
 	return status;
 }

@@ -992,13 +996,16 @@ int nfs_refresh_inode(struct inode *inode, struct nfs_fattr *fattr)
 int nfs_post_op_update_inode(struct inode *inode, struct nfs_fattr *fattr)
 {
 	struct nfs_inode *nfsi = NFS_I(inode);
+	int status = 0;
 
 	spin_lock(&inode->i_lock);
 	nfsi->cache_validity |= NFS_INO_INVALID_ATTR|NFS_INO_REVAL_PAGECACHE;
 	if (S_ISDIR(inode->i_mode))
 		nfsi->cache_validity |= NFS_INO_INVALID_DATA;
+	if ((fattr->valid & NFS_ATTR_FATTR) != 0)
+		status = nfs_refresh_inode_locked(inode, fattr);
 	spin_unlock(&inode->i_lock);
-	return nfs_refresh_inode(inode, fattr);
+	return status;
 }
 
 /**

Trond Myklebust

author

2008-10-07 22:32 UTC #8 lore raw

PATCH 7/30 NFS: Fix the NFS attribute update

In reply to a message not in the archive

Currently nfs_refresh_inode() will only update the inode metadata if it sees that the RPC call that returned the nfs_fattr was started after the last update of the inode. This means that if we have parallel RPC calls to the same inode (when sending WRITE calls, for instance), we may often miss updates.

This patch attempts to recover those missed updates by also accepting them if the ctime in the nfs_fattr is more recent than the inode's cached ctime.
It also recovers the case where the file size has increased, but the ctime has not been updated due to limited ctime resolution.

Signed-off-by Trond Myklebust <[email protected]>

---

 fs/nfs/inode.c |   44 +++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 41 insertions(+), 3 deletions(-)

fs/nfs/inode.c +41 -3

index f189169..8c514a1 100644
@@ -948,11 +948,49 @@ static int nfs_check_inode_attributes(struct inode *inode, struct nfs_fattr *fat
 	return 0;
 }
 
-static int nfs_refresh_inode_locked(struct inode *inode, struct nfs_fattr *fattr)
+static int nfs_ctime_need_update(const struct inode *inode, const struct nfs_fattr *fattr)
 {
-	struct nfs_inode *nfsi = NFS_I(inode);
+	return timespec_compare(&fattr->ctime, &inode->i_ctime) > 0;
+}
+
+static int nfs_size_need_update(const struct inode *inode, const struct nfs_fattr *fattr)
+{
+	return nfs_size_to_loff_t(fattr->size) > i_size_read(inode);
+}
 
-	if (time_after(fattr->time_start, nfsi->last_updated))
+/**
+ * nfs_inode_attrs_need_update - check if the inode attributes need updating
+ * @inode - pointer to inode
+ * @fattr - attributes
+ *
+ * Attempt to divine whether or not an RPC call reply carrying stale
+ * attributes got scheduled after another call carrying updated ones.
+ *
+ * To do so, the function first assumes that a more recent ctime means
+ * that the attributes in fattr are newer, however it also attempt to
+ * catch the case where ctime either didn't change, or went backwards
+ * (if someone reset the clock on the server) by looking at whether
+ * or not this RPC call was started after the inode was last updated.
+ * Note also the check for jiffy wraparound if the last_updated timestamp
+ * is later than 'jiffies'.
+ *
+ * The function returns 'true' if it thinks the attributes in 'fattr' are
+ * more recent than the ones cached in the inode.
+ *
+ */
+static int nfs_inode_attrs_need_update(const struct inode *inode, const struct nfs_fattr *fattr)
+{
+	const struct nfs_inode *nfsi = NFS_I(inode);
+
+	return nfs_ctime_need_update(inode, fattr) ||
+			nfs_size_need_update(inode, fattr) ||
+			time_after(fattr->time_start, nfsi->last_updated) ||
+			time_after(nfsi->last_updated, jiffies);
+}
+
+static int nfs_refresh_inode_locked(struct inode *inode, struct nfs_fattr *fattr)
+{
+	if (nfs_inode_attrs_need_update(inode, fattr))
 		return nfs_update_inode(inode, fattr);
 	return nfs_check_inode_attributes(inode, fattr);
 }

Trond Myklebust

author

2008-10-07 22:32 UTC #9 lore raw

PATCH 8/30 NFS: Fix nfs_post_op_update_inode_force_wcc()

In reply to a message not in the archive

If we believe that the attributes are old (see nfs_refresh_inode()), then we shouldn't force an update.
Also ensure that we hold the inode->i_lock across attribute checks and the call to nfs_refresh_inode_locked() to ensure that we don't race with other attribute updates.

Signed-off-by Trond Myklebust <[email protected]>

---

 fs/nfs/inode.c |   35 +++++++++++++++++++++++++++--------
 1 files changed, 27 insertions(+), 8 deletions(-)

fs/nfs/inode.c +27 -8

index 8c514a1..610d022 100644
@@ -1017,6 +1017,18 @@ int nfs_refresh_inode(struct inode *inode, struct nfs_fattr *fattr)
 	return status;
 }
 
+static int nfs_post_op_update_inode_locked(struct inode *inode, struct nfs_fattr *fattr)
+{
+	struct nfs_inode *nfsi = NFS_I(inode);
+
+	nfsi->cache_validity |= NFS_INO_INVALID_ATTR|NFS_INO_REVAL_PAGECACHE;
+	if (S_ISDIR(inode->i_mode))
+		nfsi->cache_validity |= NFS_INO_INVALID_DATA;
+	if ((fattr->valid & NFS_ATTR_FATTR) == 0)
+		return 0;
+	return nfs_refresh_inode_locked(inode, fattr);
+}
+
 /**
  * nfs_post_op_update_inode - try to update the inode attribute cache
  * @inode - pointer to inode

@@ -1033,15 +1045,10 @@ int nfs_refresh_inode(struct inode *inode, struct nfs_fattr *fattr)
  */
 int nfs_post_op_update_inode(struct inode *inode, struct nfs_fattr *fattr)
 {
-	struct nfs_inode *nfsi = NFS_I(inode);
-	int status = 0;
+	int status;
 
 	spin_lock(&inode->i_lock);
-	nfsi->cache_validity |= NFS_INO_INVALID_ATTR|NFS_INO_REVAL_PAGECACHE;
-	if (S_ISDIR(inode->i_mode))
-		nfsi->cache_validity |= NFS_INO_INVALID_DATA;
-	if ((fattr->valid & NFS_ATTR_FATTR) != 0)
-		status = nfs_refresh_inode_locked(inode, fattr);
+	status = nfs_post_op_update_inode_locked(inode, fattr);
 	spin_unlock(&inode->i_lock);
 	return status;
 }

@@ -1059,6 +1066,15 @@ int nfs_post_op_update_inode(struct inode *inode, struct nfs_fattr *fattr)
  */
 int nfs_post_op_update_inode_force_wcc(struct inode *inode, struct nfs_fattr *fattr)
 {
+	int status;
+
+	spin_lock(&inode->i_lock);
+	/* Don't do a WCC update if these attributes are already stale */
+	if ((fattr->valid & NFS_ATTR_FATTR) == 0 ||
+			!nfs_inode_attrs_need_update(inode, fattr)) {
+		fattr->valid &= ~(NFS_ATTR_WCC_V4|NFS_ATTR_WCC);
+		goto out_noforce;
+	}
 	if ((fattr->valid & NFS_ATTR_FATTR_V4) != 0 &&
 			(fattr->valid & NFS_ATTR_WCC_V4) == 0) {
 		fattr->pre_change_attr = NFS_I(inode)->change_attr;

@@ -1071,7 +1087,10 @@ int nfs_post_op_update_inode_force_wcc(struct inode *inode, struct nfs_fattr *fa
 		fattr->pre_size = i_size_read(inode);
 		fattr->valid |= NFS_ATTR_WCC;
 	}
-	return nfs_post_op_update_inode(inode, fattr);
+out_noforce:
+	status = nfs_post_op_update_inode_locked(inode, fattr);
+	spin_unlock(&inode->i_lock);
+	return status;
 }
 
 /*

Trond Myklebust

author

2008-10-07 22:32 UTC #10 lore raw

PATCH 9/30 NFS: Convert __nfs_revalidate_inode() to use nfs_refresh_inode()

In reply to a message not in the archive

In the case where there are parallel RPC calls to the same inode, we may receive stale metadata due to the lack of ordering, hence the sanity checking of metadata in nfs_refresh_inode().
Currently, __nfs_revalidate_inode() is calling nfs_update_inode() directly, without any further sanity checks, and hence may end up setting the inode up with stale metadata.

Fix is to use nfs_refresh_inode() instead of nfs_update_inode().

Signed-off-by Trond Myklebust <[email protected]>

---

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

fs/nfs/inode.c +1 -4

index 610d022..697157c 100644
@@ -724,16 +724,13 @@ __nfs_revalidate_inode(struct nfs_server *server, struct inode *inode)
 		goto out;
 	}
 
-	spin_lock(&inode->i_lock);
-	status = nfs_update_inode(inode, &fattr);
+	status = nfs_refresh_inode(inode, &fattr);
 	if (status) {
-		spin_unlock(&inode->i_lock);
 		dfprintk(PAGECACHE, "nfs_revalidate_inode: (%s/%Ld) refresh failed, error=%d\n",
 			 inode->i_sb->s_id,
 			 (long long)NFS_FILEID(inode), status);
 		goto out;
 	}
-	spin_unlock(&inode->i_lock);
 
 	if (nfsi->cache_validity & NFS_INO_INVALID_ACL)
 		nfs_zap_acl_cache(inode);

Trond Myklebust

author

2008-10-07 22:32 UTC #11 lore raw

PATCH 10/30 NFS: Don't clear nfsi->cache_validity in nfs_check_inode_attributes()

In reply to a message not in the archive

If we're merely checking the inode attributes because we suspect that the 'updated' attributes returned by the RPC call are stale, then we shouldn't be doing weak cache consistency updates or clearing the cache_validity flags.

Signed-off-by Trond Myklebust <[email protected]>

---

 fs/nfs/inode.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

fs/nfs/inode.c +0 -7

index 697157c..a2f5415 100644
@@ -905,9 +905,6 @@ static int nfs_check_inode_attributes(struct inode *inode, struct nfs_fattr *fat
 		return -EIO;
 	}
 
-	/* Do atomic weak cache consistency updates */
-	nfs_wcc_update_inode(inode, fattr);
-
 	if ((fattr->valid & NFS_ATTR_FATTR_V4) != 0 &&
 			nfsi->change_attr != fattr->change_attr)
 		invalid |= NFS_INO_INVALID_ATTR|NFS_INO_REVAL_PAGECACHE;

@@ -936,10 +933,6 @@ static int nfs_check_inode_attributes(struct inode *inode, struct nfs_fattr *fat
 
 	if (invalid != 0)
 		nfsi->cache_validity |= invalid;
-	else
-		nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR
-				| NFS_INO_INVALID_ATIME
-				| NFS_INO_REVAL_PAGECACHE);
 
 	nfsi->read_cache_jiffies = fattr->time_start;
 	return 0;

Trond Myklebust

author

2008-10-07 22:32 UTC #12 lore raw

PATCH 11/30 NFS: Fix up nfs_setattr_update_inode()

In reply to a message not in the archive

Ensure that it sets the inode metadata under the correct spinlock.

Signed-off-by Trond Myklebust <[email protected]>

---

 fs/nfs/inode.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

fs/nfs/inode.c +1 -1

index a2f5415..f3b8ed9 100644
@@ -453,6 +453,7 @@ out_big:
 void nfs_setattr_update_inode(struct inode *inode, struct iattr *attr)
 {
 	if ((attr->ia_valid & (ATTR_MODE|ATTR_UID|ATTR_GID)) != 0) {
+		spin_lock(&inode->i_lock);
 		if ((attr->ia_valid & ATTR_MODE) != 0) {
 			int mode = attr->ia_mode & S_IALLUGO;
 			mode |= inode->i_mode & ~S_IALLUGO;

@@ -462,7 +463,6 @@ void nfs_setattr_update_inode(struct inode *inode, struct iattr *attr)
 			inode->i_uid = attr->ia_uid;
 		if ((attr->ia_valid & ATTR_GID) != 0)
 			inode->i_gid = attr->ia_gid;
-		spin_lock(&inode->i_lock);
 		NFS_I(inode)->cache_validity |= NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL;
 		spin_unlock(&inode->i_lock);
 	}

Trond Myklebust

author

2008-10-07 22:32 UTC #13 lore raw

PATCH 12/30 NFS: Allow concurrent inode revalidation

In reply to a message not in the archive

Currently, if two processes are both trying to revalidate metadata for the same inode, they will find themselves being serialised. There is no good justification for this now that we have improved our ability to detect stale attribute data, so we should remove that serialisation.

Signed-off-by Trond Myklebust <[email protected]>

---

 fs/nfs/inode.c         |   43 ++-----------------------------------------
 include/linux/nfs_fs.h |    9 ++++-----
 2 files changed, 6 insertions(+), 46 deletions(-)

fs/nfs/inode.c +2 ~~-41~~

index f3b8ed9..e25009f 100644
@@ -472,37 +472,6 @@ void nfs_setattr_update_inode(struct inode *inode, struct iattr *attr)
 	}
 }
 
-static int nfs_wait_schedule(void *word)
-{
-	if (signal_pending(current))
-		return -ERESTARTSYS;
-	schedule();
-	return 0;
-}
-
-/*
- * Wait for the inode to get unlocked.
- */
-static int nfs_wait_on_inode(struct inode *inode)
-{
-	struct nfs_inode *nfsi = NFS_I(inode);
-	int error;
-
-	error = wait_on_bit_lock(&nfsi->flags, NFS_INO_REVALIDATING,
-					nfs_wait_schedule, TASK_KILLABLE);
-
-	return error;
-}
-
-static void nfs_wake_up_inode(struct inode *inode)
-{
-	struct nfs_inode *nfsi = NFS_I(inode);
-
-	clear_bit(NFS_INO_REVALIDATING, &nfsi->flags);
-	smp_mb__after_clear_bit();
-	wake_up_bit(&nfsi->flags, NFS_INO_REVALIDATING);
-}
-
 int nfs_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat)
 {
 	struct inode *inode = dentry->d_inode;

@@ -697,20 +666,15 @@ __nfs_revalidate_inode(struct nfs_server *server, struct inode *inode)
 	dfprintk(PAGECACHE, "NFS: revalidating (%s/%Ld)\n",
 		inode->i_sb->s_id, (long long)NFS_FILEID(inode));
 
-	nfs_inc_stats(inode, NFSIOS_INODEREVALIDATE);
 	if (is_bad_inode(inode))
- 		goto out_nowait;
+		goto out;
 	if (NFS_STALE(inode))
- 		goto out_nowait;
-
-	status = nfs_wait_on_inode(inode);
-	if (status < 0)
 		goto out;
 
-	status = -ESTALE;
 	if (NFS_STALE(inode))
 		goto out;
 
+	nfs_inc_stats(inode, NFSIOS_INODEREVALIDATE);
 	status = NFS_PROTO(inode)->getattr(server, NFS_FH(inode), &fattr);
 	if (status != 0) {
 		dfprintk(PAGECACHE, "nfs_revalidate_inode: (%s/%Ld) getattr failed, error=%d\n",

@@ -740,9 +704,6 @@ __nfs_revalidate_inode(struct nfs_server *server, struct inode *inode)
 		(long long)NFS_FILEID(inode));
 
  out:
-	nfs_wake_up_inode(inode);
-
- out_nowait:
 	return status;
 }

include/linux/nfs_fs.h +4 -5

index 78a5922..ca563ee 100644
@@ -200,11 +200,10 @@ struct nfs_inode {
 /*
  * Bit offsets in flags field
  */
-#define NFS_INO_REVALIDATING	(0)		/* revalidating attrs */
-#define NFS_INO_ADVISE_RDPLUS	(1)		/* advise readdirplus */
-#define NFS_INO_STALE		(2)		/* possible stale inode */
-#define NFS_INO_ACL_LRU_SET	(3)		/* Inode is on the LRU list */
-#define NFS_INO_MOUNTPOINT	(4)		/* inode is remote mountpoint */
+#define NFS_INO_ADVISE_RDPLUS	(0)		/* advise readdirplus */
+#define NFS_INO_STALE		(1)		/* possible stale inode */
+#define NFS_INO_ACL_LRU_SET	(2)		/* Inode is on the LRU list */
+#define NFS_INO_MOUNTPOINT	(3)		/* inode is remote mountpoint */
 
 static inline struct nfs_inode *NFS_I(const struct inode *inode)
 {

Trond Myklebust

author

2008-10-07 22:32 UTC #14 lore raw

PATCH 13/30 fix fs/nfs/nfsroot.c compilation

In reply to a message not in the archive

From: Adrian Bunk <[email protected]>

This patch fixes the following compile error caused by commit f9247273cb69ba101877e946d2d83044409cc8c5 (UFS: add const to parser token tabl):

<-- snip -->

...
CC fs/nfs/nfsroot.o
/home/bunk/linux/kernel-2.6/git/linux-2.6/fs/nfs/nfsroot.c:130: error: tokens causes a section type conflict make[3]: *** [fs/nfs/nfsroot.o] Error 1

<-- snip -->

Signed-off-by Adrian Bunk <[email protected]>

Signed-off-by Trond Myklebust <[email protected]>

---

 fs/nfs/nfsroot.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

fs/nfs/nfsroot.c +1 -1

index 46763d1..8478fc2 100644
@@ -127,7 +127,7 @@ enum {
 	Opt_err
 };
 
-static match_table_t __initdata tokens = {
+static match_table_t __initconst tokens = {
 	{Opt_port, "port=%u"},
 	{Opt_rsize, "rsize=%u"},
 	{Opt_wsize, "wsize=%u"},

Trond Myklebust

author

2008-10-07 22:32 UTC #15 lore raw

PATCH 14/30 nfs: ERR_PTR is expected on failure from nfs_do_clone_mount

In reply to a message not in the archive

From: Denis V. Lunev <[email protected]>

Replace NULL with ERR_PTR(-EINVAL).

Signed-off-by Denis V. Lunev <[email protected]>

Signed-off-by Trond Myklebust <[email protected]>

---

 fs/nfs/namespace.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

fs/nfs/namespace.c +1 -1

index 66df08d..d398775 100644
@@ -189,7 +189,7 @@ static struct vfsmount *nfs_do_clone_mount(struct nfs_server *server,
 					   struct nfs_clone_mount *mountdata)
 {
 #ifdef CONFIG_NFS_V4
-	struct vfsmount *mnt = NULL;
+	struct vfsmount *mnt = ERR_PTR(-EINVAL);
 	switch (server->nfs_client->rpc_ops->version) {
 		case 2:
 		case 3:

Trond Myklebust

author

2008-10-07 22:32 UTC #16 lore raw

PATCH 15/30 sunrpc: do not pin sunrpc module in the memory

In reply to a message not in the archive

From: Denis V. Lunev <[email protected]>

Basically, try_module_get here are pretty useless. Any other module using this API will pin sunrpc in memory due using exported symbols.

Signed-off-by Denis V. Lunev <[email protected]>

Signed-off-by Trond Myklebust <[email protected]>

---

 net/sunrpc/xprt.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

net/sunrpc/xprt.c +4 -8

index 99a52aa..29e401b 100644
@@ -108,13 +108,10 @@ int xprt_register_transport(struct xprt_class *transport)
 			goto out;
 	}
 
-	result = -EINVAL;
-	if (try_module_get(THIS_MODULE)) {
-		list_add_tail(&transport->list, &xprt_list);
-		printk(KERN_INFO "RPC: Registered %s transport module.\n",
-			transport->name);
-		result = 0;
-	}
+	list_add_tail(&transport->list, &xprt_list);
+	printk(KERN_INFO "RPC: Registered %s transport module.\n",
+	       transport->name);
+	result = 0;
 
 out:
 	spin_unlock(&xprt_list_lock);

@@ -143,7 +140,6 @@ int xprt_unregister_transport(struct xprt_class *transport)
 				"RPC: Unregistered %s transport module.\n",
 				transport->name);
 			list_del_init(&transport->list);
-			module_put(THIS_MODULE);
 			goto out;
 		}
 	}

Trond Myklebust

author

2008-10-07 22:32 UTC #17 lore raw

PATCH 16/30 nfs: BUG_ON in nfs_follow_mountpoint

In reply to a message not in the archive

From: Denis V. Lunev <[email protected]>

Unfortunately, BUG_ON(IS_ROOT(dentry)) can happen inside nfs_follow_mountpoint with NFS running Fedora 8 using a specific setup.
https://bugzilla.redhat.com/show_bug.cgi?id=458622

So, the situation should be handled on NFS client gracefully.

Signed-off-by Denis V. Lunev <[email protected]>

CC Trond Myklebust <[email protected]>

CC J. Bruce Fields <[email protected]>

Signed-off-by Trond Myklebust <[email protected]>

---

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

fs/nfs/namespace.c +4 -1

index d398775..64a288e 100644
@@ -105,7 +105,10 @@ static void * nfs_follow_mountpoint(struct dentry *dentry, struct nameidata *nd)
 
 	dprintk("--> nfs_follow_mountpoint()\n");
 
-	BUG_ON(IS_ROOT(dentry));
+	err = -ESTALE;
+	if (IS_ROOT(dentry))
+		goto out_err;
+
 	dprintk("%s: enter\n", __func__);
 	dput(nd->path.dentry);
 	nd->path.dentry = dget(dentry);

Trond Myklebust

author

2008-10-07 22:32 UTC #18 lore raw

PATCH 17/30 nfs: remove an obsolete nfs_flock comment

In reply to a message not in the archive

From: J. Bruce Fields <[email protected]>

We *do* now allow bsd flocks over nfs.

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Trond Myklebust <[email protected]>

---

 fs/nfs/file.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

fs/nfs/file.c +0 -7

index 3ddb004..d319b49 100644
@@ -702,13 +702,6 @@ static int nfs_flock(struct file *filp, int cmd, struct file_lock *fl)
 			filp->f_path.dentry->d_name.name,
 			fl->fl_type, fl->fl_flags);
 
-	/*
-	 * No BSD flocks over NFS allowed.
-	 * Note: we could try to fake a POSIX lock request here by
-	 * using ((u32) filp | 0x80000000) or some such as the pid.
-	 * Not sure whether that would be unique, though, or whether
-	 * that would break in other places.
-	 */
 	if (!(fl->fl_flags & FL_FLOCK))
 		return -ENOLCK;

Trond Myklebust

author

2008-10-07 22:32 UTC #19 lore raw

PATCH 18/30 NFS: missing nfs_fattr_init in nfs3_proc_getacl and nfs3_proc_setacls (resend #2)

In reply to a message not in the archive

From: Jeff Layton <[email protected]>

The fattrs used in the NFSv3 getacl/setacl calls are not being properly initialized. This occasionally causes nfs_update_inode to fall into NFSv4 specific codepaths when handling post-op attrs from these calls.

Thanks to Cai Qian for noticing the spurious NFSv4 messages in debug output from a v3 mount...

Signed-off-by Jeff Layton <[email protected]>

Signed-off-by Trond Myklebust <[email protected]>

---

 fs/nfs/nfs3acl.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

fs/nfs/nfs3acl.c +2 -0

index 423842f..cef6255 100644
@@ -229,6 +229,7 @@ struct posix_acl *nfs3_proc_getacl(struct inode *inode, int type)
 
 	dprintk("NFS call getacl\n");
 	msg.rpc_proc = &server->client_acl->cl_procinfo[ACLPROC3_GETACL];
+	nfs_fattr_init(&fattr);
 	status = rpc_call_sync(server->client_acl, &msg, 0);
 	dprintk("NFS reply getacl: %d\n", status);

@@ -322,6 +323,7 @@ static int nfs3_proc_setacls(struct inode *inode, struct posix_acl *acl,
 
 	dprintk("NFS call setacl\n");
 	msg.rpc_proc = &server->client_acl->cl_procinfo[ACLPROC3_SETACL];
+	nfs_fattr_init(&fattr);
 	status = rpc_call_sync(server->client_acl, &msg, 0);
 	nfs_access_zap_cache(inode);
 	nfs_zap_acl_cache(inode);

Trond Myklebust

author

2008-10-07 22:32 UTC #20 lore raw

PATCH 19/30 nfs: authenticated deep mounting

In reply to a message not in the archive

From: EG Keizer <keie-vHs5IaWfoDhmR6Xm/[email protected]>

Allow mount to do authenticated mounts below the root of the exported tree. The wording in RFC 2623, sec 2.3.2. allows fsinfo with UNIX authentication on the root of the export. Mounts are not always done on the root of the exported tree. Especially autoumounts often mount below the root of the exported tree.
Some server implementations (justly) require full authentication for the so-called deep mounts. The old code used AUTH_SYS only. This caused deep mounts to fail on systems requiring stronger authentication..
The client should try both authentication types and use the first one that succeeds.
This method was already partially implemented. This patch completes the implementation for NFS2 and NFS3.
This patch was developed to allow Debian systems to automount home directories on Solaris servers with krb5 authentication.

Tested on kernel 2.6.24-etchnhalf.1

Signed-off-by E.G. Keizer <keie-vHs5IaWfoDhmR6Xm/[email protected]>

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Trond Myklebust <[email protected]>

---

 fs/nfs/nfs3proc.c |   20 ++++++++++++++++++--
 fs/nfs/proc.c     |   10 ++++++++--
 2 files changed, 26 insertions(+), 4 deletions(-)

fs/nfs/nfs3proc.c +18 -2

index 1e750e4..c55be7a 100644
@@ -699,7 +699,7 @@ nfs3_proc_statfs(struct nfs_server *server, struct nfs_fh *fhandle,
 }
 
 static int
-nfs3_proc_fsinfo(struct nfs_server *server, struct nfs_fh *fhandle,
+do_proc_fsinfo(struct rpc_clnt *client, struct nfs_fh *fhandle,
 		 struct nfs_fsinfo *info)
 {
 	struct rpc_message msg = {

@@ -711,11 +711,27 @@ nfs3_proc_fsinfo(struct nfs_server *server, struct nfs_fh *fhandle,
 
 	dprintk("NFS call  fsinfo\n");
 	nfs_fattr_init(info->fattr);
-	status = rpc_call_sync(server->nfs_client->cl_rpcclient, &msg, 0);
+	status = rpc_call_sync(client, &msg, 0);
 	dprintk("NFS reply fsinfo: %d\n", status);
 	return status;
 }
 
+/*
+ * Bare-bones access to fsinfo: this is for nfs_get_root/nfs_get_sb via
+ * nfs_create_server
+ */
+static int
+nfs3_proc_fsinfo(struct nfs_server *server, struct nfs_fh *fhandle,
+		   struct nfs_fsinfo *info)
+{
+	int	status;
+
+	status = do_proc_fsinfo(server->client, fhandle, info);
+	if (status && server->nfs_client->cl_rpcclient != server->client)
+		status = do_proc_fsinfo(server->nfs_client->cl_rpcclient, fhandle, info);
+	return status;
+}
+
 static int
 nfs3_proc_pathconf(struct nfs_server *server, struct nfs_fh *fhandle,
 		   struct nfs_pathconf *info)

fs/nfs/proc.c +8 -2

index 4dbb84d..1934652 100644
@@ -65,14 +65,20 @@ nfs_proc_get_root(struct nfs_server *server, struct nfs_fh *fhandle,
 
 	dprintk("%s: call getattr\n", __func__);
 	nfs_fattr_init(fattr);
-	status = rpc_call_sync(server->nfs_client->cl_rpcclient, &msg, 0);
+	status = rpc_call_sync(server->client, &msg, 0);
+	/* Retry with default authentication if different */
+	if (status && server->nfs_client->cl_rpcclient != server->client)
+		status = rpc_call_sync(server->nfs_client->cl_rpcclient, &msg, 0);
 	dprintk("%s: reply getattr: %d\n", __func__, status);
 	if (status)
 		return status;
 	dprintk("%s: call statfs\n", __func__);
 	msg.rpc_proc = &nfs_procedures[NFSPROC_STATFS];
 	msg.rpc_resp = &fsinfo;
-	status = rpc_call_sync(server->nfs_client->cl_rpcclient, &msg, 0);
+	status = rpc_call_sync(server->client, &msg, 0);
+	/* Retry with default authentication if different */
+	if (status && server->nfs_client->cl_rpcclient != server->client)
+		status = rpc_call_sync(server->nfs_client->cl_rpcclient, &msg, 0);
 	dprintk("%s: reply statfs: %d\n", __func__, status);
 	if (status)
 		return status;

Trond Myklebust

author

2008-10-07 22:32 UTC #21 lore raw

PATCH 20/30 nfs: break up nfs_follow_referral

In reply to a message not in the archive

From: J. Bruce Fields <[email protected]>

This function is a little longer and more deeply nested than necessary.

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Trond Myklebust <[email protected]>

---

 fs/nfs/nfs4namespace.c |   84 ++++++++++++++++++++++++++----------------------
 1 files changed, 46 insertions(+), 38 deletions(-)

fs/nfs/nfs4namespace.c +46 ~~-38~~

index b112857..956cbbc 100644
@@ -110,6 +110,48 @@ static inline int valid_ipaddr4(const char *buf)
 	return 0;
 }
 
+static struct vfsmount *try_location(struct nfs_clone_mount *mountdata,
+				     char *page, char *page2,
+				     const struct nfs4_fs_location *location)
+{
+	struct vfsmount *mnt = ERR_PTR(-ENOENT);
+	char *mnt_path;
+	unsigned int s = 0;
+
+	mnt_path = nfs4_pathname_string(&location->rootpath, page2, PAGE_SIZE);
+	if (IS_ERR(mnt_path))
+		return mnt;
+	mountdata->mnt_path = mnt_path;
+
+	while (s < location->nservers) {
+		struct sockaddr_in addr = {
+			.sin_family	= AF_INET,
+			.sin_port	= htons(NFS_PORT),
+		};
+
+		if (location->servers[s].len <= 0 ||
+		    valid_ipaddr4(location->servers[s].data) < 0) {
+			s++;
+			continue;
+		}
+
+		mountdata->hostname = location->servers[s].data;
+		addr.sin_addr.s_addr = in_aton(mountdata->hostname),
+		mountdata->addr = (struct sockaddr *)&addr;
+		mountdata->addrlen = sizeof(addr);
+
+		snprintf(page, PAGE_SIZE, "%s:%s",
+				mountdata->hostname,
+				mountdata->mnt_path);
+
+		mnt = vfs_kern_mount(&nfs4_referral_fs_type, 0, page, mountdata);
+		if (!IS_ERR(mnt))
+			break;
+		s++;
+	}
+	return mnt;
+}
+
 /**
  * nfs_follow_referral - set up mountpoint when hitting a referral on moved error
  * @mnt_parent - mountpoint of parent directory

@@ -128,7 +170,6 @@ static struct vfsmount *nfs_follow_referral(const struct vfsmount *mnt_parent,
 		.authflavor = NFS_SB(mnt_parent->mnt_sb)->client->cl_auth->au_flavor,
 	};
 	char *page = NULL, *page2 = NULL;
-	unsigned int s;
 	int loc, error;
 
 	if (locations == NULL || locations->nlocations <= 0)

@@ -153,9 +194,8 @@ static struct vfsmount *nfs_follow_referral(const struct vfsmount *mnt_parent,
 	}
 
 	loc = 0;
-	while (loc < locations->nlocations && IS_ERR(mnt)) {
+	while (loc < locations->nlocations) {
 		const struct nfs4_fs_location *location = &locations->locations[loc];
-		char *mnt_path;
 
 		if (location == NULL || location->nservers <= 0 ||
 		    location->rootpath.ncomponents == 0) {

@@ -163,41 +203,9 @@ static struct vfsmount *nfs_follow_referral(const struct vfsmount *mnt_parent,
 			continue;
 		}
 
-		mnt_path = nfs4_pathname_string(&location->rootpath, page2, PAGE_SIZE);
-		if (IS_ERR(mnt_path)) {
-			loc++;
-			continue;
-		}
-		mountdata.mnt_path = mnt_path;
-
-		s = 0;
-		while (s < location->nservers) {
-			struct sockaddr_in addr = {
-				.sin_family	= AF_INET,
-				.sin_port	= htons(NFS_PORT),
-			};
-
-			if (location->servers[s].len <= 0 ||
-			    valid_ipaddr4(location->servers[s].data) < 0) {
-				s++;
-				continue;
-			}
-
-			mountdata.hostname = location->servers[s].data;
-			addr.sin_addr.s_addr = in_aton(mountdata.hostname),
-			mountdata.addr = (struct sockaddr *)&addr;
-			mountdata.addrlen = sizeof(addr);
-
-			snprintf(page, PAGE_SIZE, "%s:%s",
-					mountdata.hostname,
-					mountdata.mnt_path);
-
-			mnt = vfs_kern_mount(&nfs4_referral_fs_type, 0, page, &mountdata);
-			if (!IS_ERR(mnt)) {
-				break;
-			}
-			s++;
-		}
+		mnt = try_location(&mountdata, page, page2, location);
+		if (!IS_ERR(mnt))
+			break;
 		loc++;
 	}

Trond Myklebust

author

2008-10-07 22:32 UTC #22 lore raw

PATCH 21/30 nfs: replace while loop by for loops in nfs_follow_referral

In reply to a message not in the archive

From: J. Bruce Fields <[email protected]>

Whoever wrote this had a bizarre allergy to for loops.

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Trond Myklebust <[email protected]>

---

 fs/nfs/nfs4namespace.c |   17 +++++------------
 1 files changed, 5 insertions(+), 12 deletions(-)

fs/nfs/nfs4namespace.c +5 ~~-12~~

index 956cbbc..6bcc569 100644
@@ -116,24 +116,22 @@ static struct vfsmount *try_location(struct nfs_clone_mount *mountdata,
 {
 	struct vfsmount *mnt = ERR_PTR(-ENOENT);
 	char *mnt_path;
-	unsigned int s = 0;
+	unsigned int s;
 
 	mnt_path = nfs4_pathname_string(&location->rootpath, page2, PAGE_SIZE);
 	if (IS_ERR(mnt_path))
 		return mnt;
 	mountdata->mnt_path = mnt_path;
 
-	while (s < location->nservers) {
+	for (s = 0; s < location->nservers; s++) {
 		struct sockaddr_in addr = {
 			.sin_family	= AF_INET,
 			.sin_port	= htons(NFS_PORT),
 		};
 
 		if (location->servers[s].len <= 0 ||
-		    valid_ipaddr4(location->servers[s].data) < 0) {
-			s++;
+		    valid_ipaddr4(location->servers[s].data) < 0)
 			continue;
-		}
 
 		mountdata->hostname = location->servers[s].data;
 		addr.sin_addr.s_addr = in_aton(mountdata->hostname),

@@ -147,7 +145,6 @@ static struct vfsmount *try_location(struct nfs_clone_mount *mountdata,
 		mnt = vfs_kern_mount(&nfs4_referral_fs_type, 0, page, mountdata);
 		if (!IS_ERR(mnt))
 			break;
-		s++;
 	}
 	return mnt;
 }

@@ -193,20 +190,16 @@ static struct vfsmount *nfs_follow_referral(const struct vfsmount *mnt_parent,
 		goto out;
 	}
 
-	loc = 0;
-	while (loc < locations->nlocations) {
+	for (loc = 0; loc < locations->nlocations; loc++) {
 		const struct nfs4_fs_location *location = &locations->locations[loc];
 
 		if (location == NULL || location->nservers <= 0 ||
-		    location->rootpath.ncomponents == 0) {
-			loc++;
+		    location->rootpath.ncomponents == 0)
 			continue;
-		}
 
 		mnt = try_location(&mountdata, page, page2, location);
 		if (!IS_ERR(mnt))
 			break;
-		loc++;
 	}
 
 out:

Trond Myklebust

author

2008-10-07 22:32 UTC #23 lore raw

PATCH 22/30 nfs: prepare to share nfs_set_port

In reply to a message not in the archive

From: J. Bruce Fields <[email protected]>

We plan to use this function elsewhere.

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Trond Myklebust <[email protected]>

---

 fs/nfs/internal.h |   20 ++++++++++++++++++++
 fs/nfs/super.c    |   19 -------------------
 2 files changed, 20 insertions(+), 19 deletions(-)

fs/nfs/internal.h +20 -0

index 7bcf6ec..8d91bd8 100644
@@ -276,3 +276,23 @@ unsigned int nfs_page_array_len(unsigned int base, size_t len)
 		PAGE_SIZE - 1) >> PAGE_SHIFT;
 }
 
+
+/*
+ * Set the port number in an address.  Be agnostic about the address
+ * family.
+ */
+static inline void nfs_set_port(struct sockaddr *sap, unsigned short port)
+{
+	switch (sap->sa_family) {
+	case AF_INET: {
+	      struct sockaddr_in *ap = (struct sockaddr_in *)sap;
+	      ap->sin_port = htons(port);
+	      break;
+	}
+	case AF_INET6: {
+	       struct sockaddr_in6 *ap = (struct sockaddr_in6 *)sap;
+	       ap->sin6_port = htons(port);
+	       break;
+	}
+	}
+}

fs/nfs/super.c +0 ~~-19~~

index 1e35586..b99096b 100644
@@ -675,25 +675,6 @@ static void nfs_umount_begin(struct super_block *sb)
 }
 
 /*
- * Set the port number in an address.  Be agnostic about the address family.
- */
-static void nfs_set_port(struct sockaddr *sap, unsigned short port)
-{
-	switch (sap->sa_family) {
-	case AF_INET: {
-		struct sockaddr_in *ap = (struct sockaddr_in *)sap;
-		ap->sin_port = htons(port);
-		break;
-	}
-	case AF_INET6: {
-		struct sockaddr_in6 *ap = (struct sockaddr_in6 *)sap;
-		ap->sin6_port = htons(port);
-		break;
-	}
-	}
-}
-
-/*
  * Sanity-check a server address provided by the mount command.
  *
  * Address family must be initialized, and address must not be

Trond Myklebust

author

2008-10-07 22:32 UTC #24 lore raw

PATCH 23/30 nfs: Fix misparsing of nfsv4 fs_locations attribute

In reply to a message not in the archive

From: J. Bruce Fields <[email protected]>

The code incorrectly assumes here that the server name (or ip address) is null-terminated. This can cause referrals to fail in some cases.

Also support ipv6 addresses.

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Trond Myklebust <[email protected]>

---

 fs/nfs/internal.h      |    2 ++
 fs/nfs/nfs4namespace.c |   44 ++++++++++++++++++--------------------------
 fs/nfs/super.c         |    4 +---
 3 files changed, 21 insertions(+), 29 deletions(-)

fs/nfs/internal.h +2 -0

index 8d91bd8..5d2a5d3 100644
@@ -153,6 +153,7 @@ extern void nfs4_clear_inode(struct inode *);
 void nfs_zap_acl_cache(struct inode *inode);
 
 /* super.c */
+void nfs_parse_ip_address(char *, size_t, struct sockaddr *, size_t *);
 extern struct file_system_type nfs_xdev_fs_type;
 #ifdef CONFIG_NFS_V4
 extern struct file_system_type nfs4_xdev_fs_type;

@@ -276,6 +277,7 @@ unsigned int nfs_page_array_len(unsigned int base, size_t len)
 		PAGE_SIZE - 1) >> PAGE_SHIFT;
 }
 
+#define IPV6_SCOPE_DELIMITER	'%'
 
 /*
  * Set the port number in an address.  Be agnostic about the address

fs/nfs/nfs4namespace.c +18 ~~-26~~

68 lines not on this page · raw message

fs/nfs/super.c +1 -3

index b99096b..20dc4cc 100644
@@ -716,8 +716,6 @@ static void nfs_parse_ipv4_address(char *string, size_t str_len,
 	*addr_len = 0;
 }
 
-#define IPV6_SCOPE_DELIMITER	'%'
-
 #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
 static void nfs_parse_ipv6_scope_id(const char *string, const size_t str_len,
 				    const char *delim,

@@ -790,7 +788,7 @@ static void nfs_parse_ipv6_address(char *string, size_t str_len,
  * If there is a problem constructing the new sockaddr, set the address
  * family to AF_UNSPEC.
  */
-static void nfs_parse_ip_address(char *string, size_t str_len,
+void nfs_parse_ip_address(char *string, size_t str_len,
 				 struct sockaddr *sap, size_t *addr_len)
 {
 	unsigned int i, colons;

Trond Myklebust

author

2008-10-07 22:32 UTC #25 lore raw

PATCH 24/30 NFS: remove 8 bytes of padding from struct nfs_fattr on 64 bit builds

In reply to a message not in the archive

From: Richard Kennedy <[email protected]>

remove 8 bytes of padding from struct nfs_fattr on 64 bit builds

This also removes padding from several nfs structures, including 16 bytes from nfs4_opendata, nfs4_createdata,nfs3_createdata & 8 bytes from nfs_read_data,nfs_write_data,nfs_removeres,nfs4_closedata

This also reduces the reported stack usage of many nfs functions (30+).

Signed-off-by Richard Kennedy <[email protected]>

----

This patch is against the latest git 2.6.27-rc4.
I've built & run this on my AMD64 desktop, & successfully run _simple_ tests with a 64 bit client => 32 bit server & 32 bit client to 64 bit server.

On fedora with gcc (GCC) 4.3.0 20080428 (Red Hat 4.3.0-8) checkpatch reports 33 functions with reduced stack usage.
e.g.
__nfs_revalidate_inode [nfs] 216 => 200
_nfs4_proc_access [nfs] 304 => 288
_nfs4_proc_link [nfs] 536 => 504
_nfs4_proc_remove [nfs] 304 => 288
_nfs4_proc_rename [nfs] 584 => 552
nfs3_proc_access [nfs] 272 => 256
nfs3_proc_getacl [nfs] 384 => 368
nfs3_proc_link [nfs] 496 => 464
etc
I can supply the complete list if anyone is interested.

regards
Richard

Signed-off-by Trond Myklebust <[email protected]>

---

 include/linux/nfs_xdr.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

include/linux/nfs_xdr.h +1 -1

index 8c77c11..9cabbb3 100644
@@ -36,6 +36,7 @@ struct nfs_fattr {
 	__u32			nlink;
 	__u32			uid;
 	__u32			gid;
+	dev_t			rdev;
 	__u64			size;
 	union {
 		struct {

@@ -46,7 +47,6 @@ struct nfs_fattr {
 			__u64	used;
 		} nfs3;
 	} du;
-	dev_t			rdev;
 	struct nfs_fsid		fsid;
 	__u64			fileid;
 	struct timespec		atime;

Trond Myklebust

author

2008-10-07 22:32 UTC #26 lore raw

PATCH 25/30 NFS: SETCLIENTID truncates client ID and netid

In reply to a message not in the archive

From: Chuck Lever <[email protected]>

The sc_name field is currently 56 bytes long. This is not large enough to hold a pair of IPv6 addresses, the authentication type, the protocol name, and a uniquifier number. The maximum possible size of the name string using IPv6 addresses is just under 110 bytes, so I increased the size of the sc_name field to accomodate this maximum.

In addition, the strings in the nfs4_setclientid structure are constructed with scnprintf(), which wants to terminate its output with '\0'. The sc_netid field was large enough only for a three byte netid string and a '\0' so inet6 netids were being truncated. Perhaps we don't need the overhead of scnprintf() to do a simple string copy, but I fixed this by increasing the size of the buffer by one byte.

Since all three of the string buffers in nfs4_setclientid are constructed with scnprintf(), I increased the size of all three by one byte to document the requirement, although I don't think either the universal address field or the name field will be so small that these strings get truncated in this way.

The size of the Linux client's client ID on the wire will be larger than before. RFC 3530 suggests the size limit for client IDs is 1024, and we are still well below that.

Signed-off-by Chuck Lever <[email protected]>

Signed-off-by Trond Myklebust <[email protected]>

---

 include/linux/nfs_xdr.h |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

include/linux/nfs_xdr.h +4 -4

index 9cabbb3..f6e95bf 100644
@@ -672,16 +672,16 @@ struct nfs4_rename_res {
 	struct nfs_fattr *		new_fattr;
 };
 
-#define NFS4_SETCLIENTID_NAMELEN	(56)
+#define NFS4_SETCLIENTID_NAMELEN	(128)
 struct nfs4_setclientid {
 	const nfs4_verifier *		sc_verifier;
 	unsigned int			sc_name_len;
-	char				sc_name[NFS4_SETCLIENTID_NAMELEN];
+	char				sc_name[NFS4_SETCLIENTID_NAMELEN + 1];
 	u32				sc_prog;
 	unsigned int			sc_netid_len;
-	char				sc_netid[RPCBIND_MAXNETIDLEN];
+	char				sc_netid[RPCBIND_MAXNETIDLEN + 1];
 	unsigned int			sc_uaddr_len;
-	char				sc_uaddr[RPCBIND_MAXUADDRLEN];
+	char				sc_uaddr[RPCBIND_MAXUADDRLEN + 1];
 	u32				sc_cb_ident;
 };

A message that is not in the archive (sent off-list or not yet received): 20081007221954.20945.76616.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/[email protected]

Chuck Lever

2008-10-08 14:55 UTC #28 lore raw

Re: [PATCH 25/30] NFS: SETCLIENTID truncates client ID and netid

Just a note...

Peter Staubach (I think) suggested NFS4_SETCLIENTID_NAMELEN should be a slightly smaller value to prevent the compiler from adding wasted space in the structure to align these fields.

If nothing else, you can safely make NFS4_SETCLIENTID_NAMELEN 127 instead of 128.

Quoted 48 lines · Trond Myklebust

From: Chuck Lever <[email protected]>
The sc_name field is currently 56 bytes long. This is not large enough to hold a pair of IPv6 addresses, the authentication type, the protocol name, and a uniquifier number. The maximum possible size of the name string using IPv6 addresses is just under 110 bytes, so I increased the size of the sc_name field to accomodate this maximum.
In addition, the strings in the nfs4_setclientid structure are constructed with scnprintf(), which wants to terminate its output with '\0'. The sc_netid field was large enough only for a three byte netid string and a '\0' so inet6 netids were being truncated. Perhaps we don't need the overhead of scnprintf() to do a simple string copy, but I fixed this by increasing the size of the buffer by one byte.
Since all three of the string buffers in nfs4_setclientid are constructed with scnprintf(), I increased the size of all three by one byte to document the requirement, although I don't think either the universal address field or the name field will be so small that these strings get truncated in this way.
The size of the Linux client's client ID on the wire will be larger than before. RFC 3530 suggests the size limit for client IDs is 1024, and we are still well below that.
Signed-off-by Chuck Lever <[email protected]>
Signed-off-by Trond Myklebust <[email protected]>
---
include/linux/nfs_xdr.h | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
include/linux/nfs_xdr.h +2 -2
index 9cabbb3..f6e95bf 100644
@@ -672,16 +672,16 @@ struct nfs4_rename_res {
 	struct nfs_fattr *		new_fattr;
 };
 
-#define NFS4_SETCLIENTID_NAMELEN	(56)
+#define NFS4_SETCLIENTID_NAMELEN	(128)
 struct nfs4_setclientid {
 	const nfs4_verifier *		sc_verifier;
 	unsigned int			sc_name_len;
-	char				sc_name[NFS4_SETCLIENTID_NAMELEN];
+	char				sc_name[NFS4_SETCLIENTID_NAMELEN + 1];

include/linux/nfs_xdr.h struct nfs4_rename_res { · line 687

Quoted 10 lines

include/linux/nfs_xdr.h +2 -2

 	u32				sc_prog;
 	unsigned int			sc_netid_len;
-	char				sc_netid[RPCBIND_MAXNETIDLEN];
+	char				sc_netid[RPCBIND_MAXNETIDLEN + 1];
 	unsigned int			sc_uaddr_len;
-	char				sc_uaddr[RPCBIND_MAXUADDRLEN];
+	char				sc_uaddr[RPCBIND_MAXUADDRLEN + 1];
 	u32				sc_cb_ident;
 };

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Trond Myklebust

author

2008-10-08 17:56 UTC #29 lore raw

Re: [PATCH 25/30] NFS: SETCLIENTID truncates client ID and netid

Quoted 8 lines · Chuck Lever

Just a note...
Peter Staubach (I think) suggested NFS4_SETCLIENTID_NAMELEN should be a slightly smaller value to prevent the compiler from adding wasted space in the structure to align these fields.
If nothing else, you can safely make NFS4_SETCLIENTID_NAMELEN 127 instead of 128.

Done...
---------------------------------------------------------------------
From: Trond Myklebust <[email protected]>
Date: Wed, 8 Oct 2008 13:54:52 -0400
NFS: Save padding bytes in struct nfs4_setclientid

Peter Staubach suggested reducing NFS4_SETCLIENTID_NAMELEN by one byte so as to avoid 7 bytes of unnecessary padding.

Signed-off-by Trond Myklebust <[email protected]>

---

 include/linux/nfs_xdr.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

include/linux/nfs_xdr.h +1 -1

index f6e95bf..6ee6ae3 100644
@@ -672,7 +672,7 @@ struct nfs4_rename_res {
 	struct nfs_fattr *		new_fattr;
 };
 
-#define NFS4_SETCLIENTID_NAMELEN	(128)
+#define NFS4_SETCLIENTID_NAMELEN	(127)
 struct nfs4_setclientid {
 	const nfs4_verifier *		sc_verifier;
 	unsigned int			sc_name_len;

Signature · 6 lines

Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
www.netapp.com

Chuck Lever

Acked-by

2008-10-15 15:36 UTC #30 lore raw

Re: [PATCH 25/30] NFS: SETCLIENTID truncates client ID and netid

Quoted 11 lines · Trond Myklebust

Quoted 8 lines · Chuck Lever
Just a note...
Peter Staubach (I think) suggested NFS4_SETCLIENTID_NAMELEN should be a slightly smaller value to prevent the compiler from adding wasted space in the structure to align these fields.
If nothing else, you can safely make NFS4_SETCLIENTID_NAMELEN 127 instead of 128.
Done...

Oh, in case you need it....

Acked-by Chuck Lever <[email protected]>

Quoted 36 lines

---------------------------------------------------------------------
From: Trond Myklebust <[email protected]>
Date: Wed, 8 Oct 2008 13:54:52 -0400
NFS: Save padding bytes in struct nfs4_setclientid
Peter Staubach suggested reducing NFS4_SETCLIENTID_NAMELEN by one byte so as to avoid 7 bytes of unnecessary padding.
Signed-off-by Trond Myklebust <[email protected]>
---
include/linux/nfs_xdr.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
include/linux/nfs_xdr.h +1 -1
index f6e95bf..6ee6ae3 100644
@@ -672,7 +672,7 @@ struct nfs4_rename_res {
 	struct nfs_fattr *		new_fattr;
 };
 
-#define NFS4_SETCLIENTID_NAMELEN	(128)
+#define NFS4_SETCLIENTID_NAMELEN	(127)
 struct nfs4_setclientid {
 	const nfs4_verifier *		sc_verifier;
 	unsigned int			sc_name_len;
Signature · 7 lines
-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
www.netapp.com

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Trond Myklebust

author

2008-10-07 22:32 UTC #31 lore raw

PATCH 26/30 SUNRPC: Fix autobind on cloned rpc clients

In reply to a message not in the archive

Despite the fact that cloned rpc clients won't have the cl_autobind flag set, they may still find themselves calling rpcb_getport_async(). For this to happen, it suffices for a _parent_ rpc_clnt to use autobinding, in which case any clone may find itself triggering the !xprt_bound() case in call_bind().

The correct fix for this is to walk back up the tree of cloned rpc clients, in order to find the parent that 'owns' the transport, either because it has clnt->cl_autobind set, or because it originally created the transport...

Signed-off-by Trond Myklebust <[email protected]>

---

 net/sunrpc/rpcb_clnt.c |   36 +++++++++++++++++++++++++++++-------
 1 files changed, 29 insertions(+), 7 deletions(-)

net/sunrpc/rpcb_clnt.c +29 -7

index 24db2b4..172935b 100644
@@ -469,6 +469,28 @@ static struct rpc_task *rpcb_call_async(struct rpc_clnt *rpcb_clnt, struct rpcbi
 	return rpc_run_task(&task_setup_data);
 }
 
+/*
+ * In the case where rpc clients have been cloned, we want to make
+ * sure that we use the program number/version etc of the actual
+ * owner of the xprt. To do so, we walk back up the tree of parents
+ * to find whoever created the transport and/or whoever has the
+ * autobind flag set.
+ */
+static struct rpc_clnt *rpcb_find_transport_owner(struct rpc_clnt *clnt)
+{
+	struct rpc_clnt *parent = clnt->cl_parent;
+
+	while (parent != clnt) {
+		if (parent->cl_xprt != clnt->cl_xprt)
+			break;
+		if (clnt->cl_autobind)
+			break;
+		clnt = parent;
+		parent = parent->cl_parent;
+	}
+	return clnt;
+}
+
 /**
  * rpcb_getport_async - obtain the port for a given RPC service on a given host
  * @task: task that is waiting for portmapper request

@@ -478,10 +500,10 @@ static struct rpc_task *rpcb_call_async(struct rpc_clnt *rpcb_clnt, struct rpcbi
  */
 void rpcb_getport_async(struct rpc_task *task)
 {
-	struct rpc_clnt *clnt = task->tk_client;
+	struct rpc_clnt *clnt;
 	struct rpc_procinfo *proc;
 	u32 bind_version;
-	struct rpc_xprt *xprt = task->tk_xprt;
+	struct rpc_xprt *xprt;
 	struct rpc_clnt	*rpcb_clnt;
 	static struct rpcbind_args *map;
 	struct rpc_task	*child;

@@ -490,13 +512,13 @@ void rpcb_getport_async(struct rpc_task *task)
 	size_t salen;
 	int status;
 
+	clnt = rpcb_find_transport_owner(task->tk_client);
+	xprt = clnt->cl_xprt;
+
 	dprintk("RPC: %5u %s(%s, %u, %u, %d)\n",
 		task->tk_pid, __func__,
 		clnt->cl_server, clnt->cl_prog, clnt->cl_vers, xprt->prot);
 
-	/* Autobind on cloned rpc clients is discouraged */
-	BUG_ON(clnt->cl_parent != clnt);
-
 	/* Put self on the wait queue to ensure we get notified if
 	 * some other task is already attempting to bind the port */
 	rpc_sleep_on(&xprt->binding, task, NULL);

@@ -578,9 +600,9 @@ void rpcb_getport_async(struct rpc_task *task)
 			task->tk_pid, __func__);
 		return;
 	}
-	rpc_put_task(child);
 
-	task->tk_xprt->stat.bind_count++;
+	xprt->stat.bind_count++;
+	rpc_put_task(child);
 	return;
 
 bailout_nofree:

Trond Myklebust

author

2008-10-07 22:32 UTC #32 lore raw

PATCH 27/30 SUNRPC: Fix a memory leak in rpcb_getport_async

In reply to a message not in the archive

Signed-off-by Trond Myklebust <[email protected]>

---

 net/sunrpc/rpcb_clnt.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

net/sunrpc/rpcb_clnt.c +3 -1

index 172935b..0a22f00 100644
@@ -580,7 +580,7 @@ void rpcb_getport_async(struct rpc_task *task)
 		status = -ENOMEM;
 		dprintk("RPC: %5u %s: no memory available\n",
 			task->tk_pid, __func__);
-		goto bailout_nofree;
+		goto bailout_release_client;
 	}
 	map->r_prog = clnt->cl_prog;
 	map->r_vers = clnt->cl_vers;

@@ -605,6 +605,8 @@ void rpcb_getport_async(struct rpc_task *task)
 	rpc_put_task(child);
 	return;
 
+bailout_release_client:
+	rpc_release_client(rpcb_clnt);
 bailout_nofree:
 	rpcb_wake_rpcbind_waiters(xprt, status);
 	task->tk_status = status;

Trond Myklebust

author

2008-10-07 22:32 UTC #33 lore raw

PATCH 28/30 NFS: Client mounts hang when exported directory do not exist

In reply to a message not in the archive

From: Steve Dickson <[email protected]>

This patch fixes a regression that was introduced by the string based mounts.

nfs_mount() statically returns -EACCES for every error returned by the remote mounted. This is incorrect because -EACCES is an non-fatal error to the mount.nfs command. This error causes mount.nfs to retry the mount even in the case when the exported directory does not exist.

This patch maps the errors returned by the remote mountd into valid errno values, exactly how it was done pre-string based mounts. By returning the correct errno enables mount.nfs to do the right thing.

Signed-off-by Steve Dickson <[email protected]>

[[email protected]: nfs_stat_to_errno() now correctly returns
negative errors, so remove the sign change.]

Signed-off-by Trond Myklebust <[email protected]>

---

 fs/nfs/mount_clnt.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

fs/nfs/mount_clnt.c +2 -1

index 779d2eb..086a683 100644
@@ -14,6 +14,7 @@
 #include <linux/sunrpc/clnt.h>
 #include <linux/sunrpc/sched.h>
 #include <linux/nfs_fs.h>
+#include "internal.h"
 
 #ifdef RPC_DEBUG
 # define NFSDBG_FACILITY	NFSDBG_MOUNT

@@ -98,7 +99,7 @@ out_call_err:
 
 out_mnt_err:
 	dprintk("NFS: MNT server returned result %d\n", result.status);
-	status = -EACCES;
+	status = nfs_stat_to_errno(result.status);
 	goto out;
 }

Trond Myklebust

author

2008-10-07 22:32 UTC #34 lore raw

PATCH 29/30 NFS: Don't use range_cyclic for data integrity syncs

In reply to a message not in the archive

It is more efficient to write linearly starting from the beginning of the file.

Signed-off-by Trond Myklebust <[email protected]>

---

 fs/nfs/write.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

fs/nfs/write.c +2 -1

index 3229e21..9f98458 100644
@@ -1427,8 +1427,9 @@ static int nfs_write_mapping(struct address_space *mapping, int how)
 		.bdi = mapping->backing_dev_info,
 		.sync_mode = WB_SYNC_NONE,
 		.nr_to_write = LONG_MAX,
+		.range_start = 0,
+		.range_end = LLONG_MAX,
 		.for_writepages = 1,
-		.range_cyclic = 1,
 	};
 	int ret;

Trond Myklebust

author

2008-10-07 22:32 UTC #35 lore raw

PATCH 30/30 sunrpc: fix oops in rpc_create when the mount namespace is unshared

In reply to a message not in the archive

From: Cedric Le Goater <clg-NmTC/[email protected]>

On a system with nfs mounts, if a task unshares its mount namespace, a oops can occur when the system is rebooted if the task is the last to unreference the nfs mount. It will try to create a rpc request using utsname() which has been invalidated by free_nsproxy().

The patch fixes the issue by using the global init_utsname() which is always valid. the capability of identifying rpc clients per uts namespace stills needs some extra work so this should not be a problem.

BUG: unable to handle kernel NULL pointer dereference at 00000004
IP: [<c024c9ab>] rpc_create+0x332/0x42f
Oops: 0000 [#1] DEBUG_PAGEALLOC

Pid: 1857, comm: uts-oops Not tainted (2.6.27-rc5-00319-g7686ad5 #4)
EIP: 0060:[<c024c9ab>] EFLAGS: 00210287 CPU: 0
EIP is at rpc_create+0x332/0x42f
EAX: 00000000 EBX: df26adf0 ECX: c0251887 EDX: 00000001
ESI: df26ae58 EDI: c02f293c EBP: dda0fc9c ESP: dda0fc2c
DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068
Process uts-oops (pid: 1857, ti=dda0e000 task=dd9a0778 task.ti=dda0e000)

Stack: c0104532 dda0fffc dda0fcac dda0e000 dda0e000 dd93b7f0 00000009 c02f2880
       df26aefc dda0fc68 c01096b7 00000000 c0266ee0 c039a070 c039a070 dda0fc74
       c012ca67 c039a064 dda0fc8c c012cb20 c03daf74 00000011 00000000 c0275c90
Call Trace:
 [<c0104532>] ? dump_trace+0xc2/0xe2
 [<c01096b7>] ? save_stack_trace+0x1c/0x3a
 [<c012ca67>] ? save_trace+0x37/0x8c
 [<c012cb20>] ? add_lock_to_list+0x64/0x96
 [<c0256fc4>] ? rpcb_register_call+0x62/0xbb
 [<c02570c8>] ? rpcb_register+0xab/0xb3
 [<c0252f4d>] ? svc_register+0xb4/0x128
 [<c0253114>] ? svc_destroy+0xec/0x103
 [<c02531b2>] ? svc_exit_thread+0x87/0x8d
 [<c01a75cd>] ? lockd_down+0x61/0x81
 [<c01a577b>] ? nlmclnt_done+0xd/0xf
 [<c01941fe>] ? nfs_destroy_server+0x14/0x16
 [<c0194328>] ? nfs_free_server+0x4c/0xaa
 [<c019a066>] ? nfs_kill_super+0x23/0x27
 [<c0158585>] ? deactivate_super+0x3f/0x51
 [<c01695d1>] ? mntput_no_expire+0x95/0xb4
 [<c016965b>] ? release_mounts+0x6b/0x7a
 [<c01696cc>] ? __put_mnt_ns+0x62/0x70
 [<c0127501>] ? free_nsproxy+0x25/0x80
 [<c012759a>] ? switch_task_namespaces+0x3e/0x43
 [<c01275a9>] ? exit_task_namespaces+0xa/0xc
 [<c0117fed>] ? do_exit+0x4fd/0x666
 [<c01181b3>] ? do_group_exit+0x5d/0x83
 [<c011fa8c>] ? get_signal_to_deliver+0x2c8/0x2e0
 [<c0102630>] ? do_notify_resume+0x69/0x700
 [<c011d85a>] ? do_sigaction+0x134/0x145
 [<c0127205>] ? hrtimer_nanosleep+0x8f/0xce
 [<c0126d1a>] ? hrtimer_wakeup+0x0/0x1c
 [<c0103488>] ? work_notifysig+0x13/0x1b
 =======================
Code: 70 20 68 cb c1 2c c0 e8 75 4e 01 00 8b 83 ac 00 00 00 59 3d 00 f0 ff ff 5f 77 63 eb 57 a1 00 80 2d c0 8b 80 a8 02 00 00 8d 73 68 <8b> 40 04 83 c0 45 e8 41 46 f7 ff ba 20 00 00 00 83 f8 21 0f 4c
EIP: [<c024c9ab>] rpc_create+0x332/0x42f SS:ESP 0068:dda0fc2c

Signed-off-by Cedric Le Goater <clg-NmTC/[email protected]>

Cc Chuck Lever <[email protected]>

Cc Trond Myklebust <[email protected]>

Cc "Eric W. Biederman" <[email protected]>

Cc "Serge E. Hallyn" <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Trond Myklebust <[email protected]>

---

 net/sunrpc/clnt.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

net/sunrpc/clnt.c +2 -2

index 76739e9..a59cdf4 100644
@@ -213,10 +213,10 @@ static struct rpc_clnt * rpc_new_client(const struct rpc_create_args *args, stru
 	}
 
 	/* save the nodename */
-	clnt->cl_nodelen = strlen(utsname()->nodename);
+	clnt->cl_nodelen = strlen(init_utsname()->nodename);
 	if (clnt->cl_nodelen > UNX_MAXNODENAME)
 		clnt->cl_nodelen = UNX_MAXNODENAME;
-	memcpy(clnt->cl_nodename, utsname()->nodename, clnt->cl_nodelen);
+	memcpy(clnt->cl_nodename, init_utsname()->nodename, clnt->cl_nodelen);
 	rpc_register_client(clnt);
 	return clnt;

J. Bruce Fields

2008-10-08 19:31 UTC #36 lore raw

In reply to a message not in the archive

Quoted 9 lines · Trond Myklebust

Most of the following patches are bugfixes:
There are a couple of spin locking fixes to follow up the BKL removal. There are some attribute update fixups, mainly to make the spin locking more efficient
A fix for the problem of autobind on cloned RPC clients
The one feature that has been added is the lookup revalidation mount option to allow clients to specify that they might not want to cache negative dentries, and that they might want strict dentry revalidation.

The version of the fs_locations fixes are slightly older than my most recent. Looking back through the archive, I think it may have been my mistake. Sorry!

There's a missing patch from Chuck that makes the ip address parsing a bit more paranoid. I'll resend that, followed by the newer versions of the other patches.

Aside from the missing patch from Chuck, the only difference between the versions you have and my latest are summarized below. it's just:

- a cosmetic change to set_port to address your complaints about blocks used just to define local variables, and
- changes to stop bothering to check for scope id's in v6 addresses.

So--could you just take the following patches, and throw out the 4 corresponding patches from me in your current tree?

--b.

fs/nfs/internal.h +9 ~~-12~~

index 625abae..14199c2 100644
@@ -277,24 +277,21 @@ unsigned int nfs_page_array_len(unsigned int base, size_t len)
 		PAGE_SIZE - 1) >> PAGE_SHIFT;
 }
 
-#define IPV6_SCOPE_DELIMITER	'%'
-
 /*
  * Set the port number in an address.  Be agnostic about the address
  * family.
  */
 static inline void nfs_set_port(struct sockaddr *sap, unsigned short port)
 {
+	struct sockaddr_in *ap = (struct sockaddr_in *)sap;
+	struct sockaddr_in6 *ap6 = (struct sockaddr_in6 *)sap;
+
 	switch (sap->sa_family) {
-	case AF_INET: {
-	      struct sockaddr_in *ap = (struct sockaddr_in *)sap;
-	      ap->sin_port = htons(port);
-	      break;
-	}
-	case AF_INET6: {
-	       struct sockaddr_in6 *ap = (struct sockaddr_in6 *)sap;
-	       ap->sin6_port = htons(port);
-	       break;
-	}
+	case AF_INET:
+		ap->sin_port = htons(port);
+		break;
+	case AF_INET6:
+		ap6->sin6_port = htons(port);
+		break;
 	}
 }

fs/nfs/nfs4namespace.c +0 -2

index 30befc3..13c37de 100644
@@ -118,8 +118,6 @@ static struct vfsmount *try_location(struct nfs_clone_mount *mountdata,
 
 		mountdata->addr = (struct sockaddr *)&addr;
 
-		if (memchr(buf->data, IPV6_SCOPE_DELIMITER, buf->len))
-			continue;
 		nfs_parse_ip_address(buf->data, buf->len,
 				mountdata->addr, &mountdata->addrlen);
 		if (mountdata->addr->sa_family == AF_UNSPEC)

fs/nfs/super.c +2 -0

index b173653..78c0dd9 100644
@@ -705,6 +705,8 @@ static void nfs_parse_ipv4_address(char *string, size_t str_len,
 	*addr_len = 0;
 }
 
+#define IPV6_SCOPE_DELIMITER	'%'
+
 #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
 static void nfs_parse_ipv6_scope_id(const char *string, const size_t str_len,
 				    const char *delim,

J. Bruce Fields

2008-10-08 19:38 UTC #37 lore raw

PATCH 1/5 NFS: fix nfs_parse_ip_address() corner case

From: Chuck Lever <[email protected]>

Bruce observed that nfs_parse_ip_address() will successfully parse an IPv6 address that looks like this:

"::1%"

A scope delimiter is present, but there is no scope ID following it. This is harmless, as it would simply set the scope ID to zero. However, in some cases we would like to flag this as an improperly formed address.

We are now also careful to reject addresses where garbage follows the address (up to the length of the string), instead of ignoring the non-address characters; and where the scope ID is nonsense (not a valid device name, but also not numeric). Before, both of these cases would result in a harmless zero scope ID.

Signed-off-by Chuck Lever <[email protected]>

Signed-off-by J. Bruce Fields <[email protected]>

---
 fs/nfs/super.c |   34 +++++++++++++++++++++++-----------
 1 files changed, 23 insertions(+), 11 deletions(-)

fs/nfs/super.c +23 ~~-11~~

index 9abcd2b..c989e71 100644
@@ -727,17 +727,21 @@ static void nfs_parse_ipv4_address(char *string, size_t str_len,
 #define IPV6_SCOPE_DELIMITER	'%'
 
 #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
-static void nfs_parse_ipv6_scope_id(const char *string, const size_t str_len,
-				    const char *delim,
-				    struct sockaddr_in6 *sin6)
+static int nfs_parse_ipv6_scope_id(const char *string, const size_t str_len,
+				   const char *delim,
+				   struct sockaddr_in6 *sin6)
 {
 	char *p;
 	size_t len;
 
-	if (!(ipv6_addr_type(&sin6->sin6_addr) & IPV6_ADDR_LINKLOCAL))
-		return ;
+	if ((string + str_len) == delim)
+		return 1;
+
 	if (*delim != IPV6_SCOPE_DELIMITER)
-		return;
+		return 0;
+
+	if (!(ipv6_addr_type(&sin6->sin6_addr) & IPV6_ADDR_LINKLOCAL))
+		return 0;
 
 	len = (string + str_len) - delim - 1;
 	p = kstrndup(delim + 1, len, GFP_KERNEL);

@@ -750,14 +754,20 @@ static void nfs_parse_ipv6_scope_id(const char *string, const size_t str_len,
 			scope_id = dev->ifindex;
 			dev_put(dev);
 		} else {
-			/* scope_id is set to zero on error */
-			strict_strtoul(p, 10, &scope_id);
+			if (strict_strtoul(p, 10, &scope_id) == 0) {
+				kfree(p);
+				return 0;
+			}
 		}
 
 		kfree(p);
+
 		sin6->sin6_scope_id = scope_id;
 		dfprintk(MOUNT, "NFS: IPv6 scope ID = %lu\n", scope_id);
+		return 1;
 	}
+
+	return 0;
 }
 
 static void nfs_parse_ipv6_address(char *string, size_t str_len,

@@ -773,9 +783,11 @@ static void nfs_parse_ipv6_address(char *string, size_t str_len,
 
 		sin6->sin6_family = AF_INET6;
 		*addr_len = sizeof(*sin6);
-		if (in6_pton(string, str_len, addr, IPV6_SCOPE_DELIMITER, &delim)) {
-			nfs_parse_ipv6_scope_id(string, str_len, delim, sin6);
-			return;
+		if (in6_pton(string, str_len, addr,
+					IPV6_SCOPE_DELIMITER, &delim) != 0) {
+			if (nfs_parse_ipv6_scope_id(string, str_len,
+							delim, sin6) != 0)
+				return;
 		}
 	}

Signature · 1 line

1.5.5.rc1

J. Bruce Fields

2008-10-08 19:38 UTC #38 lore raw

PATCH 2/5 nfs: break up nfs_follow_referral

This function is a little longer and more deeply nested than necessary.

Signed-off-by J. Bruce Fields <[email protected]>

---
 fs/nfs/nfs4namespace.c |   84 ++++++++++++++++++++++++++---------------------
 1 files changed, 46 insertions(+), 38 deletions(-)

fs/nfs/nfs4namespace.c +46 ~~-38~~

index b112857..956cbbc 100644
@@ -110,6 +110,48 @@ static inline int valid_ipaddr4(const char *buf)
 	return 0;
 }
 
+static struct vfsmount *try_location(struct nfs_clone_mount *mountdata,
+				     char *page, char *page2,
+				     const struct nfs4_fs_location *location)
+{
+	struct vfsmount *mnt = ERR_PTR(-ENOENT);
+	char *mnt_path;
+	unsigned int s = 0;
+
+	mnt_path = nfs4_pathname_string(&location->rootpath, page2, PAGE_SIZE);
+	if (IS_ERR(mnt_path))
+		return mnt;
+	mountdata->mnt_path = mnt_path;
+
+	while (s < location->nservers) {
+		struct sockaddr_in addr = {
+			.sin_family	= AF_INET,
+			.sin_port	= htons(NFS_PORT),
+		};
+
+		if (location->servers[s].len <= 0 ||
+		    valid_ipaddr4(location->servers[s].data) < 0) {
+			s++;
+			continue;
+		}
+
+		mountdata->hostname = location->servers[s].data;
+		addr.sin_addr.s_addr = in_aton(mountdata->hostname),
+		mountdata->addr = (struct sockaddr *)&addr;
+		mountdata->addrlen = sizeof(addr);
+
+		snprintf(page, PAGE_SIZE, "%s:%s",
+				mountdata->hostname,
+				mountdata->mnt_path);
+
+		mnt = vfs_kern_mount(&nfs4_referral_fs_type, 0, page, mountdata);
+		if (!IS_ERR(mnt))
+			break;
+		s++;
+	}
+	return mnt;
+}
+
 /**
  * nfs_follow_referral - set up mountpoint when hitting a referral on moved error
  * @mnt_parent - mountpoint of parent directory

@@ -128,7 +170,6 @@ static struct vfsmount *nfs_follow_referral(const struct vfsmount *mnt_parent,
 		.authflavor = NFS_SB(mnt_parent->mnt_sb)->client->cl_auth->au_flavor,
 	};
 	char *page = NULL, *page2 = NULL;
-	unsigned int s;
 	int loc, error;
 
 	if (locations == NULL || locations->nlocations <= 0)

@@ -153,9 +194,8 @@ static struct vfsmount *nfs_follow_referral(const struct vfsmount *mnt_parent,
 	}
 
 	loc = 0;
-	while (loc < locations->nlocations && IS_ERR(mnt)) {
+	while (loc < locations->nlocations) {
 		const struct nfs4_fs_location *location = &locations->locations[loc];
-		char *mnt_path;
 
 		if (location == NULL || location->nservers <= 0 ||
 		    location->rootpath.ncomponents == 0) {

@@ -163,41 +203,9 @@ static struct vfsmount *nfs_follow_referral(const struct vfsmount *mnt_parent,
 			continue;
 		}
 
-		mnt_path = nfs4_pathname_string(&location->rootpath, page2, PAGE_SIZE);
-		if (IS_ERR(mnt_path)) {
-			loc++;
-			continue;
-		}
-		mountdata.mnt_path = mnt_path;
-
-		s = 0;
-		while (s < location->nservers) {
-			struct sockaddr_in addr = {
-				.sin_family	= AF_INET,
-				.sin_port	= htons(NFS_PORT),
-			};
-
-			if (location->servers[s].len <= 0 ||
-			    valid_ipaddr4(location->servers[s].data) < 0) {
-				s++;
-				continue;
-			}
-
-			mountdata.hostname = location->servers[s].data;
-			addr.sin_addr.s_addr = in_aton(mountdata.hostname),
-			mountdata.addr = (struct sockaddr *)&addr;
-			mountdata.addrlen = sizeof(addr);
-
-			snprintf(page, PAGE_SIZE, "%s:%s",
-					mountdata.hostname,
-					mountdata.mnt_path);
-
-			mnt = vfs_kern_mount(&nfs4_referral_fs_type, 0, page, &mountdata);
-			if (!IS_ERR(mnt)) {
-				break;
-			}
-			s++;
-		}
+		mnt = try_location(&mountdata, page, page2, location);
+		if (!IS_ERR(mnt))
+			break;
 		loc++;
 	}

Signature · 1 line

1.5.5.rc1

J. Bruce Fields

2008-10-08 19:38 UTC #39 lore raw

PATCH 3/5 nfs: replace while loop by for loops in nfs_follow_referral

Whoever wrote this had a bizarre allergy to for loops.

Signed-off-by J. Bruce Fields <[email protected]>

---
 fs/nfs/nfs4namespace.c |   17 +++++------------
 1 files changed, 5 insertions(+), 12 deletions(-)

fs/nfs/nfs4namespace.c +5 ~~-12~~

index 956cbbc..6bcc569 100644
@@ -116,24 +116,22 @@ static struct vfsmount *try_location(struct nfs_clone_mount *mountdata,
 {
 	struct vfsmount *mnt = ERR_PTR(-ENOENT);
 	char *mnt_path;
-	unsigned int s = 0;
+	unsigned int s;
 
 	mnt_path = nfs4_pathname_string(&location->rootpath, page2, PAGE_SIZE);
 	if (IS_ERR(mnt_path))
 		return mnt;
 	mountdata->mnt_path = mnt_path;
 
-	while (s < location->nservers) {
+	for (s = 0; s < location->nservers; s++) {
 		struct sockaddr_in addr = {
 			.sin_family	= AF_INET,
 			.sin_port	= htons(NFS_PORT),
 		};
 
 		if (location->servers[s].len <= 0 ||
-		    valid_ipaddr4(location->servers[s].data) < 0) {
-			s++;
+		    valid_ipaddr4(location->servers[s].data) < 0)
 			continue;
-		}
 
 		mountdata->hostname = location->servers[s].data;
 		addr.sin_addr.s_addr = in_aton(mountdata->hostname),

@@ -147,7 +145,6 @@ static struct vfsmount *try_location(struct nfs_clone_mount *mountdata,
 		mnt = vfs_kern_mount(&nfs4_referral_fs_type, 0, page, mountdata);
 		if (!IS_ERR(mnt))
 			break;
-		s++;
 	}
 	return mnt;
 }

@@ -193,20 +190,16 @@ static struct vfsmount *nfs_follow_referral(const struct vfsmount *mnt_parent,
 		goto out;
 	}
 
-	loc = 0;
-	while (loc < locations->nlocations) {
+	for (loc = 0; loc < locations->nlocations; loc++) {
 		const struct nfs4_fs_location *location = &locations->locations[loc];
 
 		if (location == NULL || location->nservers <= 0 ||
-		    location->rootpath.ncomponents == 0) {
-			loc++;
+		    location->rootpath.ncomponents == 0)
 			continue;
-		}
 
 		mnt = try_location(&mountdata, page, page2, location);
 		if (!IS_ERR(mnt))
 			break;
-		loc++;
 	}
 
 out:

Signature · 1 line

1.5.5.rc1

J. Bruce Fields

2008-10-08 19:38 UTC #40 lore raw

PATCH 4/5 nfs: prepare to share nfs_set_port

We plan to use this function elsewhere.

Signed-off-by J. Bruce Fields <[email protected]>

---
 fs/nfs/internal.h |   18 ++++++++++++++++++
 fs/nfs/super.c    |   19 -------------------
 2 files changed, 18 insertions(+), 19 deletions(-)

fs/nfs/internal.h +18 -0

index 24241fc..926d8db 100644
@@ -276,3 +276,21 @@ unsigned int nfs_page_array_len(unsigned int base, size_t len)
 		PAGE_SIZE - 1) >> PAGE_SHIFT;
 }
 
+/*
+ * Set the port number in an address.  Be agnostic about the address
+ * family.
+ */
+static inline void nfs_set_port(struct sockaddr *sap, unsigned short port)
+{
+	struct sockaddr_in *ap = (struct sockaddr_in *)sap;
+	struct sockaddr_in6 *ap6 = (struct sockaddr_in6 *)sap;
+
+	switch (sap->sa_family) {
+	case AF_INET:
+		ap->sin_port = htons(port);
+		break;
+	case AF_INET6:
+		ap6->sin6_port = htons(port);
+		break;
+	}
+}

fs/nfs/super.c +0 ~~-19~~

index c989e71..9608797 100644
@@ -664,25 +664,6 @@ static void nfs_umount_begin(struct super_block *sb)
 }
 
 /*
- * Set the port number in an address.  Be agnostic about the address family.
- */
-static void nfs_set_port(struct sockaddr *sap, unsigned short port)
-{
-	switch (sap->sa_family) {
-	case AF_INET: {
-		struct sockaddr_in *ap = (struct sockaddr_in *)sap;
-		ap->sin_port = htons(port);
-		break;
-	}
-	case AF_INET6: {
-		struct sockaddr_in6 *ap = (struct sockaddr_in6 *)sap;
-		ap->sin6_port = htons(port);
-		break;
-	}
-	}
-}
-
-/*
  * Sanity-check a server address provided by the mount command.
  *
  * Address family must be initialized, and address must not be

Signature · 1 line

1.5.5.rc1

J. Bruce Fields

2008-10-08 19:38 UTC #41 lore raw

PATCH 5/5 nfs: Fix misparsing of nfsv4 fs_locations attribute

The code incorrectly assumes here that the server name (or ip address) is null-terminated. This can cause referrals to fail in some cases.

Also support ipv6 addresses.

Signed-off-by J. Bruce Fields <[email protected]>

---
 fs/nfs/internal.h      |    1 +
 fs/nfs/nfs4namespace.c |   42 ++++++++++++++++--------------------------
 fs/nfs/super.c         |    2 +-
 3 files changed, 18 insertions(+), 27 deletions(-)

fs/nfs/internal.h +1 -0

index 926d8db..14199c2 100644
@@ -153,6 +153,7 @@ extern void nfs4_clear_inode(struct inode *);
 void nfs_zap_acl_cache(struct inode *inode);
 
 /* super.c */
+void nfs_parse_ip_address(char *, size_t, struct sockaddr *, size_t *);
 extern struct file_system_type nfs_xdev_fs_type;
 #ifdef CONFIG_NFS_V4
 extern struct file_system_type nfs4_xdev_fs_type;

fs/nfs/nfs4namespace.c +16 ~~-26~~

66 lines not on this page · raw message

fs/nfs/super.c +1 -1

index 9608797..713c75a 100644
@@ -791,7 +791,7 @@ static void nfs_parse_ipv6_address(char *string, size_t str_len,
  * If there is a problem constructing the new sockaddr, set the address
  * family to AF_UNSPEC.
  */
-static void nfs_parse_ip_address(char *string, size_t str_len,
+void nfs_parse_ip_address(char *string, size_t str_len,
 				 struct sockaddr *sap, size_t *addr_len)
 {
 	unsigned int i, colons;

Signature · 1 line

1.5.5.rc1

Trond Myklebust

author

2008-10-08 19:38 UTC #42 lore raw

In reply to J. Bruce Fields

Quoted 22 lines · J. Bruce Fields

Quoted 9 lines · Trond Myklebust
Most of the following patches are bugfixes:
There are a couple of spin locking fixes to follow up the BKL removal. There are some attribute update fixups, mainly to make the spin locking more efficient
A fix for the problem of autobind on cloned RPC clients
The one feature that has been added is the lookup revalidation mount option to allow clients to specify that they might not want to cache negative dentries, and that they might want strict dentry revalidation.
The version of the fs_locations fixes are slightly older than my most recent. Looking back through the archive, I think it may have been my mistake. Sorry!
There's a missing patch from Chuck that makes the ip address parsing a bit more paranoid. I'll resend that, followed by the newer versions of the other patches.
Aside from the missing patch from Chuck, the only difference between the versions you have and my latest are summarized below. it's just:

Quoted 7 lines

- a cosmetic change to set_port to address your complaints about blocks used just to define local variables, and
- changes to stop bothering to check for scope id's in v6 addresses.
So--could you just take the following patches, and throw out the 4 corresponding patches from me in your current tree?

I'll split your patch into 2 and apply on top the existing tree.

Trond

Signature · 6 lines

Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
www.netapp.com

J. Bruce Fields

2008-10-08 19:39 UTC #43 lore raw

Quoted 28 lines · Trond Myklebust

Quoted 27 lines · J. Bruce Fields
Quoted 9 lines · Trond Myklebust
Most of the following patches are bugfixes:
There are a couple of spin locking fixes to follow up the BKL removal. There are some attribute update fixups, mainly to make the spin locking more efficient
A fix for the problem of autobind on cloned RPC clients
The one feature that has been added is the lookup revalidation mount option to allow clients to specify that they might not want to cache negative dentries, and that they might want strict dentry revalidation.
The version of the fs_locations fixes are slightly older than my most recent. Looking back through the archive, I think it may have been my mistake. Sorry!
There's a missing patch from Chuck that makes the ip address parsing a bit more paranoid. I'll resend that, followed by the newer versions of the other patches.
Aside from the missing patch from Chuck, the only difference between the versions you have and my latest are summarized below. it's just:
- a cosmetic change to set_port to address your complaints about blocks used just to define local variables, and
- changes to stop bothering to check for scope id's in v6 addresses.

Quoted 4 lines

Quoted 2 lines
So--could you just take the following patches, and throw out the 4 corresponding patches from me in your current tree?
I'll split your patch into 2 and apply on top the existing tree.

That'll work too, thanks; let me know if you'd like me to do that for you.

(And note also that patch #1, from Chuck, also needs to be applied.)

--b.