From 597e7965a7bd9a0fc76a5acbaf260b05e42c474c Mon Sep 17 00:00:00 2001 From: Michael West Date: Fri, 8 Nov 2013 15:30:03 -0800 Subject: BUG #183: B200 High CPU Usage: Created a single thread to handle libusb events and expanded packet size to 16k --- host/lib/transport/libusb1_base.cpp | 19 +++++++ host/lib/transport/libusb1_zero_copy.cpp | 98 +++++++++++++++----------------- host/lib/usrp/b200/b200_io_impl.cpp | 4 +- 3 files changed, 66 insertions(+), 55 deletions(-) diff --git a/host/lib/transport/libusb1_base.cpp b/host/lib/transport/libusb1_base.cpp index 0ef53db0a..4cf3ea17d 100644 --- a/host/lib/transport/libusb1_base.cpp +++ b/host/lib/transport/libusb1_base.cpp @@ -19,10 +19,12 @@ #include #include #include +#include #include #include #include #include +#include #include #include @@ -37,9 +39,11 @@ public: libusb_session_impl(void){ UHD_ASSERT_THROW(libusb_init(&_context) == 0); libusb_set_debug(_context, debug_level); + task_handler = task::make(boost::bind(&libusb_session_impl::libusb_event_handler_task, this, _context)); } ~libusb_session_impl(void){ + task_handler.reset(); libusb_exit(_context); } @@ -49,6 +53,21 @@ public: private: libusb_context *_context; + task::sptr task_handler; + + /* + * Task to handle libusb events. There should only be one thread per libusb_context handling events. + * Using more than one thread can result in excessive CPU usage in kernel space (presumably from locking/waiting). + * The libusb documentation says it is safe, which it is, but it neglects to state the cost in CPU usage. + * Just don't do it! + */ + UHD_INLINE void libusb_event_handler_task(libusb_context *context) + { + timeval tv; + tv.tv_sec = 0; + tv.tv_usec = 100000; + libusb_handle_events_timeout(context, &tv); + } }; libusb::session::sptr libusb::session::get_global_session(void){ diff --git a/host/lib/transport/libusb1_zero_copy.cpp b/host/lib/transport/libusb1_zero_copy.cpp index 197e257da..7b1de65c3 100644 --- a/host/lib/transport/libusb1_zero_copy.cpp +++ b/host/lib/transport/libusb1_zero_copy.cpp @@ -73,6 +73,14 @@ struct lut_result_t int completed; libusb_transfer_status status; int actual_length; + boost::mutex mut; + boost::condition_variable wait_for_complete; +}; + +struct lut_result_completed { + lut_result_t& _result; + lut_result_completed(lut_result_t& result):_result(result) {} + bool operator()() const {return (_result.completed ? true : false);} }; /*! @@ -84,48 +92,11 @@ struct lut_result_t static void LIBUSB_CALL libusb_async_cb(libusb_transfer *lut) { lut_result_t *r = (lut_result_t *)lut->user_data; - r->completed = 1; + boost::lock_guard lock(r->mut); r->status = lut->status; r->actual_length = lut->actual_length; -} - -/*! - * Wait for a managed buffer to become complete. - * - * This routine processes async events until the transaction completes. - * We must call the libusb handle events in a loop because the handler - * may complete managed buffers other than the one we are waiting on. - * - * We cannot determine if handle events timed out or processed an event. - * Therefore, the timeout condition is handled by using boost system time. - * - * \param ctx the libusb context structure - * \param timeout the wait timeout in seconds - * \param completed a reference to the completed flag - * \return true for completion, false for timeout - */ -UHD_INLINE bool wait_for_completion(libusb_context *ctx, const double timeout, int &completed) -{ - //already completed by a previous call? - if (completed) return true; - - //perform a non-blocking event handle - timeval tv; - tv.tv_sec = 0; - tv.tv_usec = 0; - libusb_handle_events_timeout_completed(ctx, &tv, &completed); - if (completed) return true; - - //finish the rest with a timeout loop - const boost::system_time timeout_time = boost::get_system_time() + boost::posix_time::microseconds(long(timeout*1000000)); - while (not completed and (boost::get_system_time() < timeout_time)){ - timeval tv; - tv.tv_sec = 0; - tv.tv_usec = 10000; /*10ms*/ - libusb_handle_events_timeout_completed(ctx, &tv, &completed); - } - - return completed; + r->completed = 1; + r->wait_for_complete.notify_one(); } /*********************************************************************** @@ -154,7 +125,7 @@ public: template UHD_INLINE typename buffer_type::sptr get_new(const double timeout) { - if (wait_for_completion(_ctx, timeout, result.completed)) + if (wait_for_completion(timeout)) { if (result.status != LIBUSB_TRANSFER_COMPLETED) throw uhd::runtime_error(str(boost::format( "usb %s transfer status: %d") % _name % int(result.status))); @@ -164,9 +135,31 @@ public: return typename buffer_type::sptr(); } + UHD_INLINE bool flush(double timeout) + { + return wait_for_completion(timeout); + } + lut_result_t result; private: + /*! + * Wait for a managed buffer to become complete. + * + * \param timeout the wait timeout in seconds + * \return true for completion, false for timeout + */ + UHD_INLINE bool wait_for_completion(const double timeout) + { + boost::unique_lock lock(result.mut); + if (!result.completed) { + const boost::system_time timeout_time = boost::get_system_time() + boost::posix_time::microseconds(long(timeout*1000000)); + result.wait_for_complete.timed_wait(lock, timeout_time, lut_result_completed(result)); + } + return result.completed; + } + + boost::function _release_cb; const bool _is_recv; const std::string _name; @@ -252,8 +245,6 @@ public: ~libusb_zero_copy_single(void) { - libusb_context *ctx = libusb::session::get_global_session()->get_context(); - //cancel all transfers BOOST_FOREACH(libusb_transfer *lut, _all_luts) { @@ -261,8 +252,10 @@ public: } //process all transfers until timeout occurs - int completed = 0; - wait_for_completion(ctx, 0.01, completed); + BOOST_FOREACH(libusb_zero_copy_mb *mb, _enqueued) + { + mb->flush(0.01); + } //free all transfers BOOST_FOREACH(libusb_transfer *lut, _all_luts) @@ -276,19 +269,18 @@ public: { typename buffer_type::sptr buff; libusb_zero_copy_mb *front = NULL; + boost::mutex::scoped_lock lock(_mutex); + if (_enqueued.empty()) { - boost::mutex::scoped_lock l(_mutex); - if (_enqueued.empty()) - { - _cond.timed_wait(l, boost::posix_time::microseconds(long(timeout*1e6))); - } - if (_enqueued.empty()) return buff; - front = _enqueued.front(); + _cond.timed_wait(l, boost::posix_time::microseconds(long(timeout*1e6))); } + if (_enqueued.empty()) return buff; + front = _enqueued.front(); + lock.unlock(); buff = front->get_new(timeout); + lock.lock(); - boost::mutex::scoped_lock l(_mutex); if (buff) _enqueued.pop_front(); this->submit_what_we_can(); return buff; diff --git a/host/lib/usrp/b200/b200_io_impl.cpp b/host/lib/usrp/b200/b200_io_impl.cpp index d643ef855..f2d463a79 100644 --- a/host/lib/usrp/b200/b200_io_impl.cpp +++ b/host/lib/usrp/b200/b200_io_impl.cpp @@ -231,14 +231,14 @@ rx_streamer::sptr b200_impl::get_rx_stream(const uhd::stream_args_t &args_) //calculate packet size static const size_t hdr_size = 0 + vrt::max_if_hdr_words32*sizeof(boost::uint32_t) - + sizeof(vrt::if_packet_info_t().tlr) //forced to have trailer + //+ sizeof(vrt::if_packet_info_t().tlr) //forced to have trailer - sizeof(vrt::if_packet_info_t().cid) //no class id ever used - sizeof(vrt::if_packet_info_t().tsi) //no int time ever used ; const size_t bpp = _data_transport->get_recv_frame_size() - hdr_size; const size_t bpi = convert::get_bytes_per_item(args.otw_format); size_t spp = unsigned(args.args.cast("spp", bpp/bpi)); - spp = std::min(2000, spp); //magic maximum for framing at full rate + spp = std::min(4092, spp); //magic maximum for framing at full rate //make the new streamer given the samples per packet if (not my_streamer) my_streamer = boost::make_shared(spp); -- cgit v1.2.3 From 7961fc2388211ea2edf8313ef729408acc700dd1 Mon Sep 17 00:00:00 2001 From: Moritz Fischer Date: Mon, 11 Nov 2013 10:52:39 +0100 Subject: BUG #183: Fixed typo Signed-off-by: Moritz Fischer --- host/lib/transport/libusb1_zero_copy.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/host/lib/transport/libusb1_zero_copy.cpp b/host/lib/transport/libusb1_zero_copy.cpp index 7b1de65c3..d269eef68 100644 --- a/host/lib/transport/libusb1_zero_copy.cpp +++ b/host/lib/transport/libusb1_zero_copy.cpp @@ -272,7 +272,7 @@ public: boost::mutex::scoped_lock lock(_mutex); if (_enqueued.empty()) { - _cond.timed_wait(l, boost::posix_time::microseconds(long(timeout*1e6))); + _cond.timed_wait(lock, boost::posix_time::microseconds(long(timeout*1e6))); } if (_enqueued.empty()) return buff; front = _enqueued.front(); -- cgit v1.2.3 From 39d69b3a8871e4d6f442511e57394f83a012d3b6 Mon Sep 17 00:00:00 2001 From: Michael West Date: Tue, 19 Nov 2013 11:16:44 -0800 Subject: BUG #183: Addressed comments from code review. --- host/lib/transport/libusb1_zero_copy.cpp | 29 +++++++++++++++-------------- host/lib/usrp/b200/b200_io_impl.cpp | 2 +- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/host/lib/transport/libusb1_zero_copy.cpp b/host/lib/transport/libusb1_zero_copy.cpp index d269eef68..2d18e1623 100644 --- a/host/lib/transport/libusb1_zero_copy.cpp +++ b/host/lib/transport/libusb1_zero_copy.cpp @@ -74,12 +74,13 @@ struct lut_result_t libusb_transfer_status status; int actual_length; boost::mutex mut; - boost::condition_variable wait_for_complete; + boost::condition_variable usb_transfer_complete; }; +// Created to be used as an argument to boost::condition_variable::timed_wait() function struct lut_result_completed { - lut_result_t& _result; - lut_result_completed(lut_result_t& result):_result(result) {} + const lut_result_t& _result; + lut_result_completed(const lut_result_t& result):_result(result) {} bool operator()() const {return (_result.completed ? true : false);} }; @@ -96,7 +97,7 @@ static void LIBUSB_CALL libusb_async_cb(libusb_transfer *lut) r->status = lut->status; r->actual_length = lut->actual_length; r->completed = 1; - r->wait_for_complete.notify_one(); + r->usb_transfer_complete.notify_one(); // wake up thread waiting in wait_for_completion() member function below } /*********************************************************************** @@ -135,30 +136,30 @@ public: return typename buffer_type::sptr(); } - UHD_INLINE bool flush(double timeout) - { - return wait_for_completion(timeout); - } - + // This is public because it is accessed from the libusb_zero_copy_single constructor lut_result_t result; -private: /*! * Wait for a managed buffer to become complete. * - * \param timeout the wait timeout in seconds + * \param timeout the wait timeout in seconds. A negative value will wait forever. * \return true for completion, false for timeout */ UHD_INLINE bool wait_for_completion(const double timeout) { boost::unique_lock lock(result.mut); if (!result.completed) { - const boost::system_time timeout_time = boost::get_system_time() + boost::posix_time::microseconds(long(timeout*1000000)); - result.wait_for_complete.timed_wait(lock, timeout_time, lut_result_completed(result)); + if (timeout < 0.0) { + result.usb_transfer_complete.wait(lock); + } else { + const boost::system_time timeout_time = boost::get_system_time() + boost::posix_time::microseconds(long(timeout*1000000)); + result.usb_transfer_complete.timed_wait(lock, timeout_time, lut_result_completed(result)); + } } return result.completed; } +private: boost::function _release_cb; const bool _is_recv; @@ -254,7 +255,7 @@ public: //process all transfers until timeout occurs BOOST_FOREACH(libusb_zero_copy_mb *mb, _enqueued) { - mb->flush(0.01); + mb->wait_for_completion(0.01); } //free all transfers diff --git a/host/lib/usrp/b200/b200_io_impl.cpp b/host/lib/usrp/b200/b200_io_impl.cpp index f2d463a79..1feeff1a3 100644 --- a/host/lib/usrp/b200/b200_io_impl.cpp +++ b/host/lib/usrp/b200/b200_io_impl.cpp @@ -238,7 +238,7 @@ rx_streamer::sptr b200_impl::get_rx_stream(const uhd::stream_args_t &args_) const size_t bpp = _data_transport->get_recv_frame_size() - hdr_size; const size_t bpi = convert::get_bytes_per_item(args.otw_format); size_t spp = unsigned(args.args.cast("spp", bpp/bpi)); - spp = std::min(4092, spp); //magic maximum for framing at full rate + spp = std::min(2044, spp); //magic maximum for framing at full rate //make the new streamer given the samples per packet if (not my_streamer) my_streamer = boost::make_shared(spp); -- cgit v1.2.3