Messages in this thread | | | Date | Tue, 18 Jul 2017 10:16:10 +0800 | From | jeffy <> | Subject | Re: [RFC PATCH] Bluetooth: btusb: Fix memory leak in play_deferred |
| |
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 > > > >
| |