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 18:49, Oliver Neukum <oneukum@xxxxxxxx> wrote:
> On 05.06.22 15:45, Vincent MAILHOL wrote:
> >
> > This is how I see things:
> >   * In the open() function, the driver will do the coherent allocation
> > for its transfer_buffers, fill those into URBs and add all the URBs in
> > an anchor.
> >   * During runtime, the driver will keep recycling the same URBs (no
> > need to kill URB nor to usb_free_coherent() the transfer_buffer).
> Yes.
> >   * Finally, in the close() function, the driver has to kill the URBs
> > and usb_free_coherent() the transfer_buffers. As far as I understand,
> > no helper functions allow us to do all that, thus requiring the driver
> > to iterate through the anchor to manually usb_free_coherent() the
> > transfer buffer.
> Yes. But you cannot nicely solve that with a flag as you proposed. You
> would need to use a helper function.
> > So, the intent of this patch is to provide a method to both kill the
> > URBs and usb_free_coherent() the transfer buffer at once. The
> Well, you don't directly. Your patch frees the buffer together with the URB.
> That has some uses, but you still would need to iterate over the URBs
> Yes, there is a helper for that, but then you cover one and only one
> use case, that is, you leave no way to free the buffers without
> at the same time discrading the URBs.
>
> You can do that, but it strikes me as unelegant.

Elegancy is also my concern.

My RFC originated from this patch:
https://lore.kernel.org/linux-can/alpine.DEB.2.22.394.2206031547001.1630869@thelappy/

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


Yours sincerely,
Vincent Mailhol



[Index of Archives]     [Automotive Discussions]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [CAN Bus]

  Powered by Linux