Re: [PATCH v2 1/2] USB: core: add a memory pool to urb for host-controller private data

["David Wang" <00107082@163.com>]

At 2025-05-14 19:23:02, "Oliver Neukum" <oneukum@suse.com> wrote:
>
>
>On 13.05.25 13:38, David Wang wrote:
>> ---
>
>Hi,
>
>still an issue after a second review.
>I should have noticed earlier.
>
>> --- a/drivers/usb/core/urb.c
>> +++ b/drivers/usb/core/urb.c
>> @@ -23,6 +23,7 @@ static void urb_destroy(struct kref *kref)
>>   
>>   	if (urb->transfer_flags & URB_FREE_BUFFER)
>>   		kfree(urb->transfer_buffer);
>> +	kfree(urb->hcpriv_mempool);
>
>What if somebody uses usb_init_urb()?

I am not quite sure about the concern here, do you mean somebody create a urb,
and then usb_init_urb() here, and never use urb_destroy to release it?

That would cause memory leak if urb_destroy is not called......But is this really possible?.

>  
>>   	kfree(urb);
>>   }
>> @@ -1037,3 +1038,25 @@ int usb_anchor_empty(struct usb_anchor *anchor)
>>   
>>   EXPORT_SYMBOL_GPL(usb_anchor_empty);
>>   
>> +/**
>> + * urb_hcpriv_mempool_zalloc - alloc memory from mempool for hcpriv
>> + * @urb: pointer to URB being used
>> + * @size: memory size requested by current host controller
>> + * @mem_flags: the type of memory to allocate
>> + *
>> + * Return: NULL if out of memory, otherwise memory are zeroed
>> + */
>> +void *urb_hcpriv_mempool_zalloc(struct urb *urb, size_t size, gfp_t mem_flags)
>> +{
>> +	if (urb->hcpriv_mempool_size < size) {
>> +		kfree(urb->hcpriv_mempool);
>> +		urb->hcpriv_mempool_size = size;
>> +		urb->hcpriv_mempool = kmalloc(size, mem_flags);
>
>That could use kzalloc().

The memory would be  set to 0 before returning to user, via memset,   no matter whether the memory is 
newly alloced or just reused.  I think using kmalloc is ok here.


Thanks
David

>
>	Regards
>		Oliver