From 4c03e42f91b36a0bf59398b0f649aa0b31b02975 Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Wed, 26 Oct 2016 16:04:51 -0500 Subject: [PATCH] rx: Add rxi_FlushWriteLocked Currently, a couple of places in Rx do this: MUTEX_EXIT(&call->lock); rxi_FlushWrite(call); MUTEX_ENTER(&call->lock); This is a little silly, because if rxi_FlushWrite has anything to do, it just acquires/drops call->lock again. This seems like a very minor performance penalty, but in the right situation it can become more noticeable. Specifically, when an Rx call on the server ends successfully, rx_EndCall will rxi_FlushWrite (to send out the last Rx packet to the client) before marking the call as finished. If the client receives the last Rx packet and starts a new Rx call on the same channel before the server locks the call again, the client can receive a BUSY packet (because it looks like the previous call on the server hasn't finished yet). Nothing breaks, but this means the client waits 3 seconds to retry. This situation can probably happen with various rates of success in almost any situation, but I can see it consistently happen with 'vos move' when running 'vos' on the same machine as the source fileserver. It is most noticeable when moving a large number of small volumes (since you must wait an extra 3+ seconds per volume, where nothing is happening). To avoid this, create a new variant of rxi_FlushWrite, called rxi_FlushWriteLocked. This just assumes the call lock is already held by the caller, and avoids one extra lock/unlock pair. This is not the only place where we unlock/lock the call during the rx_EndCall situation described above, but it seems to be easiest to solve, and it's enough (for me) to avoid the 3-second delay in the 'vos move' scenario. Ideally, Rx should be able to atomically 'end' a call while sending out this last packet, but for now, this commit is easy to do. Note that rxi_FlushWrite previously didn't do much of note before locking the call. It did call rxi_FreePackets without the call lock, but calling that with the call lock is also fine; other callers do that. Change-Id: I8f71e86f6c1f6019abea21c205d2b26b7da0d808 Reviewed-on: https://gerrit.openafs.org/12421 Reviewed-by: Benjamin Kaduk Tested-by: BuildBot --- src/rx/rx.c | 4 +--- src/rx/rx_prototypes.h | 1 + src/rx/rx_rdwr.c | 34 +++++++++++++++++++++++++--------- 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/rx/rx.c b/src/rx/rx.c index e4ac431..be9949b 100644 --- a/src/rx/rx.c +++ b/src/rx/rx.c @@ -2371,9 +2371,7 @@ rx_EndCall(struct rx_call *call, afs_int32 rc) MUTEX_ENTER(&call->lock); } if (call->app.mode == RX_MODE_SENDING) { - MUTEX_EXIT(&call->lock); - rxi_FlushWrite(call); - MUTEX_ENTER(&call->lock); + rxi_FlushWriteLocked(call); } rxi_calltrace(RX_CALL_END, call); /* Call goes to hold state until reply packets are acknowledged */ diff --git a/src/rx/rx_prototypes.h b/src/rx/rx_prototypes.h index 68beade..8af5308 100644 --- a/src/rx/rx_prototypes.h +++ b/src/rx/rx_prototypes.h @@ -481,6 +481,7 @@ extern int rxi_WritevProc(struct rx_call *call, struct iovec *iov, int nio, extern int rx_WritevProc(struct rx_call *call, struct iovec *iov, int nio, int nbytes); extern void rxi_FlushWrite(struct rx_call *call); +extern void rxi_FlushWriteLocked(struct rx_call *call); extern void rx_FlushWrite(struct rx_call *call); diff --git a/src/rx/rx_rdwr.c b/src/rx/rx_rdwr.c index 4b33a14..4124658 100644 --- a/src/rx/rx_rdwr.c +++ b/src/rx/rx_rdwr.c @@ -189,9 +189,7 @@ rxi_ReadProc(struct rx_call *call, char *buf, return 0; } if (call->app.mode == RX_MODE_SENDING) { - MUTEX_EXIT(&call->lock); - rxi_FlushWrite(call); - MUTEX_ENTER(&call->lock); + rxi_FlushWriteLocked(call); continue; } } @@ -1223,12 +1221,13 @@ rx_WritevProc(struct rx_call *call, struct iovec *iov, int nio, int nbytes) } /* Flush any buffered data to the stream, switch to read mode - * (clients) or to EOF mode (servers) + * (clients) or to EOF mode (servers). If 'locked' is nonzero, call->lock must + * be already held. * * LOCKS HELD: called at netpri. */ -void -rxi_FlushWrite(struct rx_call *call) +static void +FlushWrite(struct rx_call *call, int locked) { struct rx_packet *cp = NULL; @@ -1259,7 +1258,10 @@ rxi_FlushWrite(struct rx_call *call) } #endif - MUTEX_ENTER(&call->lock); + if (!locked) { + MUTEX_ENTER(&call->lock); + } + if (call->error) call->app.mode = RX_MODE_ERROR; @@ -1307,10 +1309,24 @@ rxi_FlushWrite(struct rx_call *call) if (!(call->flags & RX_CALL_FAST_RECOVER)) { rxi_Start(call, 0); } - MUTEX_EXIT(&call->lock); + if (!locked) { + MUTEX_EXIT(&call->lock); + } } } +void +rxi_FlushWrite(struct rx_call *call) +{ + FlushWrite(call, 0); +} + +void +rxi_FlushWriteLocked(struct rx_call *call) +{ + FlushWrite(call, 1); +} + /* Flush any buffered data to the stream, switch to read mode * (clients) or to EOF mode (servers) */ void @@ -1318,6 +1334,6 @@ rx_FlushWrite(struct rx_call *call) { SPLVAR; NETPRI; - rxi_FlushWrite(call); + FlushWrite(call, 0); USERPRI; } -- 1.9.4