On Fri, Oct 4, 2013 at 10:34 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > On Fri, 4 Oct 2013, Markus Rechberger wrote: > >> > The biggest problem is that your proc_alloc_memory() routine doesn't >> > call usbfs_increase_memory_usage(). Without that, there's nothing to >> > prevent a user from allocating all the available kernel memory. >> > >> >> only root is supposed to have raw USB access, > > Not true at all. Any user may be allowed to access a USB device, > depending on the permissions for the usbfs device file. > well I mean by default if you connect an unknown device only root will usually have access to it. I know there are udev rules for some special devices. >> which limit would you suggest? > > The limit is already in place. All you have to do is call the routine > to make sure that you don't exceed the limit. Check the other places > where usbfs_increase_memory_usage() is used and you'll see how it > works. > ok >> >> Version 0.4: >> >> * overriding SG transfers once memory mapped buffers are allocated for >> >> BULK >> >> * adding pgprot_noncached to ensure that the IO memory is not cached >> >> (thanks to Ming Lei for mentioning that) >> > >> > I don't understand this. Why shouldn't the buffer memory be cached? >> > Won't uncached buffers be a lot slower? >> > >> >> I was only testing reading the data so I didn't see any caching >> effects since I don't have a device or driver which I can send a lot >> data out. >> As far as I understand pgprot_noncached would only be required when >> sending data (writing data to the DMA'able buffer). >> >> This question is still open. > > The buffer should be cached. The userspace program will have to make > sure that it doesn't try to access the buffer while DMA is in progress. > As long as that restriction is obeyed, the USB core will take care of > mapping the buffer for DMA (which flushes the cache). > ok so it can be removed. >> > As I understand it, you wrote this in order to solve problems where >> > memory couldn't be allocated for USB transfers. If the system is >> > running short on memory, won't your USBDEVFS_ALLOC_MEMORY ioctl also >> > fail? >> > >> >> No, the buffers will be kept in a list and re-used (aside of that they >> are also kept in a list in devio.c and re-used). When the buffers are >> allocated initially there's no chance to fail this process during >> run-time. > > Why not? What if the memory is already almost full? The call to > alloc_pages_exact() could fail. > > I agree this is less likely than a failure with the current system, but > it is still possible. > if it happens the allocation ioctl returns an error, and the application knows from the beginning on that this is not going to work. But once it's allocated it should be solid. Our driver starts up early as well there has never been a failure at driver startup. I mentioned within another email we also have another kernel stub driver which does the pre-allocation within the module, no issues have been reported when the pre-allocation module was in place. There are discussions on several mailinglists about pre-allocating DMA'able memory when kernel drivers are loaded, that way is not so uncommon.. >> please note the transfer buffer are allocated and freed permanently >> during run-time with the old mechanism, this mechanism usually fails >> during run-time on low end systems. > > It's true that the existing system allocates and deallocates the buffer > for each transfer. With your patch, the buffer is allocated only once. > In addition, the same buffer is used in userspace and in the kernel, > removing the need to allocate two buffers and copy everything over. > This will be a significant saving of both time and space. > > By the way, have you considered that the user program might want the > I/O transfer to start at some position other than the beginning of the > buffer? > I thought about that but I couldn't find any useful usecase for that. I guess it would be very much device dependent. The URB buffers themself aren't that big either. Just when comparing the MacOSX API it wouldn't support that either. If someone needs it he could still add it. Markus > 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
