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...
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust@xxxxxxxxxx
www.netapp.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
