linux-can.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: 'Vincent MAILHOL' <mailhol.vincent@wanadoo.fr>,
	Alan Stern <stern@rowland.harvard.edu>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Marc Kleine-Budde <mkl@pengutronix.de>,
	Rhett Aultman <rhett.aultman@samsara.com>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	linux-can <linux-can@vger.kernel.org>,
	Oliver Neukum <oneukum@suse.com>
Subject: RE: [PATCH v3 1/2] drivers: usb/core/urb: Add URB_FREE_COHERENT
Date: Wed, 22 Jun 2022 09:22:12 +0000	[thread overview]
Message-ID: <48caa879b0064ced97623bf1dad5b2d9@AcuMS.aculab.com> (raw)
In-Reply-To: <CAMZ6RqL42DKD3evR4skswaJnAwOAO_qrZgXoLax7O95xVKUomQ@mail.gmail.com>

From: Vincent MAILHOL
> Sent: 21 June 2022 16:56
> 
> On Wed. 22 Jun 2022 at 00:13, Alan Stern <stern@rowland.harvard.edu> wrote:
> > On Tue, Jun 21, 2022 at 11:59:16PM +0900, Vincent MAILHOL wrote:
> > > I (probably wrongly) assumed that urb::transfer_buffer_length was the
> > > allocated length and urb::actual_length was what was actually being
> > > transferred. Right now, I am just confused. Seems that I need to study
> > > a bit more and understand the real purpose of
> > > urb::transfer_buffer_length because I still fail to understand in
> > > which situation this can be different from the allocated length.
> >
> > urb->transfer_buffer_length is the amount of data that the driver wants
> > to send or expects to receive.  urb->actual_length is the amount of data
> > that was actually sent or actually received.
> >
> > Neither of these values has to be the same as the size of the buffer --
> > but they better not be bigger!
> 
> Thanks. Now things are a bit clearer.
> I guess that for the outcoming URB what I proposed made no sense. For
> incoming URB, I guess that most of the drivers want to set
> urb::transfer_buffer once for all with the allocated size and never
> touch it again.
> 
> Maybe the patch only makes sense of the incoming URB. Would it make
> sense to keep it but with an additional check to trigger a dmesg
> warning if this is used on an outcoming endpoint and with additional
> comment that the URB_FREE_COHERENT requires urb::transfer_buffer to be
> the allocated size?

IIRC urb are pretty big.
You'd be unlucky if adding an extra field to hold the allocated
size would ever need more memory.
So it might just be worth saving the allocated size.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

  parent reply	other threads:[~2022-06-22  9:22 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-03 19:52 [PATCH 001/001] can: gs_usb: gs_usb_open/close( ) fix memory leak Rhett Aultman
2022-06-04  2:11 ` [PATCH] " Vincent Mailhol
2022-06-04  2:26   ` Vincent MAILHOL
2022-06-04 14:08     ` Rhett Aultman
2022-06-04 14:41       ` [RFC PATCH] USB: core: urb: add new transfer flag URB_FREE_COHERENT Vincent Mailhol
2022-06-04 16:40         ` Alan Stern
2022-06-05  2:04           ` Vincent MAILHOL
2022-06-05  6:00           ` Oliver Neukum
2022-06-05 13:45             ` Vincent MAILHOL
2022-06-07  9:49               ` Oliver Neukum
2022-06-07 10:18                 ` Vincent MAILHOL
2022-06-07 11:46                   ` Oliver Neukum
2022-06-07 12:12                     ` Vincent MAILHOL
2022-06-05  2:15         ` [RFC PATCH v2] usb: " Vincent Mailhol
2022-06-04 14:53       ` [PATCH] can: gs_usb: gs_usb_open/close( ) fix memory leak Vincent MAILHOL
2022-06-09 20:47 ` [PATCH v2 0/3] URB_FREE_COHERENT gs_usb memory leak fix Rhett Aultman
2022-06-09 20:47   ` [PATCH v2 1/3] drivers: usb/core/urb: Add URB_FREE_COHERENT Rhett Aultman
2022-06-10  0:18     ` Vincent MAILHOL
2022-06-10 10:46       ` Marc Kleine-Budde
2022-06-09 20:47   ` [PATCH v2 2/3] drivers: usb/core/urb: allow URB_FREE_COHERENT Rhett Aultman
2022-06-09 23:18     ` Vincent Mailhol
2022-06-09 20:47   ` [PATCH v2 3/3] can: gs_usb: fix DMA memory leak on close Rhett Aultman
2022-06-10  0:05     ` Vincent Mailhol
2022-06-10  1:28       ` Vincent Mailhol
2022-06-10 21:33   ` [PATCH v3 0/2] URB_FREE_COHERENT gs_usb memory leak fix Rhett Aultman
2022-06-10 21:33     ` [PATCH v3 1/2] drivers: usb/core/urb: Add URB_FREE_COHERENT Rhett Aultman
2022-06-11 15:31       ` Marc Kleine-Budde
2022-06-11 16:06         ` Vincent MAILHOL
2022-06-21 13:51           ` Greg Kroah-Hartman
2022-06-21 14:59             ` Vincent MAILHOL
2022-06-21 15:03               ` Greg Kroah-Hartman
2022-06-21 15:54                 ` Vincent MAILHOL
2022-06-21 15:11               ` Alan Stern
2022-06-21 15:55                 ` Vincent MAILHOL
2022-06-21 16:14                   ` Alan Stern
2022-06-21 16:40                     ` Vincent MAILHOL
2022-06-21 17:00                       ` Greg Kroah-Hartman
2022-06-21 17:14                         ` Vincent MAILHOL
2022-06-21 17:46                           ` Greg Kroah-Hartman
2022-06-22  9:22                   ` David Laight [this message]
2022-06-22  9:41                     ` Greg Kroah-Hartman
2022-06-22 10:03                       ` David Laight
2022-06-22 11:11                         ` Oliver Neukum
2022-06-22 10:34                       ` Vincent MAILHOL
2022-06-22 12:23                         ` Greg Kroah-Hartman
2022-06-22 15:59                           ` Vincent MAILHOL
2022-06-22 18:11                             ` Rhett Aultman
2022-06-26  8:21                               ` Vincent MAILHOL
2022-06-27 19:24                                 ` Rhett Aultman
2022-06-28  1:09                                   ` Vincent MAILHOL
2022-07-04 13:02                                     ` Marc Kleine-Budde
2022-07-04 15:35                                       ` Rhett Aultman
2022-07-05  7:50                                         ` Marc Kleine-Budde
2022-06-23 17:30       ` Hongren Zenithal Zheng
2022-06-23 17:45         ` Shuah Khan
2022-06-24 14:43           ` Alan Stern
2022-06-24 16:01             ` Hongren Zenithal Zheng
2022-06-24 16:31             ` Shuah Khan
2022-06-24 18:07               ` Alan Stern
2022-06-27 22:54                 ` Shuah Khan
2022-06-28  1:35                   ` Alan Stern
2022-07-01  2:10                     ` Shuah Khan
2022-08-01 17:42                       ` Shuah Khan
2022-08-01 18:28                         ` Vincent MAILHOL
2022-08-03 23:44                           ` Shuah Khan
2022-06-10 21:33     ` [PATCH v3 2/2] can: gs_usb: fix DMA memory leak on close Rhett Aultman
2022-06-11 15:35       ` Marc Kleine-Budde
2022-06-11 16:03         ` Vincent MAILHOL
2022-06-12 21:28       ` David Laight
2022-06-12 21:33         ` Marc Kleine-Budde
2022-06-14 15:26     ` [PATCH v4 0/1] URB_FREE_COHERENT gs_usb memory leak fix Rhett Aultman
2022-06-14 15:26       ` [PATCH v4 1/1] can: gs_usb: fix DMA memory leak on close Rhett Aultman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=48caa879b0064ced97623bf1dad5b2d9@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mailhol.vincent@wanadoo.fr \
    --cc=mkl@pengutronix.de \
    --cc=oneukum@suse.com \
    --cc=rhett.aultman@samsara.com \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).