m. allan noah writes:
> Your patch makes no difference for the Fujitsu models in question.
> I'll commit the patch in the next couple days, after some more
> testing.
While working up a patch to only change the USB configuration if it is
not already current, I noticed my previous patch introduced a memory
leak with libusb-1.0. Attached are my previous patch, a fix for the
memory leak and a change to the libusb-1.0 branch that only sets the USB
configuration if not already current.
I looked at doing the same for libusb-0.1 but that doesn't have the
convenience API to get the current configuration. I am too lazy to
implement this using usb_control_message() myself for a library that I
consider obsolete (for a few years already ;-).
> allan
>
> On Mon, Dec 1, 2014 at 9:44 PM, m. allan noah <kitno...@gmail.com> wrote:
>> Ha! As we speak, I am digging into USB3 again, because command
>> counting seems to not fix every case. I will try your patch and report
>> back.
>>
>> allan
>>
>> On Mon, Dec 1, 2014 at 9:21 PM, Olaf Meeuwissen
>> <olaf.meeuwis...@avasys.jp> wrote:
>>> Olaf Meeuwissen writes:
>>>
>>>> m. allan noah writes:
>>>>
>>>>> I have added a USB3 port to my computer, and have spent a few days
>>>>> investigating this problem.
>>>>
>>>> I haven't had that kind of time, yet :-(
>>>
>>> Finally got time to do so.
>>> # Someone managed to sky-rocket the priority of USB3 support to the top
>>> # of my TODO list ;-)
>>>
>>>>> It seems to be a bug in Linux kernel related to a bookkeeping error
>>>>> when the USB device is closed. [...]
>>>
>>> Based on two days of looking at USB3 behaviour with three EPSON scanners
>>> and three backends, that does not seem to be the whole story.
>>>
>>>>> I have committed these changes to sane-backends git repo in
>>>>> 7a590f362e7e93979b706dd9e6ae34584e926ec3. If users of fujitsu scanners
>>>>> could try it out, I would appreciate any feedback. Note that this does
>>>>> not fix USB3 problems in other sane backends. Until the kernel is
>>>>> fixed, these types of repairs will have to be made on a per-backend
>>>>> basis, at the discretion of the maintainer.
>>>
>>> The epson and epson2 backends have been and still are using the kind of
>>> command counting and making sure to send an even count of reads and
>>> writes you added to the fujitsu backend. That notwithstanding, they
>>> suffer from USB3 I/O problems with the Perfection V700 (GT-X900) and
>>> ES-H300 (GT-2500). The epkowa backend does *not* even up the reads and
>>> writes and unsurprisingly shows the same problems. In addition, the
>>> GT-D1000 (GT-1500), a non-free plugin needing device, is also affected.
>>>
>>> Making the epkowa backend even up the reads and writes did *not* fix the
>>> problem. What did fix the problem for all three scanners and all three
>>> backends for me was a tiny change in sanei_usb.c. With that change, the
>>> evening up of reads and writes was not needed either.
>>>
>>> The sanei_usb.c code unconditionally sets the USB configuration. This
>>> happens even if the device has only one. I found that *not* setting the
>>> USB configuration when there is only one fixed all my USB3 problems. My
>>> understanding of the USB2 and USB3 standards is that the default USB
>>> configuration has to be set before devices become available to libusb.
>>> If that understanding is correct, then there is no need to set it unless
>>> you need a configuration that *differs* from the current one. With only
>>> one configuration to choose from, there is then no need to set it.
>>>
>>> This patch does exactly that. It skips setting the USB configuration if
>>> there is only one. It should not break USB for any of the backends and
>>> *may* fix USB3 for all of them.
>>>
>>> Note: it may still set the default configuration if there are multiple
>>> ones to choose from. I have no devices I can test with to see if
>>> that is problematic or not. Even so, modifying the code to only
>>> set a USB configuration if it differs should be safe.
>>> # Patch is in preparation.
>>>
>>> Allan, can you locally revert your fujitsu fixes and try with the patch
>>> I applied? Does that fix your USB3 issues? Do you still need to even
>>> up the reads and writes?
>>>
>>> Unless it breaks things for you, could you please commit my patch?
>>> # Even if it does not fix USB3 for you.
>>>
>>> Other people with USB3 issues are of course more than welcome to try
>>> this patch too!
>>>
>>>> [...]
Hope this helps,
--
Olaf Meeuwissen, LPIC-2 FLOSS Engineer -- AVASYS CORPORATION
FSF Associate Member #1962 Help support software freedom
http://www.fsf.org/jf?referrer=1962
>From 9b993042a25b854dde4d58d9f48f0d6f393e4373 Mon Sep 17 00:00:00 2001
From: Olaf Meeuwissen <olaf.meeuwis...@avasys.jp>
Date: Tue, 2 Dec 2014 10:03:38 +0900
Subject: [PATCH 1/3] Set USB configuration only if there are alternative ones
This fixes USB3 issues for the epson and epson2 backends. Both these
backends already made sure to send an even number of reads and writes
but were affected nevertheless.
It also solves USB3 issues with the third party epkowa backend. This
one doesn't bother to ensure even counts of packets read and written.
It works fine when this fix is applied (and it no longer clears halts
w/o a stall condition). Note, there is no need to ensure even packet
counts.
---
sanei/sanei_usb.c | 109 ++++++++++++++++++++++++++++--------------------------
1 file changed, 56 insertions(+), 53 deletions(-)
diff --git a/sanei/sanei_usb.c b/sanei/sanei_usb.c
index 491ceeb..c0fa74d 100644
--- a/sanei/sanei_usb.c
+++ b/sanei/sanei_usb.c
@@ -1379,30 +1379,32 @@ sanei_usb_open (SANE_String_Const devname, SANE_Int * dn)
"configuration (%d), choosing first config (%d)\n",
dev->descriptor.bNumConfigurations,
dev->config[0].bConfigurationValue);
- }
- result = usb_set_configuration (devices[devcount].libusb_handle,
- dev->config[0].bConfigurationValue);
- if (result < 0)
- {
- SANE_Status status = SANE_STATUS_INVAL;
- DBG (1, "sanei_usb_open: libusb complained: %s\n", usb_strerror ());
- if (errno == EPERM || errno == EACCES)
- {
- DBG (1, "Make sure you run as root or set appropriate "
- "permissions\n");
- status = SANE_STATUS_ACCESS_DENIED;
- }
- else if (errno == EBUSY)
- {
- DBG (3, "Maybe the kernel scanner driver or usblp claims the "
- "interface? Ignoring this error...\n");
- status = SANE_STATUS_GOOD;
- }
- if (status != SANE_STATUS_GOOD)
+ result = usb_set_configuration (devices[devcount].libusb_handle,
+ dev->config[0].bConfigurationValue);
+ if (result < 0)
{
- usb_close (devices[devcount].libusb_handle);
- return status;
+ SANE_Status status = SANE_STATUS_INVAL;
+
+ DBG (1, "sanei_usb_open: libusb complained: %s\n",
+ usb_strerror ());
+ if (errno == EPERM || errno == EACCES)
+ {
+ DBG (1, "Make sure you run as root or set appropriate "
+ "permissions\n");
+ status = SANE_STATUS_ACCESS_DENIED;
+ }
+ else if (errno == EBUSY)
+ {
+ DBG (3, "Maybe the kernel scanner driver or usblp claims the "
+ "interface? Ignoring this error...\n");
+ status = SANE_STATUS_GOOD;
+ }
+ if (status != SANE_STATUS_GOOD)
+ {
+ usb_close (devices[devcount].libusb_handle);
+ return status;
+ }
}
}
@@ -1443,13 +1445,13 @@ sanei_usb_open (SANE_String_Const devname, SANE_Int * dn)
DBG (5, "sanei_usb_open: interface nr: %d\n", i);
DBG (5, "sanei_usb_open: alt_setting nr: %d\n", a);
- /* Start by interfaces found in sanei_usb_init */
- if (c == 0 && i != devices[devcount].interface_nr)
- {
- DBG (5, "sanei_usb_open: interface %d not detected as "
- "a scanner by sanei_usb_init, ignoring.\n", i);
- continue;
- }
+ /* Start by interfaces found in sanei_usb_init */
+ if (c == 0 && i != devices[devcount].interface_nr)
+ {
+ DBG (5, "sanei_usb_open: interface %d not detected as "
+ "a scanner by sanei_usb_init, ignoring.\n", i);
+ continue;
+ }
interface = &dev->config[c].interface[i].altsetting[a];
@@ -1670,35 +1672,36 @@ sanei_usb_open (SANE_String_Const devname, SANE_Int * dn)
"configuration (%d), choosing first config (%d)\n",
desc.bNumConfigurations,
config0->bConfigurationValue);
- }
- result = libusb_set_configuration (devices[devcount].lu_handle,
- config0->bConfigurationValue);
- libusb_free_config_descriptor (config0);
+ result = libusb_set_configuration (devices[devcount].lu_handle,
+ config0->bConfigurationValue);
- if (result < 0)
- {
- SANE_Status status = SANE_STATUS_INVAL;
+ libusb_free_config_descriptor (config0);
- DBG (1, "sanei_usb_open: libusb complained: %s\n",
- sanei_libusb_strerror (result));
- if (result == LIBUSB_ERROR_ACCESS)
- {
- DBG (1, "Make sure you run as root or set appropriate "
- "permissions\n");
- status = SANE_STATUS_ACCESS_DENIED;
- }
- else if (result == LIBUSB_ERROR_BUSY)
+ if (result < 0)
{
- DBG (3, "Maybe the kernel scanner driver or usblp claims the "
- "interface? Ignoring this error...\n");
- status = SANE_STATUS_GOOD;
- }
+ SANE_Status status = SANE_STATUS_INVAL;
- if (status != SANE_STATUS_GOOD)
- {
- libusb_close (devices[devcount].lu_handle);
- return status;
+ DBG (1, "sanei_usb_open: libusb complained: %s\n",
+ sanei_libusb_strerror (result));
+ if (result == LIBUSB_ERROR_ACCESS)
+ {
+ DBG (1, "Make sure you run as root or set appropriate "
+ "permissions\n");
+ status = SANE_STATUS_ACCESS_DENIED;
+ }
+ else if (result == LIBUSB_ERROR_BUSY)
+ {
+ DBG (3, "Maybe the kernel scanner driver or usblp claims "
+ "the interface? Ignoring this error...\n");
+ status = SANE_STATUS_GOOD;
+ }
+
+ if (status != SANE_STATUS_GOOD)
+ {
+ libusb_close (devices[devcount].lu_handle);
+ return status;
+ }
}
}
--
2.1.3
>From fb781aac4fd5c6ee9cb74459c5e6f3a809ec41b7 Mon Sep 17 00:00:00 2001
From: Olaf Meeuwissen <olaf.meeuwis...@avasys.jp>
Date: Tue, 2 Dec 2014 11:37:25 +0900
Subject: [PATCH 2/3] Fix memory leak w/ libusb-1.0 (introduced in 9b99304)
---
sanei/sanei_usb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sanei/sanei_usb.c b/sanei/sanei_usb.c
index c0fa74d..c4f5c7c 100644
--- a/sanei/sanei_usb.c
+++ b/sanei/sanei_usb.c
@@ -1676,8 +1676,6 @@ sanei_usb_open (SANE_String_Const devname, SANE_Int * dn)
result = libusb_set_configuration (devices[devcount].lu_handle,
config0->bConfigurationValue);
- libusb_free_config_descriptor (config0);
-
if (result < 0)
{
SANE_Status status = SANE_STATUS_INVAL;
@@ -1700,10 +1698,12 @@ sanei_usb_open (SANE_String_Const devname, SANE_Int * dn)
if (status != SANE_STATUS_GOOD)
{
libusb_close (devices[devcount].lu_handle);
+ libusb_free_config_descriptor (config0);
return status;
}
}
}
+ libusb_free_config_descriptor (config0);
/* Claim the interface */
result = libusb_claim_interface (devices[devcount].lu_handle,
--
2.1.3
>From 9cb4c635f334a7fc98bffefb3da5c194ad79c905 Mon Sep 17 00:00:00 2001
From: Olaf Meeuwissen <olaf.meeuwis...@avasys.jp>
Date: Tue, 2 Dec 2014 11:59:35 +0900
Subject: [PATCH 3/3] Set USB configuration only if different from the current
one
This follows up on earlier USB3 fixes. It only addresses libusb-1.0
because there is no convenience API for libusb-0.1 to get the current
configuration.
---
sanei/sanei_usb.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/sanei/sanei_usb.c b/sanei/sanei_usb.c
index c4f5c7c..d66ec66 100644
--- a/sanei/sanei_usb.c
+++ b/sanei/sanei_usb.c
@@ -1673,8 +1673,10 @@ sanei_usb_open (SANE_String_Const devname, SANE_Int * dn)
desc.bNumConfigurations,
config0->bConfigurationValue);
- result = libusb_set_configuration (devices[devcount].lu_handle,
- config0->bConfigurationValue);
+ result = 0;
+ if (config != config0->bConfigurationValue)
+ result = libusb_set_configuration (devices[devcount].lu_handle,
+ config0->bConfigurationValue);
if (result < 0)
{
--
2.1.3
--
sane-devel mailing list: sane-devel@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/sane-devel
Unsubscribe: Send mail with subject "unsubscribe your_password"
to sane-devel-requ...@lists.alioth.debian.org
