Hi,
with the help of kerneloops.org I've spotted a nice little interaction
between the TTY layer and the bluetooth code, however the tty layer is
not something I'm all too familiar with so I rather ask than brute-force
fix the code incorrectly.
The raw details are at:
http://www.kerneloops.org/search.php?search=uart_flush_buffer
What happens is that, on closing the bluetooth tty,
the tty layer goes into the release_dev() function,
which first does a bunch of stuff, then sets the file->private_data to NULL,
does some more stuff and then calls the ldisc close function. Which in this
case, is hci_uart_tty_close().
Now, hci_uart_tty_close() calls hci_uart_close() which clears some internal bit,
and then calls hci_uart_flush()... which calls back to the tty layers' uart_flush_buffer() function.
(in drivers/bluetooth/hci_tty.c around line 194)
Which then WARN_ON()'s because that's not allowed/supposed to be called
this late in the shutdown of the port....
should the bluetooth driver even call this flush function at all??
Greetings,
Arjan van de Ven
Hi Arjan,
I've not been able to find this file, "drivers/bluetooth/hci_tty.c", but
anyway, This seems to be what happens: Hci_uart_close() flushes using
hci_uart_flush(). Subsequently, in hci_dev_do_close(), (one step in
hci_unregister_dev()), hci_uart_flush() is called again. The comment in
uart_flush_buffer(), relating to the WARN_ON(), indicates you can't
flush after the port is closed; which sounds reasonable. I think
hci_uart_close() should set hdev->flush to NULL before returning.
Hci_dev_do_close() does check for this. The code path is rather
involved and I'm not entirely clear of all steps, but I think that's
what should be done.
Patch for stupidly obsolete kernel attached.
David
David Newall wrote:
> Hi Arjan,
>
> I've not been able to find this file, "drivers/bluetooth/hci_tty.c", but
> anyway, This seems to be what happens: Hci_uart_close() flushes using
> hci_uart_flush(). Subsequently, in hci_dev_do_close(), (one step in
> hci_unregister_dev()), hci_uart_flush() is called again. The comment in
> uart_flush_buffer(), relating to the WARN_ON(), indicates you can't
> flush after the port is closed; which sounds reasonable. I think
> hci_uart_close() should set hdev->flush to NULL before returning.
> Hci_dev_do_close() does check for this. The code path is rather
> involved and I'm not entirely clear of all steps, but I think that's
> what should be done.
>
> Patch for stupidly obsolete kernel attached.
looks reasonable; unfortunately I don't know the tty code well enough to judge this patch...
Alan?
On Thu, 20 Dec 2007 21:17:10 +0100
Arjan van de Ven <[email protected]> wrote:
> David Newall wrote:
> > Hi Arjan,
> >
> > I've not been able to find this file, "drivers/bluetooth/hci_tty.c", but
> > anyway, This seems to be what happens: Hci_uart_close() flushes using
> > hci_uart_flush(). Subsequently, in hci_dev_do_close(), (one step in
> > hci_unregister_dev()), hci_uart_flush() is called again. The comment in
> > uart_flush_buffer(), relating to the WARN_ON(), indicates you can't
> > flush after the port is closed; which sounds reasonable. I think
> > hci_uart_close() should set hdev->flush to NULL before returning.
> > Hci_dev_do_close() does check for this. The code path is rather
> > involved and I'm not entirely clear of all steps, but I think that's
> > what should be done.
> >
> > Patch for stupidly obsolete kernel attached.
>
> looks reasonable; unfortunately I don't know the tty code well enough to judge this patch...
> Alan?
I don't know the bluetooth code well enough to even guess and I've not
had time to study this one.
Alan