From: Eric Blossom Date: Fri, 4 Sep 2009 08:51:29 +0000 (-0700) Subject: Fix race condition that caused commands such as stop_rx_streaming to fail. X-Git-Url: https://git.gag.com/?a=commitdiff_plain;h=5d040bc94b40cab5420303f959695d89fe83e031;p=debian%2Fgnuradio Fix race condition that caused commands such as stop_rx_streaming to fail. This fixes the bulk of the problem. Next step is to drop data packets while waiting for the reply. --- diff --git a/usrp2/host/lib/control.cc b/usrp2/host/lib/control.cc index 4b8597c6..bb71f79c 100644 --- a/usrp2/host/lib/control.cc +++ b/usrp2/host/lib/control.cc @@ -30,26 +30,34 @@ namespace usrp2 { pending_reply::pending_reply(unsigned int rid, void *buffer, size_t len) - : d_rid(rid), d_mutex(), d_cond(&d_mutex), d_buffer(buffer), d_len(len) + : d_rid(rid), d_buffer(buffer), d_len(len), d_mutex(), d_cond(&d_mutex), + d_complete(false) { } pending_reply::~pending_reply() { - signal(); // Needed? + notify_completion(); // Needed? } int - pending_reply::wait(double secs) + pending_reply::wait_for_completion(double secs) { - omni_mutex_lock l(d_mutex); omni_time abs_timeout = omni_time::time(omni_time(secs)); - return d_cond.timedwait(abs_timeout.d_secs, abs_timeout.d_nsecs); + omni_mutex_lock l(d_mutex); + while (!d_complete){ + int r = d_cond.timedwait(abs_timeout.d_secs, abs_timeout.d_nsecs); + if (r == 0) // timed out + return 0; + } + return 1; } void - pending_reply::signal() + pending_reply::notify_completion() { + omni_mutex_lock l(d_mutex); + d_complete = true; d_cond.signal(); } diff --git a/usrp2/host/lib/control.h b/usrp2/host/lib/control.h index 9adc1618..8769e452 100644 --- a/usrp2/host/lib/control.h +++ b/usrp2/host/lib/control.h @@ -118,10 +118,13 @@ namespace usrp2 { { private: unsigned int d_rid; - omni_mutex d_mutex; - omni_condition d_cond; void *d_buffer; size_t d_len; + + // d_mutex is used with d_cond and also protects d_complete + omni_mutex d_mutex; + omni_condition d_cond; + bool d_complete; public: /*! @@ -140,12 +143,12 @@ namespace usrp2 { * Returns: 1 = ok, reply packet in buffer * 0 = timeout */ - int wait(double secs); + int wait_for_completion(double secs); /*! * Allows creating thread to resume after copying reply into buffer */ - void signal(); + void notify_completion(); /*! * Retrieve pending reply ID diff --git a/usrp2/host/lib/usrp2_impl.cc b/usrp2/host/lib/usrp2_impl.cc index 1ecfd734..b81b3cd6 100644 --- a/usrp2/host/lib/usrp2_impl.cc +++ b/usrp2/host/lib/usrp2_impl.cc @@ -298,22 +298,24 @@ namespace usrp2 { cmd->eop.len = sizeof(cmd->eop); } + + bool + usrp2::impl::transmit_cmd(void *cmd, size_t len) + { + return d_eth_buf->tx_frame(cmd, len) == eth_buffer::EB_OK; + } + bool - usrp2::impl::transmit_cmd(void *cmd, size_t len, pending_reply *p, double secs) + usrp2::impl::transmit_cmd_and_wait(void *cmd, size_t len, pending_reply *p, double secs) { - if (p) - d_pending_replies[p->rid()] = p; + d_pending_replies[p->rid()] = p; - // Transmit command - if (d_eth_buf->tx_frame(cmd, len) != eth_buffer::EB_OK) { + if (!transmit_cmd(cmd, len)){ d_pending_replies[p->rid()] = 0; return false; } - int res = 1; - if (p) - res = p->wait(secs); - + int res = p->wait_for_completion(secs); d_pending_replies[p->rid()] = 0; return res == 1; } @@ -404,7 +406,7 @@ namespace usrp2 { // Copy reply into caller's buffer memcpy(rp->buffer(), p, std::min(oplen, buflen)); - rp->signal(); + rp->notify_completion(); d_pending_replies[rid] = 0; return data_handler::RELEASE; } @@ -485,7 +487,7 @@ namespace usrp2 { cmd.op.gain = htons(u2_double_to_fxpt_gain(gain)); pending_reply p(cmd.op.rid, &reply, sizeof(reply)); - if (!transmit_cmd(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) + if (!transmit_cmd_and_wait(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) return false; bool success = (ntohx(reply.ok) == 1); @@ -512,7 +514,7 @@ namespace usrp2 { cmd.eop.len = sizeof(cmd.eop); pending_reply p(cmd.op.rid, &reply, sizeof(reply)); - if (!transmit_cmd(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) + if (!transmit_cmd_and_wait(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) return false; bool success = (ntohx(reply.ok) == 1); @@ -532,7 +534,7 @@ namespace usrp2 { cmd.op.freq_lo = htonl(u2_fxpt_freq_lo(fxpt)); pending_reply p(cmd.op.rid, &reply, sizeof(reply)); - if (!transmit_cmd(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) + if (!transmit_cmd_and_wait(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) return false; bool success = (ntohx(reply.ok) == 1); @@ -569,7 +571,7 @@ namespace usrp2 { cmd.op.decim = htonl(decimation_factor); pending_reply p(cmd.op.rid, &reply, sizeof(reply)); - if (!transmit_cmd(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) + if (!transmit_cmd_and_wait(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) return false; bool success = (ntohx(reply.ok) == 1); @@ -589,7 +591,7 @@ namespace usrp2 { cmd.op.scale_iq = htonl(((scale_i & 0xffff) << 16) | (scale_q & 0xffff)); pending_reply p(cmd.op.rid, &reply, sizeof(reply)); - if (!transmit_cmd(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) + if (!transmit_cmd_and_wait(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) return false; bool success = (ntohx(reply.ok) == 1); @@ -636,12 +638,13 @@ namespace usrp2 { bool success = false; pending_reply p(cmd.op.rid, &reply, sizeof(reply)); - success = transmit_cmd(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT); + success = transmit_cmd_and_wait(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT); success = success && (ntohx(reply.ok) == 1); if (success) d_channel_rings[channel] = ring_sptr(new ring(d_eth_buf->max_frames())); + //fprintf(stderr, "usrp2::start_rx_streaming: success = %d\n", success); return success; } } @@ -677,9 +680,10 @@ namespace usrp2 { bool success = false; pending_reply p(cmd.op.rid, &reply, sizeof(reply)); - success = transmit_cmd(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT); + success = transmit_cmd_and_wait(&cmd, sizeof(cmd), &p, 10 * DEF_CMD_TIMEOUT); success = success && (ntohx(reply.ok) == 1); d_channel_rings[channel].reset(); + //fprintf(stderr, "usrp2::stop_rx_streaming: success = %d\n", success); return success; } } @@ -747,7 +751,7 @@ namespace usrp2 { cmd.op.gain = htons(u2_double_to_fxpt_gain(gain)); pending_reply p(cmd.op.rid, &reply, sizeof(reply)); - if (!transmit_cmd(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) + if (!transmit_cmd_and_wait(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) return false; bool success = (ntohx(reply.ok) == 1); @@ -774,7 +778,7 @@ namespace usrp2 { cmd.eop.len = sizeof(cmd.eop); pending_reply p(cmd.op.rid, &reply, sizeof(reply)); - if (!transmit_cmd(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) + if (!transmit_cmd_and_wait(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) return false; bool success = (ntohx(reply.ok) == 1); @@ -794,7 +798,7 @@ namespace usrp2 { cmd.op.freq_lo = htonl(u2_fxpt_freq_lo(fxpt)); pending_reply p(cmd.op.rid, &reply, sizeof(reply)); - if (!transmit_cmd(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) + if (!transmit_cmd_and_wait(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) return false; bool success = (ntohx(reply.ok) == 1); @@ -831,7 +835,7 @@ namespace usrp2 { cmd.op.interp = htonl(interpolation_factor); pending_reply p(cmd.op.rid, &reply, sizeof(reply)); - if (!transmit_cmd(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) + if (!transmit_cmd_and_wait(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) return false; bool success = (ntohx(reply.ok) == 1); @@ -882,7 +886,7 @@ namespace usrp2 { cmd.op.scale_iq = htonl(((scale_i & 0xffff) << 16) | (scale_q & 0xffff)); pending_reply p(cmd.op.rid, &reply, sizeof(reply)); - if (!transmit_cmd(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) + if (!transmit_cmd_and_wait(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) return false; bool success = (ntohx(reply.ok) == 1); @@ -1010,7 +1014,7 @@ namespace usrp2 { cmd.eop.len = sizeof(cmd.eop); pending_reply p(cmd.op.rid, &reply, sizeof(reply)); - if (!transmit_cmd(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) + if (!transmit_cmd_and_wait(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) return false; return ntohx(reply.ok) == 1; @@ -1069,7 +1073,7 @@ namespace usrp2 { return false; pending_reply p(cmd.op.rid, &reply, sizeof(reply)); - if (!transmit_cmd(&cmd, sizeof(cmd), &p, 4*DEF_CMD_TIMEOUT)) + if (!transmit_cmd_and_wait(&cmd, sizeof(cmd), &p, 4*DEF_CMD_TIMEOUT)) return false; bool success = (ntohx(reply.ok) == 1); @@ -1108,7 +1112,7 @@ namespace usrp2 { cmd.eop.len = sizeof(cmd.eop); pending_reply p(cmd.op.rid, &reply, sizeof(reply)); - if (!transmit_cmd(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) + if (!transmit_cmd_and_wait(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) return false; bool success = (ntohx(reply.ok) == 1); @@ -1135,7 +1139,7 @@ namespace usrp2 { cmd.eop.len = sizeof(cmd.eop); pending_reply p(cmd.op.rid, &reply, sizeof(reply)); - if (!transmit_cmd(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) + if (!transmit_cmd_and_wait(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) return false; return ntohx(reply.ok) == 1; @@ -1157,7 +1161,7 @@ namespace usrp2 { cmd.eop.len = sizeof(cmd.eop); pending_reply p(cmd.op.rid, &reply, sizeof(reply)); - if (!transmit_cmd(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) + if (!transmit_cmd_and_wait(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) return false; return ntohx(reply.ok) == 1; @@ -1197,7 +1201,7 @@ namespace usrp2 { reply = (op_generic_t *)malloc(rlen+bytes); pending_reply p(cmd.op.rid, reply, rlen+bytes); - if (transmit_cmd(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) { + if (transmit_cmd_and_wait(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) { uint32_t nwords = (reply->len-rlen)/sizeof(uint32_t); uint32_t *data = (uint32_t *)(reply+rlen/wlen); for (unsigned int i = 0; i < nwords; i++) @@ -1264,7 +1268,7 @@ namespace usrp2 { bool ok = false; op_generic_t reply; pending_reply p(cmd->op.rid, &reply, sizeof(reply)); - if (transmit_cmd(cmd, l, &p, DEF_CMD_TIMEOUT)) + if (transmit_cmd_and_wait(cmd, l, &p, DEF_CMD_TIMEOUT)) ok = (ntohx(reply.ok) == 1); free(cmd); @@ -1286,7 +1290,7 @@ namespace usrp2 { cmd.eop.len = sizeof(cmd.eop); pending_reply p(cmd.op.rid, &reply, sizeof(reply)); - if (!transmit_cmd(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) + if (!transmit_cmd_and_wait(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) return false; bool success = (ntohx(reply.ok) == 1); @@ -1315,7 +1319,7 @@ namespace usrp2 { cmd.eop.len = sizeof(cmd.eop); pending_reply p(cmd.op.rid, &reply, sizeof(reply)); - if (!transmit_cmd(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) + if (!transmit_cmd_and_wait(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) return false; bool success = (ntohx(reply.ok) == 1); @@ -1348,7 +1352,7 @@ namespace usrp2 { cmd.eop.len = sizeof(cmd.eop); pending_reply p(cmd.op.rid, &reply, sizeof(reply)); - if (!transmit_cmd(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) + if (!transmit_cmd_and_wait(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) return false; bool success = (ntohx(reply.ok) == 1); @@ -1377,7 +1381,7 @@ namespace usrp2 { cmd.eop.len = sizeof(cmd.eop); pending_reply p(cmd.op.rid, &reply, sizeof(reply)); - if (!transmit_cmd(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) + if (!transmit_cmd_and_wait(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) return false; bool success = (ntohx(reply.ok) == 1); @@ -1406,7 +1410,7 @@ namespace usrp2 { cmd.eop.len = sizeof(cmd.eop); pending_reply p(cmd.op.rid, &reply, sizeof(reply)); - if (!transmit_cmd(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) + if (!transmit_cmd_and_wait(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) return false; bool success = (ntohx(reply.ok) == 1); @@ -1443,7 +1447,7 @@ namespace usrp2 { cmd.eop.len = sizeof(cmd.eop); pending_reply p(cmd.op.rid, &reply, sizeof(reply)); - if (!transmit_cmd(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) + if (!transmit_cmd_and_wait(&cmd, sizeof(cmd), &p, DEF_CMD_TIMEOUT)) return false; bool success = (ntohx(reply.ok) == 1); diff --git a/usrp2/host/lib/usrp2_impl.h b/usrp2/host/lib/usrp2_impl.h index ec96f3a7..434e0e9b 100644 --- a/usrp2/host/lib/usrp2_impl.h +++ b/usrp2/host/lib/usrp2_impl.h @@ -105,7 +105,8 @@ namespace usrp2 { void stop_bg(); void init_config_rx_v2_cmd(op_config_rx_v2_cmd *cmd); void init_config_tx_v2_cmd(op_config_tx_v2_cmd *cmd); - bool transmit_cmd(void *cmd, size_t len, pending_reply *p, double secs=0.0); + bool transmit_cmd_and_wait(void *cmd, size_t len, pending_reply *p, double secs=0.0); + bool transmit_cmd(void *cmd, size_t len); virtual data_handler::result operator()(const void *base, size_t len); data_handler::result handle_control_packet(const void *base, size_t len); data_handler::result handle_data_packet(const void *base, size_t len);