diff mbox series

[v2,1/3] drivers: usb/core/urb: Add URB_FREE_COHERENT

Message ID 20220609204714.2715188-2-rhett.aultman@samsara.com
State New
Headers show
Series URB_FREE_COHERENT gs_usb memory leak fix | expand

Commit Message

Rhett Aultman June 9, 2022, 8:47 p.m. UTC
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 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() on the
transfer buffer when the URB is killed, similarly to how
URB_FREE_BUFFER triggers a call to kfree().

In order to have all the flags in numerical order, URB_DIR_IN is
renumbered from 0x0200 to 0x0400 so that URB_FREE_COHERENT can reuse
value 0x0200.

From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
 drivers/usb/core/urb.c | 3 +++
 include/linux/usb.h    | 3 ++-
 2 files changed, 5 insertions(+), 1 deletion(-)

Comments

Vincent MAILHOL June 10, 2022, 12:18 a.m. UTC | #1
On Fri. 10 juin 2022 à 06:11, Rhett Aultman <rhett.aultman@samsara.com> wrote:

The From tag goes at the beginning of the patch.

From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>

> 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 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() on the
> transfer buffer when the URB is killed, similarly to how
> URB_FREE_BUFFER triggers a call to kfree().
>
> In order to have all the flags in numerical order, URB_DIR_IN is
> renumbered from 0x0200 to 0x0400 so that URB_FREE_COHERENT can reuse
> value 0x0200.
>
> From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>

Move the From tag from here to the first line.

> Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> ---
>  drivers/usb/core/urb.c | 3 +++
>  include/linux/usb.h    | 3 ++-
>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> 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..945d68ea1d76 100644
> --- a/include/linux/usb.h
> +++ b/include/linux/usb.h
> @@ -1328,9 +1328,10 @@ 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      0x0200  /* 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 */
> +#define URB_DIR_IN             0x0400  /* Transfer from device to host */
>  #define URB_DIR_OUT            0
>  #define URB_DIR_MASK           URB_DIR_IN
Marc Kleine-Budde June 10, 2022, 10:46 a.m. UTC | #2
On 10.06.2022 09:18:51, Vincent MAILHOL wrote:
> On Fri. 10 juin 2022 à 06:11, Rhett Aultman <rhett.aultman@samsara.com> wrote:
> 
> The From tag goes at the beginning of the patch.
> 
> From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> 
> > 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 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() on the
> > transfer buffer when the URB is killed, similarly to how
> > URB_FREE_BUFFER triggers a call to kfree().
> >
> > In order to have all the flags in numerical order, URB_DIR_IN is
> > renumbered from 0x0200 to 0x0400 so that URB_FREE_COHERENT can reuse
> > value 0x0200.
> >
> > From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> 
> Move the From tag from here to the first line.

Usually git send-email places the "From:" automatically at the beginning.

> > Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>

Rhett, please add your S-o-b here, too, as the patch went though your
hands.

Marc
diff mbox series

Patch

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..945d68ea1d76 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1328,9 +1328,10 @@  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	0x0200  /* 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 */
+#define URB_DIR_IN		0x0400	/* Transfer from device to host */
 #define URB_DIR_OUT		0
 #define URB_DIR_MASK		URB_DIR_IN