On 12/15/2013 10:08 AM, Gianluca Anzolin wrote:
On Sun, Dec 15, 2013 at 09:03:35AM -0500, Peter Hurley wrote:
On 12/15/2013 06:24 AM, Gianluca Anzolin wrote:
On Fri, Dec 13, 2013 at 12:35:26AM +0100, Alexander Holler wrote:
Am 12.12.2013 21:36, schrieb Peter Hurley:
What currently happens is that when one kills rfcomm (and any other
terminal which might use that tty), the entry in /dev doesn't
disappear. That means the same call to refcomm with the same device
(e.g. [/dev/]rfcomm1 doesn't work.
Thanks for the report, Alexander.
Point 4 above details a different situation; something else is
happening.
Would you please detail the necessary steps to reproduce this regression?
(How do you 'kill' rfcomm? etc. Shell command lines would be best.)
Just call
rfcomm connect rfcomm9 01:23:45:67:89:ab
wait until the connection happened (a message will appear) and then
press ctrl-c. This still terminates the bluetooth connection, but the
device in /dev is now left.
Yes I'm able to reproduce the regression which is indeed caused by that
commit.
However I'm puzzled. Surely there is a fifth case I didn't cover because
when rfcomm_dev_state_change() is called, the tty_port is there but the tty is
not, and therefore I cannot get a reference to it and send the HUP.
There is a fifth case, but it's crazy.
The tty has been properly shutdown and destroyed because the tty file handle
was closed, not hungup. The rfcomm device reference was properly put
when the tty was released.
But when the remote hangs up (and sends disc), then rfcomm_dev_state_change()
is called -- to kill the port reference (thus the rfcomm device) that was
instantiated locally! Ridiculous. Doubly ridiculous because it's the local
port shutdown that closes the dlc locally that sends the disconnect (and sets
the local dlc state) that triggers the received rfcomm_dev_state_change()!
If this behavior is desirable (or necessary because it's been exposed to
userspace), then why was the design ever reference-counted to begin with?
Regards,
Peter Hurley
The attached patch fixes the regression by releasing the tty_port in the
shutdown method(). This way we can avoid strange games in the dlc callback
where we are constrained by the dlc lock.
If this kind of approach is acceptable I will submit the patch for inclusion in
a separate email.
This solution is acceptable to me, but I think the comment should briefly
explain why this fix is necessary, and the changelog should explain why in detail.
Perhaps with a fixme comment that rfcomm_tty_install() should just take over
the port reference (instead of adding one) and rfcomm_tty_cleanup() should
conditionally release on RFCOMM_RELEASE_ONHUP.
Because then:
1) this fix would not be necessary.
2) the release in rfcomm_tty_hangup() would not be necessary
3) the second release in rfcomm_release_dev would not be necessary
4) the RFCOMM_TTY_RELEASED bit could be removed
Regards,
Peter Hurley
--
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
