Messages in this thread | | | Subject | Re: [PATCH 03/23] usb: gadget: f_sourcesink: free requests in sourcesink_disable() | From | Krzysztof Opasiak <> | Date | Fri, 06 Nov 2015 10:58:39 +0100 |
| |
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@samsung.com> >>>> --- >>>> 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.
Best regards, -- Krzysztof Opasiak Samsung R&D Institute Poland Samsung Electronics
| |