Hi Andrei -
On Thu, 24 May 2012, Andrei Emeltchenko wrote:
On Wed, May 23, 2012 at 08:44:22AM -0700, Mat Martineau wrote:
+static struct l2cap_chan *a2mp_chan_open(struct l2cap_conn *conn)
+{
+ struct l2cap_chan *chan;
+
+ chan = l2cap_chan_create();
+ if (!chan)
+ return NULL;
+
+ BT_DBG("chan %p", chan);
+
+ hci_conn_hold(conn->hcon);
Holding the hcon will keep the ACL open after all of the other L2CAP
channels have closed (unless I missed some code later in the patch
series). The A2MP fixed channel should not keep the ACL open. If
the connection is not held here, then there shouldn't be a put in
l2cap_chan_del for the A2MP channel either.
l2cap_chan_del makes hci_conn_put, also for a2mp channel. The behavior is
the same for fixed and normal channels.
And when does l2cap_chan_del get called for a fixed channel? The
fixed channel must not do an hci_conn_hold so the ACL is allowed to
close when all dynamic L2CAP channels have closed.
The current approach is to have amp_mgr for hci_conn. It will be freed
when in hci_conn_del together with other l2cap channels in
hci_chan_list_flush and then we make amp_mgr_put() and destroy mgr.
The idea is to make a2mp chan similar to other chans and other chans do
hci_conn_hold and hci_conn_put. Otherwise I would need to add extra checks
before hci_conn_put which is ugly.
I've tested this with PTS and no memory leaks found so far.
The risk is not memory leaks - if the other device closes the ACL,
then the A2MP channel will be cleaned up. But if you had two BlueZ
devices with A2MP connected to each other, the ACL would never close
as long as the devices remained powered and in range! Nothing would
trigger the flush as long as the hci_conn is held.
I think the checks for hci_conn_put are necessary, even if they are
ugly.
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
