On Tue. 7 Jun 2022 at 20:46, Oliver Neukum <oneukum@xxxxxxxx> wrote: > On 07.06.22 12:18, Vincent MAILHOL wrote: > > Here the proposed solution was to keep a pointer of all the > > transfer_buffer in a local array to be able to free them when closing. > > I really found that original patch to be unelegant which led me to > > propose this RFC. > It was. > > Comparatively, I still think my patch to be a more elegant solution, > > and the original author also seems to share my thoughts. > > > > If my patch is unelegant, then what would be the elegant/state of the > > art way to free all this DMA allocated memory? > > (pointing to any reference driver implementation should be enough for > > me to understand). > > I have two options to offer. > > 1. Use the existing URB_NO_TRANSFER_D;MA_MAP to decide > how to free memory. If doing so, all drivers using DMA would need to be adjusted (or else that would be a double free). Similar to URB_FREE_BUFFER, I think it must be a separate flag so that the drivers still have the option to release the memory by hand if needed for specific use cases. > 2. Provide an explicit API in usbcore among the anchor API Like a usb_kill_anchored_urbs_and_free_coherent()? Wouldn't it be inconsistent with the URB_FREE_BUFFER flag? It is weird to me if a normal kmalloc()ed buffer would have to use a flag and DMAed buffers a special API call. I mean, yes, it would work but I am not convinced that this approach is more elegant. The other advantage I see in my approach is that by setting the URB_FREE_COHERENT flag, all the kill/free functions from the URB API would benefit. For example, usb_free_urb() would also benefit. Right now, I only see the use case for usb_kill_anchored_urbs() but I am worried that we will have to duplicate many of the URB API. Yours sincerely, Vincent Mailhol
