Skip to contentSkip to conversation

Resent of two NFS patches.

PATCH New NeilBrown 1 participant 3 messages Nov 30, 2011

Subsystems (3)
lore
Share & tools
lore

NeilBrown

author
#0 lore raw

cover Resent of two NFS patches.

Hi Trond,
Following are two patches have already been sent in some form, but don't seem to be in your tree yet.

First is a simple bug fix for a recent breakage.

The second allows SOFTCONN connections to fail quickly on a fatal error (such as net-unreachable). You did comment on the first version and wondered about merging some similar functions, so this version does that merge.

Thanks,
NeilBrown

---

NeilBrown (2):
sunrpc: wake up SOFTCONN tasks when a connection error happens. NFS - fix recent breakage to NFS error handling.

 fs/nfs/file.c                  |    4 ++--
 include/linux/sunrpc/sched.h   |    7 +++++--
 net/sunrpc/auth_gss/auth_gss.c |    4 ++--
 net/sunrpc/rpcb_clnt.c         |    2 +-
 net/sunrpc/sched.c             |   39 +++++++++------------------------------
 net/sunrpc/xprt.c              |    2 +-
 net/sunrpc/xprtsock.c          |    6 +++++-
 7 files changed, 25 insertions(+), 39 deletions(-)
Signature · 1 line
Signature
author
#1 lore raw

PATCH 1/2 NFS - fix recent breakage to NFS error handling.

commit 02c24a82187d5a628c68edfe71ae60dc135cd178 made a small and presumably unintended change to write error handling in NFS.

Previously an error from filemap_write_and_wait_range would only be of interest if nfs_file_fsync did not return an error. After this commit, an error from filemap_write_and_wait_range would mean that nfs_file_fsync would not even be called.

This means that:
1/ you are more likely to see EIO than e.g. EDQUOT or ENOSPC. 2/ NFS_CONTEXT_ERROR_WRITE remains set for longer so more writes are
synchronous.

This patch restores previous behaviour.

---

 fs/nfs/file.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
fs/nfs/file.c +2 -2
index eca56d4..1d8e7bf 100644
@@ -272,13 +272,13 @@ nfs_file_fsync(struct file *file, loff_t start, loff_t end, int datasync)
 			datasync);
 
 	ret = filemap_write_and_wait_range(inode->i_mapping, start, end);
-	if (ret)
-		return ret;
 	mutex_lock(&inode->i_mutex);
 
 	nfs_inc_stats(inode, NFSIOS_VFSFSYNC);
 	have_error = test_and_clear_bit(NFS_CONTEXT_ERROR_WRITE, &ctx->flags);
 	status = nfs_commit_inode(inode, FLUSH_SYNC);
+	if (status >= 0 && ret < 0)
+		status = ret;
 	have_error |= test_bit(NFS_CONTEXT_ERROR_WRITE, &ctx->flags);
 	if (have_error)
 		ret = xchg(&ctx->error, 0);
author
#2 lore raw

PATCH 2/2 sunrpc: wake up SOFTCONN tasks when a connection error happens.

In reply to NeilBrown

A 'SOFTCONN' task should fail if there is an error or a major timeout during connection.

However errors are currently converted into a timeout (60seconds for TCP) which is treated as a minor timeout and 3 of these are required before failure.

The result of this is that if you try to mount an NFSv4 filesystem (which doesn't require rpcbind and the failure modes that provides) from a server which you do not have a route to (an so get NETUNREACHABLE), you have an unnecessary 3 minutes timeout.

So when ENETUNREACH is reported for a connection - or other errors which are fatal, wake up any SOFTCONN tasks with that error - rather than letting them wait 60 seconds and then generate ETIMEDOUT.

This causes the above mentioned mount attempt to fail instantly.

This patch unifies rpc_wake_up and rpc_wake_up_status and adds a 'flag' option so we can just wake tasks with a given flag.

---

 include/linux/sunrpc/sched.h   |    7 +++++--
 net/sunrpc/auth_gss/auth_gss.c |    4 ++--
 net/sunrpc/rpcb_clnt.c         |    2 +-
 net/sunrpc/sched.c             |   39 +++++++++------------------------------
 net/sunrpc/xprt.c              |    2 +-
 net/sunrpc/xprtsock.c          |    6 +++++-
 6 files changed, 23 insertions(+), 37 deletions(-)
include/linux/sunrpc/sched.h +5 -2
index e775689..f4af625 100644
@@ -233,9 +233,12 @@ void		rpc_sleep_on_priority(struct rpc_wait_queue *,
 					int priority);
 void		rpc_wake_up_queued_task(struct rpc_wait_queue *,
 					struct rpc_task *);
-void		rpc_wake_up(struct rpc_wait_queue *);
+void		rpc_wake_up_status(struct rpc_wait_queue *, int, int);
+static inline void rpc_wake_up(struct rpc_wait_queue *q)
+{
+	rpc_wake_up_status(q, 0, 0);
+}
 struct rpc_task *rpc_wake_up_next(struct rpc_wait_queue *);
-void		rpc_wake_up_status(struct rpc_wait_queue *, int);
 int		rpc_queue_empty(struct rpc_wait_queue *);
 void		rpc_delay(struct rpc_task *, unsigned long);
 void *		rpc_malloc(struct rpc_task *, size_t);
net/sunrpc/auth_gss/auth_gss.c +2 -2
index afb5655..15701d9 100644
@@ -334,7 +334,7 @@ static void
 __gss_unhash_msg(struct gss_upcall_msg *gss_msg)
 {
 	list_del_init(&gss_msg->list);
-	rpc_wake_up_status(&gss_msg->rpc_waitqueue, gss_msg->msg.errno);
+	rpc_wake_up_status(&gss_msg->rpc_waitqueue, gss_msg->msg.errno, 0);
 	wake_up_all(&gss_msg->waitqueue);
 	atomic_dec(&gss_msg->count);
 }
@@ -367,7 +367,7 @@ gss_handle_downcall_result(struct gss_cred *gss_cred, struct gss_upcall_msg *gss
 	}
 	gss_cred->gc_upcall_timestamp = jiffies;
 	gss_cred->gc_upcall = NULL;
-	rpc_wake_up_status(&gss_msg->rpc_waitqueue, gss_msg->msg.errno);
+	rpc_wake_up_status(&gss_msg->rpc_waitqueue, gss_msg->msg.errno, 0);
 }
 
 static void
net/sunrpc/rpcb_clnt.c +1 -1
index 8761bf8..aadcf24 100644
@@ -151,7 +151,7 @@ static const struct rpc_call_ops rpcb_getport_ops = {
 static void rpcb_wake_rpcbind_waiters(struct rpc_xprt *xprt, int status)
 {
 	xprt_clear_binding(xprt);
-	rpc_wake_up_status(&xprt->binding, status);
+	rpc_wake_up_status(&xprt->binding, status, 0);
 }
 
 static void rpcb_map_release(void *data)
net/sunrpc/sched.c +9 -30
index d12ffa5..02617b8 100644
@@ -493,12 +493,14 @@ struct rpc_task * rpc_wake_up_next(struct rpc_wait_queue *queue)
 EXPORT_SYMBOL_GPL(rpc_wake_up_next);
 
 /**
- * rpc_wake_up - wake up all rpc_tasks
+ * rpc_wake_up_status - wake up some or all rpc_tasks
  * @queue: rpc_wait_queue on which the tasks are sleeping
+ * @status: status to set, or zero
+ * @flag:  only wake tasks with this flag set.
  *
  * Grabs queue->lock
  */
-void rpc_wake_up(struct rpc_wait_queue *queue)
+void rpc_wake_up_status(struct rpc_wait_queue *queue, int status, int flag)
 {
 	struct rpc_task *task, *next;
 	struct list_head *head;
@@ -507,34 +509,11 @@ void rpc_wake_up(struct rpc_wait_queue *queue)
 	head = &queue->tasks[queue->maxpriority];
 	for (;;) {
 		list_for_each_entry_safe(task, next, head, u.tk_wait.list)
-			rpc_wake_up_task_queue_locked(queue, task);
-		if (head == &queue->tasks[0])
-			break;
-		head--;
-	}
-	spin_unlock_bh(&queue->lock);
-}
-EXPORT_SYMBOL_GPL(rpc_wake_up);
-
-/**
- * rpc_wake_up_status - wake up all rpc_tasks and set their status value.
- * @queue: rpc_wait_queue on which the tasks are sleeping
- * @status: status value to set
- *
- * Grabs queue->lock
- */
-void rpc_wake_up_status(struct rpc_wait_queue *queue, int status)
-{
-	struct rpc_task *task, *next;
-	struct list_head *head;
-
-	spin_lock_bh(&queue->lock);
-	head = &queue->tasks[queue->maxpriority];
-	for (;;) {
-		list_for_each_entry_safe(task, next, head, u.tk_wait.list) {
-			task->tk_status = status;
-			rpc_wake_up_task_queue_locked(queue, task);
-		}
+			if (flag == 0 || (task->tk_flags & flag)) {
+				if (status)
+					task->tk_status = status;
+				rpc_wake_up_task_queue_locked(queue, task);
+			}
 		if (head == &queue->tasks[0])
 			break;
 		head--;
net/sunrpc/xprt.c +1 -1
index f4385e4..1603bb9 100644
@@ -471,7 +471,7 @@ EXPORT_SYMBOL_GPL(xprt_adjust_cwnd);
 void xprt_wake_pending_tasks(struct rpc_xprt *xprt, int status)
 {
 	if (status < 0)
-		rpc_wake_up_status(&xprt->pending, status);
+		rpc_wake_up_status(&xprt->pending, status, 0);
 	else
 		rpc_wake_up(&xprt->pending);
 }
net/sunrpc/xprtsock.c +5 -1
index 55472c4..d1bb5d4 100644
@@ -2157,7 +2157,11 @@ static void xs_tcp_setup_socket(struct work_struct *work)
 	case -ECONNREFUSED:
 	case -ECONNRESET:
 	case -ENETUNREACH:
-		/* retry with existing socket, after a delay */
+		/* Retry with existing socket after a delay, except
+		 * for SOFTCONN tasks which fail. */
+		xprt_clear_connecting(xprt);
+		rpc_wake_up_status(&xprt->pending, status, RPC_TASK_SOFTCONN);
+		return;
 	case 0:
 	case -EINPROGRESS:
 	case -EALREADY: