Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1033176AbbKFJ6r (ORCPT ); Fri, 6 Nov 2015 04:58:47 -0500 Received: from mailout2.w1.samsung.com ([210.118.77.12]:33079 "EHLO mailout2.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1031997AbbKFJ6n (ORCPT ); Fri, 6 Nov 2015 04:58:43 -0500 X-AuditID: cbfec7f4-f79c56d0000012ee-35-563c79d0884f Subject: Re: [PATCH 03/23] usb: gadget: f_sourcesink: free requests in sourcesink_disable() To: Peter Chen , Robert Baldyga References: <1446555242-3733-1-git-send-email-r.baldyga@samsung.com> <1446555242-3733-4-git-send-email-r.baldyga@samsung.com> <20151106081512.GD12560@shlinux2> <563C69C3.2030102@samsung.com> <20151106094844.GA2809@shlinux2> Cc: balbi@ti.com, gregkh@linuxfoundation.org, andrzej.p@samsung.com, m.szyprowski@samsung.com, b.zolnierkie@samsung.com, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org From: Krzysztof Opasiak Message-id: <563C79CF.1090403@samsung.com> Date: Fri, 06 Nov 2015 10:58:39 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-version: 1.0 In-reply-to: <20151106094844.GA2809@shlinux2> Content-type: text/plain; charset=windows-1252; format=flowed Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrCLMWRmVeSWpSXmKPExsVy+t/xa7oXKm3CDJ4/kbaY9bKdxWLjjPWs Fgfv11s0L17PZnF51xw2i0XLWpkt1h65y25xbPZfJosHh3eyO3B6/Dvcz+Sxf+4ado++LasY PY7f2M7k8XmTXABrFJdNSmpOZllqkb5dAlfG+w3PWQu6hCtunLrD2MB4mr+LkZNDQsBEYvuX I+wQtpjEhXvr2boYuTiEBJYySvRem8sCkhASeM4o8Wi9EIgtLBArMeXkHqAGDg4RgWCJxRMS IOqfMkrs2byCEcRhFtjCKLH+/jUWkCI2AX2JebtEQXp5BbQkvj47zQZiswioSpy5ugBssahA hMSps2/ZIGoEJX5Mvge2l1NAV+Lmu1tgcWYBW4kF79exQNjyEpvXvGWewCgwC0nLLCRls5CU LWBkXsUomlqaXFCclJ5rqFecmFtcmpeul5yfu4kREvZfdjAuPmZ1iFGAg1GJh/fGEuswIdbE suLK3EOMEhzMSiK8csw2YUK8KYmVValF+fFFpTmpxYcYpTlYlMR55+56HyIkkJ5YkpqdmlqQ WgSTZeLglGpgTElsOqIq9Xf6DkufWsUlqzZUCLLOF7uS9uTB9bfzfCre6Zi2/3e+LCF3NtLF 0z7TjUXu5oWzPhY5B5+dTv4xm5P7sLPo5KWctaE/Js052PiII8bG0/Jo5codX3/c/r98qX/o 1dZLK07cVtu857TH87L0LzMVzPhkAlY8MmvodNjp13BehP/4ASWW4oxEQy3mouJEAEkX4cp3 AgAA Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2631 Lines: 65 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 >>>> --- >>>> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/