Re: [PATCH v7 19/19] staging: r8188eu: remove shared buffer for usb requests

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

 



On 9/17/21 18:03, Pavel Skripkin wrote:
On 9/17/21 17:55, Greg Kroah-Hartman wrote:
On Fri, Sep 17, 2021 at 09:18:37AM +0200, Fabio M. De Francesco wrote:
From: Pavel Skripkin <paskripkin@xxxxxxxxx>

This driver used shared buffer for usb requests. It led to using
mutexes, i.e no usb requests can be done in parallel.

USB requests can be fired in parallel since USB Core allows it. In
order to allow them, remove usb_vendor_req_buf from dvobj_priv (since
USB I/O is the only user of it) and remove also usb_vendor_req_mutex
(since there is nothing to protect).

Ah, you are removing this buffer, nice!

But, just because the USB core allows multiple messages to be sent to a
device at the same time, does NOT mean that the device itself can handle
that sort of a thing.

Keeping that lock might be a good idea, until you can prove otherwise.
You never know, maybe there's never any contention at all for it because
these accesses are all done in a serial fashion and the lock
grab/release is instant.  But if that is not the case, you might really
get a device confused here by throwing multiple control messages at it
in ways that it is not set up to handle at all.

So please do not drop the lock.

More comments below.


We have tested this change. I've tested it in qemu with TP-Link
TL-WN722N v2 / v3 [Realtek RTL8188EUS], and Fabio has tested it on his
host for like a whole evening.

I agree, that our testing does not cover all possible cases and I can't
say it was "good stress testing", so, I think, we need some comments
from maintainers.

@Larry, @Phillip, does this change looks reasonable for this chip?

			 ^^^^^^^^^^^

I mean mutex removal, sorry for confusion :)



With regards,
Pavel Skripkin




[Index of Archives]     [Linux Driver Development]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux