* Kernel bug: bluetooth meets TTY layer
@ 2007-12-20 15:25 Arjan van de Ven
2007-12-20 19:40 ` David Newall
0 siblings, 1 reply; 4+ messages in thread
From: Arjan van de Ven @ 2007-12-20 15:25 UTC (permalink / raw)
To: Alan Cox, Marcel Holtmann; +Cc: linux-kernel
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Kernel bug: bluetooth meets TTY layer
2007-12-20 15:25 Kernel bug: bluetooth meets TTY layer Arjan van de Ven
@ 2007-12-20 19:40 ` David Newall
2007-12-20 20:17 ` Arjan van de Ven
0 siblings, 1 reply; 4+ messages in thread
From: David Newall @ 2007-12-20 19:40 UTC (permalink / raw)
To: Arjan van de Ven; +Cc: Alan Cox, Marcel Holtmann, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 714 bytes --]
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
[-- Attachment #2: hci_ldisc.c.patch --]
[-- Type: text/x-diff, Size: 513 bytes --]
--- hci_ldisc.c 2007-09-11 02:54:02.000000000 +0930
+++ hci_ldisc.c.new 2007-12-21 06:03:11.000000000 +1030
@@ -203,16 +203,17 @@
static int hci_uart_close(struct hci_dev *hdev)
{
BT_DBG("hdev %p", hdev);
if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
return 0;
hci_uart_flush(hdev);
+ hdev->flush = NULL;
return 0;
}
/* Send frames from HCI layer */
static int hci_uart_send_frame(struct sk_buff *skb)
{
struct hci_dev* hdev = (struct hci_dev *) skb->dev;
struct tty_struct *tty;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Kernel bug: bluetooth meets TTY layer
2007-12-20 19:40 ` David Newall
@ 2007-12-20 20:17 ` Arjan van de Ven
2007-12-20 21:26 ` Alan Cox
0 siblings, 1 reply; 4+ messages in thread
From: Arjan van de Ven @ 2007-12-20 20:17 UTC (permalink / raw)
To: David Newall; +Cc: Alan Cox, Marcel Holtmann, linux-kernel
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?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Kernel bug: bluetooth meets TTY layer
2007-12-20 20:17 ` Arjan van de Ven
@ 2007-12-20 21:26 ` Alan Cox
0 siblings, 0 replies; 4+ messages in thread
From: Alan Cox @ 2007-12-20 21:26 UTC (permalink / raw)
To: Arjan van de Ven; +Cc: David Newall, Marcel Holtmann, linux-kernel
On Thu, 20 Dec 2007 21:17:10 +0100
Arjan van de Ven <arjan@linux.intel.com> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2007-12-20 21:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-12-20 15:25 Kernel bug: bluetooth meets TTY layer Arjan van de Ven
2007-12-20 19:40 ` David Newall
2007-12-20 20:17 ` Arjan van de Ven
2007-12-20 21:26 ` Alan Cox
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).