On Sun. 5 juin 2022 at 01:40, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote : > On Sat, Jun 04, 2022 at 11:41:57PM +0900, Vincent Mailhol wrote: > > When allocating URB memory with kmalloc(), drivers can simply set the > > URB_FREE_BUFFER flag in urb::transfer_flags and that way, the memory > > will be freed in the background when calling killing the URB (for > > example with usb_kill_anchored_urbs()). > > > > However, there are no equivalent mechanism when allocating DMA memory > > (with usb_alloc_coherent()). > > > > This patch adds a new flag: URB_FREE_COHERENT. Setting this flag will > > cause the kernel to automatically call usb_free_coherent() when the > > URB is killed, similarly to how URB_FREE_BUFFER triggers a call to > > kfree(). > > > > Signed-off-by: Vincent Mailhol <mailhol.vincent@xxxxxxxxxx> > > --- > > Hi Rhett Aultman, > > > > I put the code snippet I previously sent into a patch. It is not > > tested (this is why I post it as RFC). Please feel free to add this to > > your series. > > --- > > drivers/usb/core/urb.c | 3 +++ > > include/linux/usb.h | 1 + > > 2 files changed, 4 insertions(+) > > > > diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c > > index 33d62d7e3929..1460fdac0b18 100644 > > --- a/drivers/usb/core/urb.c > > +++ b/drivers/usb/core/urb.c > > @@ -22,6 +22,9 @@ static void urb_destroy(struct kref *kref) > > > > if (urb->transfer_flags & URB_FREE_BUFFER) > > kfree(urb->transfer_buffer); > > + else if (urb->transfer_flags & URB_FREE_COHERENT) > > + usb_free_coherent(urb->dev, urb->transfer_buffer_length, > > + urb->transfer_buffer, urb->transfer_dma); > > > > kfree(urb); > > } > > diff --git a/include/linux/usb.h b/include/linux/usb.h > > index 60bee864d897..2200b3785fdb 100644 > > --- a/include/linux/usb.h > > +++ b/include/linux/usb.h > > @@ -1328,6 +1328,7 @@ extern int usb_disabled(void); > > #define URB_NO_INTERRUPT 0x0080 /* HINT: no non-error interrupt > > * needed */ > > #define URB_FREE_BUFFER 0x0100 /* Free transfer buffer with the URB */ > > +#define URB_FREE_COHERENT 0x0400 /* Free DMA memory of transfer buffer */ > > > > /* The following flags are used internally by usbcore and HCDs */ > > #define URB_DIR_IN 0x0200 /* Transfer from device to host */ > > I don't see anything wrong with this, except that it would be nice to keep > the flag values in numerical order. In other words, set URB_FREE_COHERENT > to 0x0200 and change URB_DIR_IN to 0x0400. Indeed, the URB_DIR_IN macro is not part of the UAPI so it should be safe to renumber it. Maybe I was confused by the USB_DIR_IN which is part of UAPI so some part of my subcocient was telling me not to change it... Thanks for pointing this out, I will send a v2 right away (and I will keep the RFC tag because this is not yet tested). Yours sincerely, Vincent Mailhol
