Re: [PATCH 1/6] Bluetooth: Fix deadlock when closing socket

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Andrei -

On Fri, 7 Sep 2012, Andrei Emeltchenko wrote:

Hi Marcel,

On Thu, Sep 06, 2012 at 01:10:51PM -0700, Marcel Holtmann wrote:
Hi Mat,

If we have unacked frames when closing bluetooth socket we deadlock
since conn->chan_lock, chan->lock and socket lock are taken. Remove
__l2cap_wait_ack completely.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@xxxxxxxxx>

I don't think you want to remove this code completely, at least not
without giving some thought to the problem it is solving.

The problem is that programs may have an open socket which they send
some data on, then immediately close.  There is no feedback when data
is actually sent over the air, so the socket may end up getting torn
down while there is still data in the HCI tx buffer or some data was
lost and needs to be retransmitted.  Waiting for an acknowledgement
confirms that the application's sent data made it to the remote
device.

Without this code, it's difficult to use l2test on a number of
qualification tests.  Profiles or applications using ERTM may depend
on the "wait for ack before closing" behavior in order to have a clean
disconnect.

isn't that what we have SO_LINGER for?

Looking at the code I suspect that SO_LINGER is not working. Maybe we need
to merge linger code and wait_ack stuff.

It does look like a bug that the "wait_ack" behavior happens even without SO_LINGER.

In order to do SO_LINGER right, it would be better to check chan->tx_q instead of chan->unacked_frames to makes sure all data is sent and acked. chan->unacked_frames only tells you how many sent frames are unacked and does not take in to account queued data that hasn't been sent yet.

--
Mat Martineau

The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

--
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


[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux