mirror of
https://git.openafs.org/openafs.git
synced 2025-02-01 05:57:43 +00:00
rx: dec rx_nWaiting on clearing RX_CALL_WAIT_PROC
Currently, a couple of callers (rxi_ResetCall, and rxi_AttachServerProc) will decrement rx_nWaiting only if RX_CALL_WAIT_PROC is set for a call, and the call is on a queue (presumably rx_incomingCallQueue). This can cause an imbalance in rx_nWaiting if these code paths are reached when, in another thread, rx_GetCall has removed the call from its queue, but it has not yet cleared RX_CALL_WAIT_PROC (this can happen while it is waiting for call->lock). In this situation, rx_GetCall will remove the call from its queue, wait, and e.g. rxi_ResetCall will clear RX_CALL_WAIT_PROC; neither will decrement rx_nWaiting. This is possible if a new call is started on a call channel with an extant call that is waiting for a thread; we will rxi_ResetCall in rxi_ReceivePacket, but rx_GetCall may be running at the same time. This race may also be possible via rxi_AttachServerProc via rxi_UpdatePeerReach -> TryAttach -> rxi_AttachServerProc while rx_GetCall is running, but I'm not sure. To avoid this, decrement rx_nWaiting based on RX_CALL_WAIT_PROC alone, regardless of whether or not the call is on a queue. This mirrors the incrementing rx_nWaiting behavior, where rx_nWaiting is only incremented if RX_CALL_WAIT_PROC is unset for a call, so this should guarantee that rx_nWaiting does not become unbalanced. Backport of commit 660720d1f54a867e21f78b6ec4c024235e4c37b7 Change-Id: I3372e053d284e10702971769487a7580a6842ef2 Reviewed-on: http://gerrit.openafs.org/8015 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Derrick Brashear <shadow@dementix.org>
This commit is contained in:
parent
339f8452e9
commit
55dbeea076
20
src/rx/rx.c
20
src/rx/rx.c
@ -4922,12 +4922,11 @@ rxi_AttachServerProc(struct rx_call *call,
|
||||
if (call->flags & RX_CALL_WAIT_PROC) {
|
||||
/* Conservative: I don't think this should happen */
|
||||
call->flags &= ~RX_CALL_WAIT_PROC;
|
||||
MUTEX_ENTER(&rx_waiting_mutex);
|
||||
rx_nWaiting--;
|
||||
MUTEX_EXIT(&rx_waiting_mutex);
|
||||
if (queue_IsOnQueue(call)) {
|
||||
queue_Remove(call);
|
||||
|
||||
MUTEX_ENTER(&rx_waiting_mutex);
|
||||
rx_nWaiting--;
|
||||
MUTEX_EXIT(&rx_waiting_mutex);
|
||||
}
|
||||
}
|
||||
call->state = RX_STATE_ACTIVE;
|
||||
@ -5429,6 +5428,11 @@ rxi_ResetCall(struct rx_call *call, int newcall)
|
||||
osi_rxWakeup(&call->twind);
|
||||
#endif
|
||||
|
||||
if (flags & RX_CALL_WAIT_PROC) {
|
||||
MUTEX_ENTER(&rx_stats_mutex);
|
||||
rx_nWaiting--;
|
||||
MUTEX_EXIT(&rx_stats_mutex);
|
||||
}
|
||||
#ifdef RX_ENABLE_LOCKS
|
||||
/* The following ensures that we don't mess with any queue while some
|
||||
* other thread might also be doing so. The call_queue_lock field is
|
||||
@ -5443,12 +5447,6 @@ rxi_ResetCall(struct rx_call *call, int newcall)
|
||||
MUTEX_ENTER(call->call_queue_lock);
|
||||
if (queue_IsOnQueue(call)) {
|
||||
queue_Remove(call);
|
||||
if (flags & RX_CALL_WAIT_PROC) {
|
||||
|
||||
MUTEX_ENTER(&rx_waiting_mutex);
|
||||
rx_nWaiting--;
|
||||
MUTEX_EXIT(&rx_waiting_mutex);
|
||||
}
|
||||
}
|
||||
MUTEX_EXIT(call->call_queue_lock);
|
||||
CLEAR_CALL_QUEUE_LOCK(call);
|
||||
@ -5456,8 +5454,6 @@ rxi_ResetCall(struct rx_call *call, int newcall)
|
||||
#else /* RX_ENABLE_LOCKS */
|
||||
if (queue_IsOnQueue(call)) {
|
||||
queue_Remove(call);
|
||||
if (flags & RX_CALL_WAIT_PROC)
|
||||
rx_nWaiting--;
|
||||
}
|
||||
#endif /* RX_ENABLE_LOCKS */
|
||||
|
||||
|
Loading…
x
Reference in New Issue
Block a user