On Oct 7, 2009, at 7:43 PM, Trond Myklebust wrote:
On Wed, 2009-10-07 at 19:02 -0400, Chuck Lever wrote:
On Oct 7, 2009, at 6:22 PM, Trond Myklebust wrote:
On Wed, 2009-10-07 at 18:02 -0400, Chuck Lever wrote:
Clean up: re-arrange the cases in call_transmit_status() to make
the
code easier to read.
Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
---
net/sunrpc/clnt.c | 24 ++++++++++++------------
1 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 57f39b7..c26669c 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -1175,25 +1175,21 @@ call_transmit(struct rpc_task *task)
/*
* 5a. Handle cleanup after a transmission
+ * If we've been waiting on the socket's write_space()
+ * callback, or if the server is temporarily unreachable,
+ * continue holding the transport lock.
*/
static void
call_transmit_status(struct rpc_task *task)
{
+ dprint_status(task);
+
task->tk_action = call_status;
+
switch (task->tk_status) {
- case -EAGAIN:
- break;
- default:
- xprt_end_transmit(task);
- rpc_task_force_reencode(task);
+ case -EAGAIN: /* no write space */
break;
- /*
- * Special cases: if we've been waiting on the
- * socket's write_space() callback, or if the
- * socket just returned a connection error,
- * then hold onto the transport lock.
- */
- case -ECONNREFUSED:
+ case -ECONNREFUSED: /* connection problems */
case -ECONNRESET:
case -ENOTCONN:
case -EHOSTDOWN:
@@ -1206,6 +1202,10 @@ call_transmit_status(struct rpc_task *task)
break;
}
rpc_task_force_reencode(task);
+ break;
+ default: /* success, or some other error */
+ xprt_end_transmit(task);
+ rpc_task_force_reencode(task);
}
}
This puts the most common case (success) at the very end of the
switch
statement. Most compilers will generate the most efficient code when
it
is at the very beginning...
I dropped this one, since it's not that important.
But I wouldn't expect the compiler could optimize the default: case
the way you described. No matter what order you write these in, the
code has to check each of the other cases first before deciding to
execute the default: arm. If it's that important, maybe we should
add
a "case 0:" arm?
Yes. That's probably a good idea...
Well, after looking at the actual evidence (ie objdump output)...
Unfortunately, adding "case 0:" doesn't seem to change the object
code. Nothing I tried seemed to prevent the compiler from placing the
zero case at the end of the function. I had to add
if (task->tk_status == 0) {
xprt_end_transmit(task);
rpc_task_force_reencode(task);
return;
}
switch (task->tk_status) {
...
before the compiler would put the check for zero status first.
Hopefully, open coding it would make the arrangement permanent,
regardless of any compiler optimizations.
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
