On 14/01/16 12:50, Oliver Neukum wrote:
On Thu, 2016-01-14 at 12:20 +0000, Dean Jenkins wrote:
Unwanted blocking occurs when read_descriptors() is getting the
descriptor information from the allocated rawdescriptors structure of a
USB hub but a device connected to the hub is failing and the device is
being recovered.
Quite possibly so. It is a separate issue. I'd be happy to provide
a patch.
But it is not the only read that might block. The rest is also
annoying. An uninterruptible sleep on an event that can take
so long is no good.
I agree that your patch is a separate issue / fix.
I am working on a patch to remove the locks for read_descriptors(). I am
trying to prove that removing the locking is safe so it will take me
some time to prove it.
This patch helps in the case where the userland application is being
terminated by Ctrl-C whilst a failing USB device is being recovered due
to communication failure which causes blocking to get the USB hub
descriptors information from memory.
Yes, that is the patch's point.
Have you checked that the calling sysfs layer can handle the returned
error code of -EINTR ? What code handles that ?
-EINTR is supposed to go to user space.
read_descriptors() is at the bottom of a system call but the -EINTR will
go to some sysfs layer code (the caller function) which I have failed to
identify. So the error code will pass through some unknown kernel code
(at least I don't know it) before reaching userland
Note that read_descriptors() gets called multiple times (I have seen 3
calls) per a single sysfs read.
OK, but what does that change?
It means that I think there is a loop somewhere in the sysfs kernel
layer which calls read_descriptors() multiple times (I see 3 calls in my
debug output) because read_descriptors() can return partial information
of the descriptors. read_descriptors() does not have to return all the
descriptors information in a single call. I mean that the caller
function can pass in an offset parameter "off" and length parameter
"count" to read_descriptors() to start the read from and to limit the
read length. So what source code is managing the offset and count
parameter values in the layer above read_descriptors() ?
For example what happens if the -EINTR is returned on the 1st call to
read_descriptors(), will there be a 2nd and 3rd call (due to the unknown
loop), or will the system call terminate showing no data, or return
partial data, or crash as -EINTR is not handled properly because the
return value is how many bytes were read which is assumed to be positive ?
This also means that the sequential read_descriptors() calls are not
atomic which is a risk; the descriptor information is not locked for a
given instance of a sysfs read. However it should be OK as the sysfs vfs
descriptors entry should be destroyed before the rawdescriptors memory
is freed. But the destroying of the sysfs vfs descriptors entry occur
whilst read_descriptors() runs but I hope locking is present in the
sysfs layer to ensure it is OK. I just need to find the relevant source
code...
Regards,
Dean
--
Dean Jenkins
Embedded Software Engineer
Linux Transportation Solutions
Mentor Embedded Software Division
Mentor Graphics (UK) Ltd.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
