From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751607AbdGRCQQ (ORCPT ); Mon, 17 Jul 2017 22:16:16 -0400 Received: from regular1.263xmail.com ([211.150.99.135]:34558 "EHLO regular1.263xmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751351AbdGRCQP (ORCPT ); Mon, 17 Jul 2017 22:16:15 -0400 X-263anti-spam: KSV:0;BIG:0; X-MAIL-GRAY: 0 X-MAIL-DELIVERY: 1 X-KSVirus-check: 0 X-ADDR-CHECKED4: 1 X-ABS-CHECKED: 1 X-SKE-CHECKED: 1 X-ANTISPAM-LEVEL: 2 X-RL-SENDER: jeffy.chen@rock-chips.com X-FST-TO: oneukum@suse.com X-SENDER-IP: 103.29.142.67 X-LOGIN-NAME: jeffy.chen@rock-chips.com X-UNIQUE-TAG: <2c3c14101d998133c19cc831760f1e61> X-ATTACHMENT-NUM: 0 X-DNS-TYPE: 0 Message-ID: <596D6F6A.1090004@rock-chips.com> Date: Tue, 18 Jul 2017 10:16:10 +0800 From: jeffy User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:19.0) Gecko/20130126 Thunderbird/19.0 MIME-Version: 1.0 To: Oliver Neukum , Marcel Holtmann CC: LKML , xiyou.wangcong@gmail.com, Brian Norris , Douglas Anderson , Johan Hedberg , "Gustavo F. Padovan" Subject: Re: [RFC PATCH] Bluetooth: btusb: Fix memory leak in play_deferred References: <1498126243-4771-1-git-send-email-jeffy.chen@rock-chips.com> <65CF80EF-EA58-4126-889D-07A8FF9D52DB@holtmann.org> <594C8F0D.4000100@rock-chips.com> <1499168300.17946.3.camel@neukum.org> <596588F8.5070402@rock-chips.com> <1500305216.16072.2.camel@suse.com> In-Reply-To: <1500305216.16072.2.camel@suse.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Oliver, Thanks for your reply. On 07/17/2017 11:26 PM, Oliver Neukum wrote: > Am Mittwoch, den 12.07.2017, 10:27 +0800 schrieb jeffy: >> Hi Oliver, >> >> Thanx for your comments, and sorry for reply late. >> >> >>> If you do that you have to change submit_tx_urb() to be called under a >>> spinlock. >> >> sorry, why we need that? since submit_tx_urb is basically >> usb_anchor_urb/usb_submit_urb/usb_free_urb > > You need to fix the GFP_KERNEL therein. oh, i see the problem. > >>>> or referenced, but the caller would unref it himself >>>> later? >>> >>> The caller is responsible for its own references. >> hmm, maybe unref it in the complete callback(btusb_tx_complete?), and if >> we do so, we may need to detect which urb came from here... > > I do not get your reasoning there. If an URB has executed, it belongs > onto the anchor for URBs to be used again. the urbs we submit here are referenced but unanchored, so i think we can: 1/ unreference it here and put it in tx_anchor, and let urb core to do the unachor(and unreference) or 2/ we unreference it in the complete callback. i'll send a new version for 2/ > >>>> and for tx_anchor, we put urb in it, and kill them all during suspending >>>> to prevent transfer. so i guess it would be safe to put deferred urb in >>>> to it after resume too? >>>> but i don't know much about usb/btusb, so i could be wrong all about that :) >>> >>> IIRC the reason for directly submitting them was the spinlock. >> sorry, i'm not clear about this, could you help to explain more? do you >> mean txlock? > > Yes > > Regards > Oliver > > > >