libftdi Archives

Subject: Re: ftdi_transfer_data_done busy loop

From: Omri Steiner <omri@xxxxxxxxxxxx>
To: libftdi@xxxxxxxxxxxxxxxxxxxxxxx
Cc: Eugene Hutorny <eugene@xxxxxxxxxxxxx>
Date: Wed, 2 Oct 2019 15:25:04 +0300
I'm not sure where you supplied the patch which adds timeouts.
Anyway, here's a patch which fixes it w/o adding hardcoded timeouts (they're hardcoded inside libusb instead).
It also doesn't break the API.

diff --git a/src/ftdi.c b/src/ftdi.c
index 988a9f1..ed2c817 100644
--- a/src/ftdi.c
+++ b/src/ftdi.c
@@ -1884,22 +1884,16 @@ struct ftdi_transfer_control *ftdi_read_data_submit(struct ftdi_context *ftdi, u
 int ftdi_transfer_data_done(struct ftdi_transfer_control *tc)
 {
     int ret;
-    struct timeval to = { 0, 0 };
     while (!tc->completed)
     {
-        ret = libusb_handle_events_timeout_completed(tc->ftdi->usb_ctx,
-                &to, &tc->completed);
+        ret = libusb_handle_events_completed(tc->ftdi->usb_ctx,
+                &tc->completed);
         if (ret < 0)
         {
             if (ret == LIBUSB_ERROR_INTERRUPTED)
                 continue;
-            libusb_cancel_transfer(tc->transfer);
-            while (!tc->completed)
-                if (libusb_handle_events_timeout_completed(tc->ftdi->usb_ctx,
-                        &to, &tc->completed) < 0)
-                    break;
-            libusb_free_transfer(tc->transfer);
-            free (tc);
+
+            ftdi_transfer_data_cancel(tc, NULL);
             return ret;
         }
     }
@@ -1931,18 +1925,23 @@ int ftdi_transfer_data_done(struct ftdi_transfer_control *tc)
 void ftdi_transfer_data_cancel(struct ftdi_transfer_control *tc,
                                struct timeval * to)
 {
-    struct timeval tv = { 0, 0 };
-
+    int ret;
     if (!tc->completed && tc->transfer != NULL)
     {
-        if (to == NULL)
-            to = &tv;
-
         libusb_cancel_transfer(tc->transfer);
         while (!tc->completed)
         {
-            if (libusb_handle_events_timeout_completed(tc->ftdi->usb_ctx, to, &tc->completed) < 0)
+            if (to != NULL) {
+                ret = libusb_handle_events_timeout_completed(tc->ftdi->usb_ctx,
+                            to, &tc->completed) < 0;
+            } else {
+                ret = libusb_handle_events_completed(tc->ftdi->usb_ctx,
+                            &tc->completed) < 0;
+            }
+
+            if (ret < 0) {
                 break;
+            }
         }
     }
 

On Wed, Oct 2, 2019 at 2:28 PM Thomas Jarosch <thomas.jarosch@xxxxxxxxxxxxx> wrote:
Hi Omri,

You wrote on Thu, Aug 22, 2019 at 11:22:35AM +0300:
> It seems that both ftdi_transfer_data_done and ftdi_transfer_data_cancel
> call libusb_handle_events_timeout_completed with zero timeval, with the
> intention of indefinitely blocking (it even states so in
> ftdi_transfer_data_cancel's documentation).
> However, in practice, libusb_handle_events_timeout_completed with zero
> timeval just acts as a non-blocking function, which means both of these
> functions actually busy-wait instead of polling indefinitely.
> Am I missing something?
> Shall I propose a patch?

ftdi_transfer_data_cancel() allows the user to pass an optional timeout.

ftdi_transfer_data_done() lacks this API and we can't
easily extend it without breaking existing users.

I've checked the libusb_handle_events_timeout_completed() API documentation,
theoretically it should not make much of a difference in user visible
behavior if we specify a timeout of one second or so.

CC:ing Eugene as he might know this code more than me.
(=read: Supplied the patch that added the timeouts)

[second email]
> Do you think it's worth fixing for the next release?
> I can write up a patch and manually test it.

if there's really no user visible change, I think it's worth it
as it will make the code more efficient and reduce power consumption.

Cheers,
Thomas

--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to libftdi+unsubscribe@xxxxxxxxxxxxxxxxxxxxxxx   



libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to libftdi+unsubscribe@xxxxxxxxxxxxxxxxxxxxxxx


Current Thread