Re: [PATCH v2 1/1] Increase USB transfer limit

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

 




> Il giorno 15 dic 2016, alle ore 06:51, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> ha scritto:
> 
> Hmmm, this is the third patch labelled v2 that you have mailed.  In the 
> future, be more careful about your version numbering.

Ok. Maybe I misunderstood versioning of patches. I thought of versions numbering the concept/approach rather than the actual revision of the patch. Should they all have been sent with an incremented number ?

> 
>> On Wed, 14 Dec 2016, Mateusz Berezecki wrote:
>> 
>> Promote a variable keeping track of USB transfer memory usage to a
>> wider data type and allow for higher bandwidth transfers from a large
>> number of USB devices connected to a single host.
>> 
>> Signed-off-by: Mateusz Berezecki <mateuszb@xxxxxxxxxxx>
>> ---
>> drivers/usb/core/devio.c | 43 ++++++++++++++++---------------------------
>> 1 file changed, 16 insertions(+), 27 deletions(-)
>> 
>> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
>> index 4016dae..52747b6 100644
>> --- a/drivers/usb/core/devio.c
>> +++ b/drivers/usb/core/devio.c
>> @@ -134,42 +134,35 @@ enum snoop_when {
>> #define USB_DEVICE_DEV        MKDEV(USB_DEVICE_MAJOR, 0)
>> 
>> /* Limit on the total amount of memory we can allocate for transfers */
>> -static unsigned usbfs_memory_mb = 16;
>> +static u32 usbfs_memory_mb = 16;
>> module_param(usbfs_memory_mb, uint, 0644);
>> MODULE_PARM_DESC(usbfs_memory_mb,
>>        "maximum MB allowed for usbfs buffers (0 = no limit)");
>> 
>> -/* Hard limit, necessary to avoid arithmetic overflow */
>> -#define USBFS_XFER_MAX        (UINT_MAX / 2 - 1000000)
>> -
>> -static atomic_t usbfs_memory_usage;    /* Total memory currently allocated */
>> +static atomic64_t usbfs_memory_usage;    /* Total memory currently allocated */
>> 
>> /* Check whether it's okay to allocate more memory for a transfer */
>> -static int usbfs_increase_memory_usage(unsigned amount)
>> +static int usbfs_increase_memory_usage(u64 amount)
>> {
>> -    unsigned lim;
>> +    u64 lim;
>> 
>> -    /*
>> -     * Convert usbfs_memory_mb to bytes, avoiding overflows.
>> -     * 0 means use the hard limit (effectively unlimited).
>> -     */
>>    lim = ACCESS_ONCE(usbfs_memory_mb);
>> -    if (lim == 0 || lim > (USBFS_XFER_MAX >> 20))
>> -        lim = USBFS_XFER_MAX;
>> -    else
>> -        lim <<= 20;
>> +    lim <<= 20;
>> 
>> -    atomic_add(amount, &usbfs_memory_usage);
>> -    if (atomic_read(&usbfs_memory_usage) <= lim)
>> -        return 0;
>> -    atomic_sub(amount, &usbfs_memory_usage);
>> -    return -ENOMEM;
>> +    atomic64_add(amount, &usbfs_memory_usage);
>> +
>> +    if (lim > 0 && atomic64_read(&usbfs_memory_usage) > lim) {
>> +        atomic64_sub(amount, &usbfs_memory_usage);
>> +        return -ENOMEM;
>> +    }
>> +
>> +    return 0;
>> }
>> 
>> /* Memory for a transfer is being deallocated */
>> -static void usbfs_decrease_memory_usage(unsigned amount)
>> +static void usbfs_decrease_memory_usage(u64 amount)
>> {
>> -    atomic_sub(amount, &usbfs_memory_usage);
>> +    atomic64_sub(amount, &usbfs_memory_usage);
>> }
>> 
>> static int connected(struct usb_dev_state *ps)
>> @@ -1191,7 +1184,7 @@ static int proc_bulk(struct usb_dev_state *ps, void __user *arg)
>>    if (!usb_maxpacket(dev, pipe, !(bulk.ep & USB_DIR_IN)))
>>        return -EINVAL;
>>    len1 = bulk.len;
>> -    if (len1 >= USBFS_XFER_MAX)
>> +    if (len1 >= (INT_MAX - sizeof(struct urb)))
>>        return -EINVAL;
>>    ret = usbfs_increase_memory_usage(len1 + sizeof(struct urb));
>>    if (ret)
>> @@ -1584,10 +1577,6 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb
>>        return -EINVAL;
>>    }
>> 
>> -    if (uurb->buffer_length >= USBFS_XFER_MAX) {
>> -        ret = -EINVAL;
>> -        goto error;
>> -    }
>>    if (uurb->buffer_length > 0 &&
>>            !access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
>>                uurb->buffer, uurb->buffer_length)) {
>> 
> 
> This all looks good.
> 
> Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>

Cool. Thanks for help and feedback!

Mateusz 


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