On 07/21/2013 04:08 AM, Gianluca Anzolin wrote:
On Sat, Jul 20, 2013 at 10:11:50AM -0400, Peter Hurley wrote:
Sorry Gianluca, I should have been more specific here.
There's no need to test for dlc->state == BT_CLOSED in carrier_raised().
At the point where port->carrier_raised is called, the tty will have been
linked with the file descriptor, so if the dlc->state goes to BT_CLOSED,
then rfcomm_dev_state_change() will call
tty_hangup() -> driver hangup() -> tty_port_hangup() -> tty_port_shutdown()
This call chain will
1. set the file_ops to hung_up_tty_fops which will cause tty_hung_up_p() to
return true
2. clear ASYNCB_INITIALIZED in port->flags
3. wakeup port->open_wait
So an open() parked in the schedule loop of tty_port_block_til_ready()
will wake and exit the loop with either -EAGAIN or -ERESTARTSYS.
rfcomm_dev_state_change() should only do a wakeup on port->open_wait when
dlc->state == BT_CONNECTED.
In case of success I should also call some device_move,
rfcomm_tty_copy_pending and rfcomm_dlc_unthrottle. Could I do it in
carrier_raised directly?
I wouldn't. That would be a nasty hack and a potential problem if a
signal occurred.
The device_move() isn't dependent on success, and can stay in .activate().
I changed the code and it's cleaner than before, very nice. However the
device_move() is really dependent on success: the parent device is there only
when the connection has been successfully established.
Oh, right.
So I have to call that function after the carrier is raised, or right before.
Since you already told me that calling it in the .carrier_raised method is
unwise the only place left is the state_change callback of the dlc.
That seems fine. [ Another place would be in rfcomm_tty_open() just after
tty_port_open() returns success -- the tty is still locked here so it won't
race with .close/.hangup() ]
I do wonder why the tty device is re-parented to the host controller device.
It's obviously not for subsystem teardown. Maybe one of the bluetooth
maintainers could clarify this? Are there userspace components waiting for
this uevent?
Conversely in .shutdown the check for the device parent == NULL takes care of
the scenario in which the aforementioned device_move() is never called.
Another unrelated question: I'm working on the rfcomm_dev_add function to avoid
the two nested locks. When the patch is ready should I send it separately or
can I include it with the other patches?
Your preference.
Just a reminder: the dlc lock will still need to be dropped after bumping the
rfcomm_dev ref count via rfcomm_dev_get(), because when you subsequently drop
both references, rfcomm_dev destruction will attempt to gain the dlc lock,
resulting in deadlock. [Or do it as a work item]
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
