Re: [PATCH 03/23] usb: gadget: f_sourcesink: free requests in sourcesink_disable()

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

 



Hi,

On 11/10/2015 03:02 AM, Peter Chen wrote:
On Fri, Nov 06, 2015 at 10:58:39AM +0100, Krzysztof Opasiak wrote:


On 11/06/2015 10:48 AM, Peter Chen wrote:
On Fri, Nov 06, 2015 at 09:50:11AM +0100, Robert Baldyga wrote:
On 11/06/2015 09:15 AM, Peter Chen wrote:
On Tue, Nov 03, 2015 at 01:53:42PM +0100, Robert Baldyga wrote:
USB requests in SourceSink function are allocated in sourcesink_get_alt()
function, so we prefer to free them rather in sourcesink_disable() than
in source_sink_complete() when request is completed with error. It provides
better symetry in resource management and improves code readability.

Signed-off-by: Robert Baldyga <r.baldyga@xxxxxxxxxxx>
---
  drivers/usb/gadget/function/f_sourcesink.c | 33 +++++++++++++++++++++++++++---
  1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/function/f_sourcesink.c b/drivers/usb/gadget/function/f_sourcesink.c
index a8b68c6..d8f5f56 100644
--- a/drivers/usb/gadget/function/f_sourcesink.c
+++ b/drivers/usb/gadget/function/f_sourcesink.c
@@ -22,6 +22,8 @@
  #include "g_zero.h"
  #include "u_f.h"

+#define REQ_CNT	8
+

It would be buffer if we can have module parameter for this like
qlen at f_loopback.
@@ -561,7 +568,6 @@ static void source_sink_complete(struct usb_ep *ep, struct usb_request *req)
  				req->actual, req->length);
  		if (ep == ss->out_ep)
  			check_read_data(ss, req);
-		free_ep_req(ep, req);

I find the current code may cause memory leak, since above code
is only called one time.


I don't see what you mean. Can you explain a bit more in which situation
can memory leak take place?

Oh, sorry, I have not seen there is a "break;" at the source_sink_start_ep for
none-iso endpoint, so, I am wrong, no memory leak here.

I have tried changing bulk for 8 requests, the performance improves
greatly, but still a little problem for OUT,  I will see what's the matter.
Besides, it will be better we can have a qlen parameters like f_loopback,
in that case, we can improve the performance for gadget, and get the best performance
when testing with usbtest, I will do it later.


Moreover, I would suggest to add qlen and iso_qlen params not only
qlen as even now we are using different qlen for bulk and iso.


Krzysztof, would you know why we have different qlen for bulk and iso now?


Well, in my opinion iso and bulk use different buf sizes and simply different number of requests is needed to saturate the bandwidth.

--
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



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

  Powered by Linux