2004-10-01 20:36:43

by Marcel Sebek

[permalink] [raw]
Subject: [2.6.9-rc3-bk1] Sleeping function called from invalid context

I upgraded from 2.6.8-rc2-bk10.
When I run pppd, a lot of debug messages are typed out.
pppd uses /dev/ttyUSB0 as serial device (driver ftdi_sio).

My .config is at http://wremcont.mysteria.cz/config-2.6.9-rc3-bk1
Kernel debug output is at http://wremcont.mysteria.cz/messages-2.6.9-rc3-bk1


lsusb:
Bus 001 Device 002: ID 0403:6001 Future Technology Devices International, Ltd 8-bit FIFO
Bus 001 Device 001: ID 0000:0000


lspci:
0000:00:00.0 Host bridge: VIA Technologies, Inc. VT8363/8365 [KT133/KM133] (rev 02)
0000:00:01.0 PCI bridge: VIA Technologies, Inc. VT8363/8365 [KT133/KM133 AGP]
0000:00:07.0 ISA bridge: VIA Technologies, Inc. VT82C686 [Apollo Super South] (rev 22)
0000:00:07.1 IDE interface: VIA Technologies, Inc. VT82C586A/B/VT82C686/A/B/VT823x/A/C PIPC Bus Master IDE (rev 10)
0000:00:07.2 USB Controller: VIA Technologies, Inc. VT82xxxxx UHCI USB 1.1 Controller (rev 10)
0000:00:07.4 Host bridge: VIA Technologies, Inc. VT82C686 [Apollo Super ACPI] (rev 30)
0000:00:07.5 Multimedia audio controller: VIA Technologies, Inc. VT82C686 AC97 Audio Controller (rev 20)
0000:01:00.0 VGA compatible controller: ATI Technologies Inc Rage 128 Pro Ultra TF


--
Marcel Sebek
jabber: [email protected] ICQ: 279852819
linux user number: 307850 GPG ID: 5F88735E
GPG FP: 0F01 BAB8 3148 94DB B95D 1FCA 8B63 CA06 5F88 735E


2004-10-02 03:59:55

by Andrew Morton

[permalink] [raw]
Subject: Re: [2.6.9-rc3-bk1] Sleeping function called from invalid context

[email protected] (Marcel Sebek) wrote:
>
> I upgraded from 2.6.8-rc2-bk10.
> When I run pppd, a lot of debug messages are typed out.
> pppd uses /dev/ttyUSB0 as serial device (driver ftdi_sio).
>
> My .config is at http://wremcont.mysteria.cz/config-2.6.9-rc3-bk1
> Kernel debug output is at http://wremcont.mysteria.cz/messages-2.6.9-rc3-bk1

OK, we have a problem.

Debug: sleeping function called from invalid context at mm/slab.c:2052
in_atomic():1, irqs_disabled():1
[__might_sleep+180/224] __might_sleep+0xb4/0xe0
[pg0+273102006/1069224960] hcd_submit_urb+0x106/0x190 [usbcore]
[kmem_cache_alloc+91/96] kmem_cache_alloc+0x5b/0x60
[pg0+273105606/1069224960] usb_control_msg+0x36/0xa0 [usbcore]
[pg0+272960692/1069224960] ftdi_set_termios+0x144/0x5f0 [ftdi_sio]
[pg0+273029492/1069224960] serial_set_termios+0x44/0xa0 [usbserial]
[tty_wait_until_sent+215/240] tty_wait_until_sent+0xd7/0xf0
[change_termios+480/528] change_termios+0x1e0/0x210
[set_termios+227/288] set_termios+0xe3/0x120
[tty_ioctl+1025/1248] tty_ioctl+0x401/0x4e0
[sys_ioctl+233/608] sys_ioctl+0xe9/0x260
[sysenter_past_esp+82/113] sysenter_past_esp+0x52/0x71
bad: scheduling while atomic!

In rc3 change_termios() was altered so that it calls
(*tty->driver->set_termios)() under spin_lock_irqsave(tty_termios_lock).

But ftdi_set_termios() wants to perform USB I/O, which involves sleeping
allocations all over the place.

There may be other ->set_termios() implementations which want to sleep, too.

2004-10-02 19:08:19

by Linus Torvalds

[permalink] [raw]
Subject: Re: [2.6.9-rc3-bk1] Sleeping function called from invalid context



On Fri, 1 Oct 2004, Andrew Morton wrote:
>
> In rc3 change_termios() was altered so that it calls
> (*tty->driver->set_termios)() under spin_lock_irqsave(tty_termios_lock).
>
> But ftdi_set_termios() wants to perform USB I/O, which involves sleeping
> allocations all over the place.

Alan, my first reaction was "that's just a buggy driver", but looking at
the reason for it (telling the device at the other end to change it's
speed), it's clearly not the driver. The new spinlock is just wrong.

We might be able to change it to a per-tty semaphore instead, which sounds
reasonably safe, but in the meantime I don't think we have any other
choice than to just leave the driver/ldisc routines unprotected. They
should be just reading the state, after all.

I'm committing this temporary fix..

Linus
----
===== drivers/char/tty_ioctl.c 1.13 vs edited =====
--- 1.13/drivers/char/tty_ioctl.c 2004-09-29 14:27:20 -07:00
+++ edited/drivers/char/tty_ioctl.c 2004-10-02 12:06:40 -07:00
@@ -145,6 +145,13 @@
wake_up_interruptible(&tty->link->read_wait);
}
}
+
+ /*
+ * Fixme! We should really try to protect the driver and ldisc
+ * termios usage too. But they need to be able to sleep, so
+ * the global termios spinlock is not the right thing.
+ */
+ spin_unlock_irqrestore(&tty_termios_lock, flags);

if (tty->driver->set_termios)
(*tty->driver->set_termios)(tty, &old_termios);
@@ -155,7 +162,6 @@
(ld->set_termios)(tty, &old_termios);
tty_ldisc_deref(ld);
}
- spin_unlock_irqrestore(&tty_termios_lock, flags);
}

static int set_termios(struct tty_struct * tty, void __user *arg, int opt)

2004-10-02 20:00:15

by Alan

[permalink] [raw]
Subject: Re: [2.6.9-rc3-bk1] Sleeping function called from invalid context

On Sad, 2004-10-02 at 20:08, Linus Torvalds wrote:
> We might be able to change it to a per-tty semaphore instead, which sounds
> reasonably safe, but in the meantime I don't think we have any other
> choice than to just leave the driver/ldisc routines unprotected. They
> should be just reading the state, after all.

Yep - already discussed on the linux-kernel and linux-usb lists. You can
do it with a semaphore. Its currently in testing. The USB drivers need
to sleep in order to handle the fact that their change is an
asynchronous message.

We do actually have to lock it or some drivers crash when you do the
fork 100 processes and all keep doing termios stuff attack.

Will send you a patch once its finished the test runs

Alan