: Remove redundant synchronisation variable
PATCH 7/9 : Remove redundant synchronisation variable
This removes the synchronisation variable `ccid2hctx_sendwait', which is set to 1 when the CCID2 sender may send a new packet, and which is set to 0 otherwise
The variable is redundant, since it is only used in combination with the hc_tx_send_packet/ hc_tx_packet_sent function pair. Both functions are called under socket lock, so the following happens when the CCID2 may send a new packet:
* it sets sendwait = 1 in tx_send_packet and returns 0;
* the subsequent call to tx_packet_sent clears the sendwait flag;
* since tx_send_packet returns 0 if and only if sendwait = 1, the BUG_ON condition in tx_packet_sent is never satisfied, since that function is never called when tx_send_packet returns a value different from 0 (cf. dccp_write_xmit);
* the call to tx_packet_sent clears the flag so that the condition "!sendwait" is true the next time tx_packet_sent is called.
In other words, it is sufficient to just return 0 / not-0 to synchronise tx_send_packet and tx_packet_sent -- which is what the patch does.
--- net/dccp/ccids/ccid2.c | 11 ++--------- net/dccp/ccids/ccid2.h | 1 - 2 files changed, 2 insertions(+), 10 deletions(-)
net/dccp/ccids/ccid2.c +2 -9
@@ -126,13 +126,8 @@ static int ccid2_hc_tx_send_packet(struct sock *sk, struct sk_buff *skb) {struct ccid2_hc_tx_sock *hctx =ccid2_hc_tx_sk (sk); -if (hctx->ccid2hctx_pipe < hctx->ccid2hctx_cwnd) { -/* OK we can send... make sure previous packet was sent off */ -if (!hctx->ccid2hctx_sendwait) { - hctx->ccid2hctx_sendwait =1 ; -return 0 ; - } - } +if (hctx->ccid2hctx_pipe < hctx->ccid2hctx_cwnd) +return 0 ;return 1 ;/* XXX CCID should dequeue when ready instead of polling */ }
@@ -236,8 +231,6 @@ static void ccid2_hc_tx_packet_sent(struct sock *sk, int more, unsigned int len)struct ccid2_seq *next;u64 seq; -BUG_ON (!hctx->ccid2hctx_sendwait); - hctx->ccid2hctx_sendwait =0 ; hctx->ccid2hctx_pipe++;/* There is an issue. What if another packet is sent between
net/dccp/ccids/ccid2.h +0 -1
@@ -66,7 +66,6 @@ struct ccid2_hc_tx_sock {struct timer_list ccid2hctx_rtotimer;u64 ccid2hctx_rpseq;int ccid2hctx_rpdupack; -int ccid2hctx_sendwait;unsigned long ccid2hctx_last_cong;u64 ccid2hctx_high_ack; };
Signature · 1 line
1.5.2.2.238.g7cbf2f2-dirty
Quoted 15 lines · Gerrit Renker
This removes the synchronisation variable `ccid2hctx_sendwait', which is set to 1 when the CCID2 sender may send a new packet, and which is set to 0 otherwise
The variable is redundant, since it is only used in combination with the hc_tx_send_packet/
hc_tx_packet_sentfunction pair. Both functions are called under socket lock, so the following happens when the CCID2 may send a new packet:* it sets sendwait = 1 in
tx_send_packetand returns 0;
* the subsequent call totx_packet_sentclears the sendwait flag;
* sincetx_send_packetreturns 0 if and only if sendwait = 1, theBUG_ONcondition intx_packet_sentis never satisfied, since that function is never called whentx_send_packetreturns a value different from 0 (cf.dccp_write_xmit);
* the call totx_packet_sentclears the flag so that the condition "!sendwait" is true the next timetx_packet_sentis called.
Quoted 4 lines
In other words, it is sufficient to just return 0 / not-0 to synchronise
tx_send_packetandtx_packet_sent-- which is what the patch does.Signed-off-by Gerrit Renker <[email protected]>