[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1500305216.16072.2.camel@suse.com>
Date: Mon, 17 Jul 2017 17:26:56 +0200
From: Oliver Neukum <oneukum@...e.com>
To: jeffy <jeffy.chen@...k-chips.com>,
Marcel Holtmann <marcel@...tmann.org>
Cc: LKML <linux-kernel@...r.kernel.org>, xiyou.wangcong@...il.com,
Brian Norris <briannorris@...omium.org>,
Douglas Anderson <dianders@...omium.org>,
Johan Hedberg <johan.hedberg@...il.com>,
"Gustavo F. Padovan" <gustavo@...ovan.org>
Subject: Re: [RFC PATCH] Bluetooth: btusb: Fix memory leak in play_deferred
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.
> > > 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.
> > > 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
Powered by blists - more mailing lists