Hi Mat, On Mon, May 28, 2012 at 10:24:56AM +0300, Andrei Emeltchenko wrote: > > >>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. > > > > > >Does it indicate that we have problem with rather freeing amp_mgr? > > > > It looks like the amp_mgr will be freed if the channel is closed. > > There's nothing that will close the channel except for: > > > > * Remote device disconnects the ACL > > * Remote device goes out of range > > * Local BR/EDR controller is powered down or reset > > > > >>I think the checks for hci_conn_put are necessary, even if they are > > >>ugly. > > > > > >I think that if I have hci_conn_hold when creating channel and > > >hci_conn_put when closing channel that does not differ from the case when > > >I do not have hci_conn_put/hold. > > > > If there was something that closed the A2MP channel, there would be > > no difference. But nothing closes the A2MP channel. > > I actually was thinking about timers which could do clean up. > > > >So you are probably referring to the case when hci_conn needs to be closed > > >but cannot because it has hci_conn_hold in a2mp channel? > > > > Yes. > > > > >This also means that we still have L2CAP channel (a2mp) hanging. > > >This seems right to me if I did not miss something. > > > > Since A2MP is a fixed channel, it is not opened and closed like > > typical L2CAP channels. Consider this scenario: > > > > 1. Remote device decides to connect > > 2. ACL is created > > 3. Remote device sends A2MP discover > > 4. A2MP channel is set up, AMP manager is created, hci_conn_hold for > > A2MP channel > > 5. Remote device connects L2CAP channel over BR/EDR. hci_conn_hold > > for the data channel > > 6. A2MP communication to set up physical link > > 7. L2CAP channel move to AMP controller > > 8. Data sent over AMP. > > 9. L2CAP channel disconnected, hci_conn_put for the data channel > > 10. Device stays in range. hci_conn reference count is not 0, so > > ACL is not closed. > > My concern here if A2MP does not reference hci_conn can hci_conn be > removed while we still have A2MP chan in some situation? I just tested code with PTS and got one crash: <------8<-------------------------------------------------------------------- | [ 275.100760] [5] hci_chan_del: hci0 conn f3ff0800 chan e818e540 | [ 275.109610] [19] l2cap_send_rr_or_rnr: chan e806b800, poll 0 | [ 275.127922] [19] l2cap_send_sframe: chan e806b800, control f5ef3ed8 | [ 275.133532] [19] l2cap_send_sframe: reqseq 2, final 0, poll 0, super 0 | [ 275.173033] [5] hci_conn_del: hci0 conn f3ff0800 handle 11 | [ 275.181131] [5] hci_chan_list_flush: conn f3ff0800 | [ 275.202645] [5] amp_mgr_put: mgr e8260600 <------8<-------------------------------------------------------------------- Here we deleted the last L2CAP channel <------8<------------------------------------------------------------------ | [ 275.295020] BUG: unable to handle kernel NULL pointer dereference at | (null) | [ 275.298791] IP: [<f8362e6c>] l2cap_do_send+0x1c/0xa0 [bluetooth] | [ 275.298791] *pde = 00000000 | [ 275.298791] Oops: 0000 [#1] SMP <------8<------------------------------------------------------------------ Then we got the crash probably hci_conn deleted faster then we get reply from AMP controller <------8<--------------------------------------------------------------------- | [ 275.298791] EIP: [<f8362e6c>] l2cap_do_send+0x1c/0xa0 [bluetooth] | SS:ESP 0068:f5ef3e58 | [ 275.298791] CR2: 0000000000000000 | [ 276.553113] Bluetooth: hci1 command tx timeout | [ 276.553706] ---[ end trace b08f23ddb2b49a2e ]--- | [ 276.577494] [30] hci_queue_cb: Queue cmd ed3d5540 opt e8260600 | [ 276.723885] [30] l2cap_segment_sdu: chan e806b800, msg f5919e94, len 22 | [ 276.790990] BUG: unable to handle kernel NULL pointer dereference at | 00000010 | [ 276.794865] IP: [<f83694dd>] l2cap_chan_send+0x28d/0xb40 [bluetooth] <------8<--------------------------------------------------------------------- Best regards Andrei Emeltchenko -- 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
