Re: [RFC PATCH] USB: core: urb: add new transfer flag URB_FREE_COHERENT

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

 



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



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux