Re: [PATCH] usb: sysfs: make locking interruptible

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 14 Jan 2016, Dean Jenkins wrote:

> 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.

What's so hard about proving it?  The rawdescriptors and device 
descriptor are created when the device is initialized, before the sysfs 
files have been registered.  The descriptors are not changed or 
deallocated until the device structure is freed, which is well after 
the sysfs files have been unregistered.

(There is one exception: For a wireless USB device, 
usb_authorize_device() will re-read the device descriptor.  I don't 
think this will cause any trouble.)

> >> 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

Not in the sysfs kernel layer.  In the seq_file layer (which sysfs 
uses).

> 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() ?

seq_file.

> 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...

The sysfs layer insures that calls to the ->show and ->store callbacks 
cannot race with file removal.  (This is particularly tricky in the 
case where the callback calls device_del!)

Alan Stern

--
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



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux