Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1032848AbbKFISI (ORCPT ); Fri, 6 Nov 2015 03:18:08 -0500 Received: from mail-bn1on0137.outbound.protection.outlook.com ([157.56.110.137]:60288 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1031988AbbKFISF (ORCPT ); Fri, 6 Nov 2015 03:18:05 -0500 Authentication-Results: spf=fail (sender IP is 192.88.168.50) 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: Fri, 6 Nov 2015 16:15:57 +0800 From: Peter Chen To: Robert Baldyga CC: , , , , , , Subject: Re: [PATCH 03/23] usb: gadget: f_sourcesink: free requests in sourcesink_disable() Message-ID: <20151106081512.GD12560@shlinux2> References: <1446555242-3733-1-git-send-email-r.baldyga@samsung.com> <1446555242-3733-4-git-send-email-r.baldyga@samsung.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <1446555242-3733-4-git-send-email-r.baldyga@samsung.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-EOPAttributedMessage: 0 X-Microsoft-Exchange-Diagnostics: 1;BY2FFO11FD031;1:z6IF+UMnuXwt56Q7sd1YYDacMd5/50168Ng6sZDoxRj9xyV4n2pbKOJTRAqGHZKz5VMC/75qcJ1Kc1BeNMdpvRVPWt9citepQq4+lVO9O+yF4srJI7izHT6ggw/YMjNaOBKkpLvP0ZGT59wYQISgiKLUwLUGBeEOZAqtQJjlqwqvuMOeiAhz74itpGG4SgFAC0krkHS3pdqlwcu2g2N7rrGv9NhTBUk3EeOCgKscNL0qVIS4Jh5iR3TNCm2a2lRczf9qrfCiPp37lyaaCNRGBSjGrSOXf6yIcBagE3keWNiKsMbGFD13Clm6XmiubfEbFDfmW4xG5Qox94wTumlBJUeCc9D7BjwzkS6qD/hY+3Y+thTJEq11vADgUm4eaZn1 X-Forefront-Antispam-Report: CIP:192.88.168.50;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10019020)(6009001)(2980300002)(1109001)(1110001)(339900001)(199003)(189002)(24454002)(92566002)(50986999)(23726002)(47776003)(19580405001)(86362001)(85426001)(81156007)(83506001)(33716001)(46406003)(33656002)(106466001)(97756001)(50466002)(5001960100002)(105606002)(2950100001)(110136002)(11100500001)(76176999)(189998001)(97736004)(54356999)(5008740100001)(87936001)(4001350100001)(5007970100001)(19580395003)(104016004)(15975445007)(6806005)(77096005);DIR:OUT;SFP:1102;SCL:1;SRVR:BY2PR0301MB0632;H:tx30smr01.am.freescale.net;FPR:;SPF:Fail;PTR:InfoDomainNonexistent;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;BY2PR0301MB0632;2:/SdwrClhGW+eBdUH3nVT1QPnGRJWzgZR9HdHkwkbdGpohjisaREyeanMqMpXsM17ExR7fbZqEqCrVKX0FEtu7QcCcqsVcX3FNbauAA1xDouhsC8/E2oI0hCz/ctuCgR4q+trb6n/mWRa/xAhaBijITcLvENMKNPVo06wf/AlY5U=;3:N65RL+iUdWSGJbJJQlzrD7SmEOsSktmWOSRbh21VniI6oIbUm3Hr47XIpEqxHa+2ywr0ophgRQNrsDe/Rfc16gaBDjqzTw+3U8LM/u3fyUQxRxxHZkgWctZ+5TinvHbqy/mDsyrRcsU8oyWn6XRaRWsK3iDaUXtBTYokJWMgDBewdB/U7sekOzs2c5Zu/pXLdUrev1ph59kqgYFTITjJAK1+fnxCzC8PJZEckIH5TYxwSEg7Ge8rKoSHqkjNnPkhYAshFr8OAppd51eF8CYW3A==;25:uTFfLuSErofq49H1xXSHPr0dRjJX4FEXUIrMk2mNXCI1taJyLMiZz9fNgubGqJyOTq3h6gKM6j2XzUHDdjufwQF37HjoTdXTa1CZfoHRG4tITfMOmV/VLaKzNt13Vswex2BbqKMr4kVF6ulR+na9Jw3LiFCbLcAFJpvgINuV+sMRg/xx7InHPcVG3CrnCwZVl5sfj3l6GMD66IXnGNtb7GGBoWb1wPcwYzG6L+C44G1io9sKqGuY1piN6DDBh2nvHAXnEGcZW3IY2WnXy/ZWgg== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(42134001)(42139001);SRVR:BY2PR0301MB0632; X-Microsoft-Exchange-Diagnostics: 1;BY2PR0301MB0632;20:gNYhBEyDsmRKRDbxZ5SGUn2wQshtU+nifhowJcjzTlTpr6IxU4bVUN+Apfmt1JablWc6qcG+cFq6JWxIlckrDIExheRerZBotoin+yO5mEb4FlXQjoHIuk6E/7MUMj1ClQmuUPyOlYyLdJFsJcv+96IYrrqphmlgk4ohpLPQ6dmEh2/Kva+Gvy7y3zTbTYJL3vbOUIMQjo1T+oWBUnDuur+dbaDXLPawGfUMK6cZOY/sKtEoOl2iKzYyacRw3Vkk5P58bb+SEpDRLOFR8i0VL700IjEtUwLDw6z4DUuVoVdq4tZye+jVicLQndy3LrwdauXGAPfThLHWjTGXgHRtLx58IUK2rwkZJ0+MY82GD7o=;4:YgFWqQuQVK5v3b3yRxHBg+KLF7djSixpf+wAvX/k4hyEB4ibFIz+BmN/h+VinChRLK9+WS4UjIUtgi+lk66zSrCvAtLnlPXwZ0db3Jg/ffxxm/cb0dtmRTErgQE0cDzS1PudI4SZOytgGMRM5UgfDyuQiXdAxAUo5dTtDeGPEHOF5LH3X8sDXNdd5eV7ZDaELSlMwAKzAOR8wyWThXaGXipTiKt4xcCVw2hZQUrg+KgXmnXvu8dcq4jtGO76jx6QTFnwMlDE633Kshtp/X+CNnQgbyBRlPuBOi7OVy2J9sauo93lpU0xnqURHp8luw+CTLSjQ0745zu9CmlT7gHkWFd5EeF5swbZFgSDCtsOlW7VZdm9s9P7osBp15uEB3ay X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(520078)(8121501046)(5005006)(10201501046)(3002001);SRVR:BY2PR0301MB0632;BCL:0;PCL:0;RULEID:;SRVR:BY2PR0301MB0632; X-Forefront-PRVS: 07521929C1 X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;BY2PR0301MB0632;23:PcZR7eS/xZiwHIwBDDItpbjpN5O1XmfkOJee7gr?= =?us-ascii?Q?wozNjAfwQHGzP/+g2hBBVwloGlAODcO+rgrN6owhzzP4QjECC/mbC2h7GDdn?= =?us-ascii?Q?sgl+Seu9zbW7LpaW6iT9A6shQPC4NTg8FCV33/pSqCLpVnWg0NTKMBgebEAo?= =?us-ascii?Q?FcH+x779RzRSRNYiTqlWRq/HnSs9ZujGDkUSkB5k8NesRAahNC4aVX2J0DBj?= =?us-ascii?Q?SZvaZZdXU9GQSUjuZFM0OdjvtpyVk66nHqa+WTfiDDvPdgidUqHy6Pno42hW?= =?us-ascii?Q?b07f290DHElHfG4vNRPGxH+b31Zvk5ALUgyWHFr7bO+BvozMzsW5gz8frlRa?= =?us-ascii?Q?YIQWSWMVclnEeupXoM4fuOGOhHHD3CNvv1R+msq2WS/iBfzfVDkxLZW4EQqa?= =?us-ascii?Q?feTMlx8nXGclkWQwcZAqx1UxSZ0rWhFoDkCUdtSihklN+Q2S0Yed/VPsor1e?= =?us-ascii?Q?WECs3UTFv3I7PLmWzF2/BXM0lf4o7845lBPJ6qpvkSW523pKpP5K8vSLOQJQ?= =?us-ascii?Q?377nnaMevIi8ayU10ODHQAc1+Dh6mds/32q1588Ql44GrfDre/RhjpLjbqVU?= =?us-ascii?Q?5Iny8rCzi1ae8fTrZ1BNzTEfOG1kEQd97mM9aEZx1HtsJQDKDZYaOmBB3xVn?= =?us-ascii?Q?gaeeGayCcdTHrxGaSP0IGVU6/z8uwE0CFDpceUqCtP/BtMSBx2Ck/6i01ZDm?= =?us-ascii?Q?UYSuw+VbzBQ+HwMkVlzE9h9f5v61JOPtiJbtqyhERAE4x1drpBt/qIaZUV8f?= =?us-ascii?Q?GqcCakELqZQmw5Tro/hHNDBvZ/rNY4DmNbserIThPpWtDR/agYExRKU+aFCB?= =?us-ascii?Q?v1RSZcU+73yFjZ4CJwtQcP78u8jl3+XxQyH/sq6SX/l8D/a8fVt45e4RKO48?= =?us-ascii?Q?bKfY2PUCzt206eKAlEgndIq/naCx24Qo9qk6vFMNQ58UONl1u/+OG1OgHoGL?= =?us-ascii?Q?eoTZGkuZBVOaS4y9eTFXOjG3Gv0SNkvKJUVwnSq8OxP35gSlJsS6OpJVhoOP?= =?us-ascii?Q?PiBDUE0kGKVnbNtAW8KvDjSAiBpgejTZ/b9u/ukpHsydc87eebJoV/iiOuUZ?= =?us-ascii?Q?8g4vCwep2emm/PBs9jcjOTijIZqNkpNf/Fd/Y9mH164AmDCDAhw9HljLyPat?= =?us-ascii?Q?QQW4EzERN8AY=3D?= X-Microsoft-Exchange-Diagnostics: 1;BY2PR0301MB0632;5:4w3UItQT9EXOnKuGBkjqTY+Dy1G/b8Nl9y12zqTJieFHLRQtdppeCo49RHN9j0IRWGYUaQ9E/URoQsz0dV6/gfbPtbvDp5hCbXw1FLZFBJcTSUfXrJ+dm4AP8Nkq0Pwt0+G/YT2Rxp/h0cHUqgKzGw==;24:ZSJXZ+kVc8nR/YrrJKriRSMTzk4XI40M2OTOxrszpGnh3ymN5k9q35FdvdWgvhFkfPTA+FD1dGp0QayYAyBr/YaJiT98sxSv9LZUyx8FYDc=;20:x7YnhqrM4NKAgeN1+iD4LoVKe7uOIIQYRLxSkXQYV6GDnHJLRQCzuY26nWjqYD4IDbwq/N5JySrTd2pKwZLkQg== SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: freescale.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Nov 2015 08:18:01.3570 (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.168.50];Helo=[tx30smr01.am.freescale.net] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY2PR0301MB0632 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3693 Lines: 122 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. > /* > * SOURCE/SINK FUNCTION ... a primary testing vehicle for USB peripheral > * controller drivers. > @@ -51,6 +53,11 @@ struct f_sourcesink { > struct usb_ep *iso_out_ep; > int cur_alt; > > + struct usb_request *in_req; > + struct usb_request *out_req; > + struct usb_request *iso_in_req[REQ_CNT]; > + struct usb_request *iso_out_req[REQ_CNT]; > + > unsigned pattern; > unsigned isoc_interval; > unsigned isoc_maxpacket; > @@ -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. > return; > > case -EOVERFLOW: /* buffer overrun on read means that > @@ -613,7 +619,7 @@ static int source_sink_start_ep(struct f_sourcesink *ss, bool is_in, > ep = is_in ? ss->in_ep : ss->out_ep; > } > > - for (i = 0; i < 8; i++) { > + for (i = 0; i < REQ_CNT; i++) { > req = ss_alloc_ep_req(ep, size); > if (!req) > return -ENOMEM; > @@ -635,8 +641,18 @@ static int source_sink_start_ep(struct f_sourcesink *ss, bool is_in, > free_ep_req(ep, req); > } > > - if (!is_iso) > + if (is_iso) { > + if (is_in) > + ss->iso_in_req[i] = req; > + else > + ss->iso_out_req[i] = req; > + } else { > + if (is_in) > + ss->in_req = req; > + else > + ss->out_req = req; The req will be different for both bulk and iso, why you only have array for iso requests? > break; > + } > } > > return status; > @@ -764,8 +780,19 @@ static int sourcesink_get_alt(struct usb_function *f, unsigned intf) > static void sourcesink_disable(struct usb_function *f) > { > struct f_sourcesink *ss = func_to_ss(f); > + int i; > > disable_source_sink(ss); > + > + free_ep_req(ss->in_ep, ss->in_req); > + free_ep_req(ss->out_ep, ss->out_req); > + > + if (ss->iso_in_ep) { > + for (i = 0; i < REQ_CNT; i++) { > + free_ep_req(ss->iso_in_ep, ss->iso_in_req[i]); > + free_ep_req(ss->iso_out_ep, ss->iso_out_req[i]); > + } > + } > } > > /*-------------------------------------------------------------------------*/ > -- > 1.9.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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/