Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752247AbbKJChU (ORCPT ); Mon, 9 Nov 2015 21:37:20 -0500 Received: from mail-bl2on0142.outbound.protection.outlook.com ([65.55.169.142]:30641 "EHLO na01-bl2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751421AbbKJChS (ORCPT ); Mon, 9 Nov 2015 21:37:18 -0500 Authentication-Results: spf=fail (sender IP is 192.88.158.2) smtp.mailfrom=freescale.com; vger.kernel.org; dkim=none (message not signed) header.d=none;vger.kernel.org; dmarc=none action=none header.from=freescale.com; Date: Tue, 10 Nov 2015 10:02:57 +0800 From: Peter Chen To: Krzysztof Opasiak CC: Robert Baldyga , , , , , , , Subject: Re: [PATCH 03/23] usb: gadget: f_sourcesink: free requests in sourcesink_disable() Message-ID: <20151110020256.GA7558@shlinux2> 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> <563C79CF.1090403@samsung.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <563C79CF.1090403@samsung.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-EOPAttributedMessage: 0 X-Microsoft-Exchange-Diagnostics: 1;BL2FFO11FD006;1:UmxubeRwZYmvt/agIeYviVCKphYbXTs21YqtfwSHByIi4xY/C/OgYocX7jvECTanJ5hy67SzQUxuf4osEpyAhbX25wCYJ3/IWO3oD+swbpLaJagO2irEBPMQcEQOIdG8q0RtiDuXURAseOdO5FxBBcXPtnnAFedlsIRS7aYkVqPF2CsCTMhjj+R8/h8bevpVKTKmlMmUrqVRSgLrGFvc37bSJ0Za+Ex8X4rMoEVfuL4k5rP8p4L9e6VunxPjW/ineS0hab8awGQGLU7IzQ6ufMZlQZpalo5nSON2Qh8yw+FD2V1whMV+/tSdhmNT4Pfd3nswSYlfnwFK5uEo0LRLWg7VzIgJBm5QbFcPo9Mff15PJffY9GD2Pq+NOyJxDhrv X-Forefront-Antispam-Report: CIP:192.88.158.2;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10019020)(6009001)(2980300002)(1110001)(1109001)(339900001)(479174004)(377454003)(199003)(189002)(24454002)(19580395003)(5008740100001)(5001960100002)(23726002)(86362001)(81156007)(47776003)(46406003)(69596002)(19580405001)(4001350100001)(33716001)(92566002)(97736004)(105606002)(5007970100001)(6806005)(93886004)(54356999)(110136002)(11100500001)(50986999)(33656002)(106466001)(76176999)(97756001)(2950100001)(104016004)(50466002)(83506001)(189998001)(77096005)(87936001)(85426001);DIR:OUT;SFP:1102;SCL:1;SRVR:BLUPR03MB552;H:az84smr01.freescale.net;FPR:;SPF:Fail;PTR:InfoDomainNonexistent;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;BLUPR03MB552;2:pL/CiLZuuxCXx9O9fj6X7B1oNqT1Ci3wc0MPLke+9qovQsfA9t/CGNaMCqfrBG18ZjvLLSnkyhbbLP39PMKnTMeisE+di0B55ptso7cT/CmiYtz0VGpUGRPb35HmCZVTeJNGr2Ekw78gB47pB2lZWJrRjfeLPbpbWHPl445JHYA=;3:n6UgIb2KEkTbES1sC3UavZWxKUgWHYhVu/w2PdeEGV0uTl1yvBvUsHj7rGUgS6BLTHk3NnmXhS+4nrYsWKywEFZVUZAmKZ8HYu13ednzqxg2OuoOMvKbajO+lbhIeGQd+VNLpc3tPZyx+nOsAo4ip42+nyW70FcCBnV65Dbhy/ZJuAXZU4gfCGutBN+4Wz+F6nCC5wgSFucfrCPVOb79sAHlzQ0UhDa0iuyQD8xRgiw=;25:MNf3Obl9zntcXL9f+vA5c7Nma8sUbxSyWxHJx4SzWMJ9E8MtysQsyWT6+yWgH2MHwy/RU7BvtLbtcTMZzilrVaKXEjjI5Dr/8xDWBHXdKBlmCMYNMYDKqb6rjM5l7y1NqjP8SEZIOap7+UniSQYEij2jD9SlmVhIS43xlKmwXpvylKbxMz69ccDHP5PEt8yw5HOEIu6ZJAqqVbL6L/Yvke00ocYA4aAADe5/zhc8wCplBxWEM4ee8XmjmT5aJnvB0yHoywoaKDaPj9s451ayRQ== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BLUPR03MB552; X-Microsoft-Exchange-Diagnostics: 1;BLUPR03MB552;20:w+vhMLRACGPbZXZ0diZlS65N2MSIMxXJldRn0rEkNKM+8IM5riFqqHHyvi+sEnGHzjUfXb5NDMBNOx+IU2DBH6RRvI0gnMTKhBZcdK3qulr3o1lxLhRKXm4sgrB5QmD5zRwJo5I9wWdp5HWtwDoq7iHxLXY1t8RqkhRSYIARNW6mH7bPt2xbq94UWCP4eDJQOjqLMXfx8TmE4obbruay6mPI9eFHokJ/BQ65QVxWlnRLQoURCyalyntvUjq/VJRDeAUNigpgM8mbZwrcU5HGfpgyU6hdGNMZYaIR8eIqtt/LokQ9Vf26weh90sl7pp30gSTj9G5ITw3LcjrXMkMDA8Zw72ug19DU+pf7pYRh0qg=;4:hziIQ9BtpzoB2hYwiqzJfrd4hBsi1RQcpwrxgejxqVX5pq9hNd2nfqX8s3cAXI289zYPJxxxO7UkOhOwp2wtgS39c7TaMoaz8tuCg8qUBHf5NyW6U84jRj2Neu9bzgy9qx9zR4DqP9QxIvMS5MwAkh2zIpmCQHfa4RXDBIoac1ZgDItku2EcmVkQfBLFkEaSw9sW1iT9i1lcHsWXQ88/NFP349kOi9mRisKH4Sq33FB6LkK7BquIpyAR2IH63yPtD2pJi1YV02pH6McroMUFeMbGuc2q4HaJs+Ld1bGWSgXqFV/umRPYIsmPcm34d1mIm7kxebUzOJZacC+AiNDtadPv4+3afznL/VZ/nW28vvo2EHqTQDuRpZKUOO3xWHFE X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(5005006)(520078)(8121501046)(3002001)(10201501046);SRVR:BLUPR03MB552;BCL:0;PCL:0;RULEID:;SRVR:BLUPR03MB552; X-Forefront-PRVS: 07562C22DA X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;BLUPR03MB552;23:mzlkqGGW4KLRaT+Lkmrv2+p8Bf1O5fOsWQdwBUwn1v?= =?us-ascii?Q?tYpuyAQ2Ajh1BnXV7VbjmSJF3ZAEf/kiYfxCv6828iVKdj9v2Dw5AREqxd0z?= =?us-ascii?Q?ddA9fNsDW466X7h4pOIOSJ0l00ziV63iaxxg0HdHNDT0/SfSKn4JQ+tRIQ3t?= =?us-ascii?Q?uhKUiX9TdKI7CguF5qL0TfkLBbtW43cVG33iKgHMW2iGhBr5/4DBv8HOKwCR?= =?us-ascii?Q?OyGy/w30gPPcjguYIFYJnBpPoGGX7ahLdC4mN2kk1gI22w/4s2Qwhy4B5mJH?= =?us-ascii?Q?Enz36y4VKQFUoyQ3Qrt/Ht/5uViMe6KdrvAiwAFFkQVKnmE3MqRbcwhyKOsc?= =?us-ascii?Q?FC+8crgR9XjrtDMJknp8NftWQ+MmbpRtdXzWOwJFEHM5KOBRwufVJ4KwOpvS?= =?us-ascii?Q?MJljaDkAHs7a0/d8sqfkvP0Xk5fwAiLlDsJ4Gg05sjyjCgVgtEwvPPWWyBXM?= =?us-ascii?Q?M9c2VTKjG7hq2PcHGBuJ+WqRBbWw+KcqW60skcd23DINOt8M+GaUcbORYDuY?= =?us-ascii?Q?feDwyWgJKr2OSqGbKyhR07vZYx1ssUU1vs6WYiQN/Wuec2avrNnu9eiXTksG?= =?us-ascii?Q?7FfIIBWltbHhH+O2JAbspyDgKfEmLUGUsd+g0k9x2iWmBeycJg0oXy97dx9V?= =?us-ascii?Q?BceAHLROx9DsgJpDlMwlt4SLMohzF5WNsy6HRSk5uzlLeYdwdcNXpRYVZzp3?= =?us-ascii?Q?ckf+BooWqb9U+tIlpkT6GevXNgkkoBJS/7W8BNC2Tms+u7lmVqN7lmm6tZj1?= =?us-ascii?Q?W0bh3m7HvzUYG2SyNL8GZCu2NtJjbRenvaQcJ2NZ2Kag7+sglZI2AFHT36mU?= =?us-ascii?Q?5VvXrqjORyTJMP4i+72L1JUaFnFQpgySXnZN/8Td2U8eWEibiF4D6KE4LEy+?= =?us-ascii?Q?BsyXQClXcCY7mYhtva5Why9nsId+2czmtAdT5yWNEPcdkBNDuDCCnbG/M3Jl?= =?us-ascii?Q?nKpZmgkL3oqSghBzwmkRC9VECM05ALXAVVSU5gAI8hyLw4WecnrenQohdIyy?= =?us-ascii?Q?OBzBxjQZNUnBDpt1IvCyu6aFHZZZ2x5AWeYFrLeqDnuj1FOrX8aRovMLtw6S?= =?us-ascii?Q?yVQlStuHy1mVrF/snqFyJmISj2Aw7pRcOVZjnGWwKpwyJYfHiwFkxcilibj9?= =?us-ascii?Q?3/Q4MONp+4iab9c9wQVCBx835vkcXPIn4HaezZnyEh/saKiw5hhPRTZkV075?= =?us-ascii?Q?kKt5MLm0IY+JM=3D?= X-Microsoft-Exchange-Diagnostics: 1;BLUPR03MB552;5:Ty2JsDsjh7gjjOv1OBTz5heOb6gZ5/JK71je4tVSfEqApUGhNeRcliXeNFr4O4WETw37sRRBhWLmv29PQwz7HrkOxfOcuwKFt2SJCqlhksERfhIzAGqKDQyX2jgXltaBgXKbDhoTtHy1DS0/oDaEog==;24:XVqJ3dqbB0pkPLZVCdOCauW1HNrx8wBd8Hag4NGi6scUqKbEpEvUYrpIKoXwOL4KMcF7iMcIAJgIe54NL99XxkY6hEZfDJslSexV5/pSMuw=;20:QsqaKqdi9rvrkYnGVzb2LyWFwewQ75kMBLcH7kFrwSHO2gq8f5B/eg0UAOjjP/tFu1/TnjjYGCwJFq+A8ASbog== SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: freescale.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Nov 2015 02:05:14.0816 (UTC) X-MS-Exchange-CrossTenant-Id: 710a03f5-10f6-4d38-9ff4-a80b81da590d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=710a03f5-10f6-4d38-9ff4-a80b81da590d;Ip=[192.88.158.2];Helo=[az84smr01.freescale.net] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLUPR03MB552 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2791 Lines: 68 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 > >>>>--- > >>>> 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? -- Best Regards, Peter Chen -- 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/