diff options
author | Dave Young <hidave.darkstar@gmail.com> | 2009-09-27 16:00:42 +0000 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2009-09-27 13:35:16 -0700 |
commit | f278a2f7bbc2239f479eaf63d0b3ae573b1d746c (patch) | |
tree | c768ba0ccfefa0be7d9e330b3d86100e0c48472b | |
parent | 569ec4cc779c8aae03a4659939d08822c9e4a242 (diff) |
tty: Fix regressions caused by commit b50989dc
The following commit made console open fails while booting:
commit b50989dc444599c8b21edc23536fc305f4e9b7d5
Author: Alan Cox <alan@linux.intel.com>
Date: Sat Sep 19 13:13:22 2009 -0700
tty: make the kref destructor occur asynchronously
Due to tty release routines run in a workqueue now, error like the
following will be reported while booting:
INIT open /dev/console Input/output error
It also causes hibernation regression to appear as reported at
http://bugzilla.kernel.org/show_bug.cgi?id=14229
The reason is that now there's latency issue with closing, but when
we open a "closing not finished" tty, -EIO will be returned.
Fix it as per the following Alan's suggestion:
Fun but it's actually not a bug and the fix is wrong in itself as
the port may be closing but not yet being destructed, in which case
it seems to do the wrong thing. Opening a tty that is closing (and
could be closing for long periods) is supposed to return -EIO.
I suspect a better way to deal with this and keep the old console
timing is to split tty->shutdown into two functions.
tty->shutdown() - called synchronously just before we dump the tty
onto the waitqueue for destruction
tty->cleanup() - called when the destructor runs.
We would then do the shutdown part which can occur in IRQ context
fine, before queueing the rest of the release (from tty->magic = 0
... the end) to occur asynchronously
The USB update in -next would then need a call like
if (tty->cleanup)
tty->cleanup(tty);
at the top of the async function and the USB shutdown to be split
between shutdown and cleanup as the USB resource cleanup and final
tidy cannot occur synchronously as it needs to sleep.
In other words the logic becomes
final kref put
make object unfindable
async
clean it up
Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
[ rjw: Rebased on top of 2.6.31-git, reworked the changelog. ]
Signed-off-by: "Rafael J. Wysocki" <rjw@sisk.pl>
[ Changed serial naming to match new rules, dropped tty_shutdown as per
comments from Alan Stern - Linus ]
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r-- | drivers/char/tty_io.c | 15 | ||||
-rw-r--r-- | drivers/usb/serial/usb-serial.c | 14 | ||||
-rw-r--r-- | include/linux/tty_driver.h | 13 |
3 files changed, 27 insertions, 15 deletions
diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c index ea18a129b0b..59499ee0fe6 100644 --- a/drivers/char/tty_io.c +++ b/drivers/char/tty_io.c @@ -1389,7 +1389,7 @@ EXPORT_SYMBOL(tty_shutdown); * of ttys that the driver keeps. * * This method gets called from a work queue so that the driver private - * shutdown ops can sleep (needed for USB at least) + * cleanup ops can sleep (needed for USB at least) */ static void release_one_tty(struct work_struct *work) { @@ -1397,10 +1397,9 @@ static void release_one_tty(struct work_struct *work) container_of(work, struct tty_struct, hangup_work); struct tty_driver *driver = tty->driver; - if (tty->ops->shutdown) - tty->ops->shutdown(tty); - else - tty_shutdown(tty); + if (tty->ops->cleanup) + tty->ops->cleanup(tty); + tty->magic = 0; tty_driver_kref_put(driver); module_put(driver->owner); @@ -1415,6 +1414,12 @@ static void release_one_tty(struct work_struct *work) static void queue_release_one_tty(struct kref *kref) { struct tty_struct *tty = container_of(kref, struct tty_struct, kref); + + if (tty->ops->shutdown) + tty->ops->shutdown(tty); + else + tty_shutdown(tty); + /* The hangup queue is now free so we can reuse it rather than waste a chunk of memory for each port */ INIT_WORK(&tty->hangup_work, release_one_tty); diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c index ff75a3589e7..aa6b2ae951a 100644 --- a/drivers/usb/serial/usb-serial.c +++ b/drivers/usb/serial/usb-serial.c @@ -192,7 +192,7 @@ void usb_serial_put(struct usb_serial *serial) * This is the first place a new tty gets used. Hence this is where we * acquire references to the usb_serial structure and the driver module, * where we store a pointer to the port, and where we do an autoresume. - * All these actions are reversed in serial_release(). + * All these actions are reversed in serial_cleanup(). */ static int serial_install(struct tty_driver *driver, struct tty_struct *tty) { @@ -339,15 +339,16 @@ static void serial_close(struct tty_struct *tty, struct file *filp) } /** - * serial_release - free resources post close/hangup + * serial_cleanup - free resources post close/hangup * @port: port to free up * * Do the resource freeing and refcount dropping for the port. * Avoid freeing the console. * - * Called when the last tty kref is dropped. + * Called asynchronously after the last tty kref is dropped, + * and the tty layer has already done the tty_shutdown(tty); */ -static void serial_release(struct tty_struct *tty) +static void serial_cleanup(struct tty_struct *tty) { struct usb_serial_port *port = tty->driver_data; struct usb_serial *serial; @@ -361,9 +362,6 @@ static void serial_release(struct tty_struct *tty) dbg("%s - port %d", __func__, port->number); - /* Standard shutdown processing */ - tty_shutdown(tty); - tty->driver_data = NULL; serial = port->serial; @@ -1210,7 +1208,7 @@ static const struct tty_operations serial_ops = { .chars_in_buffer = serial_chars_in_buffer, .tiocmget = serial_tiocmget, .tiocmset = serial_tiocmset, - .shutdown = serial_release, + .cleanup = serial_cleanup, .install = serial_install, .proc_fops = &serial_proc_fops, }; diff --git a/include/linux/tty_driver.h b/include/linux/tty_driver.h index 3566129384a..b0867798252 100644 --- a/include/linux/tty_driver.h +++ b/include/linux/tty_driver.h @@ -45,8 +45,16 @@ * * void (*shutdown)(struct tty_struct * tty); * - * This routine is called when a particular tty device is closed for - * the last time freeing up the resources. + * This routine is called synchronously when a particular tty device + * is closed for the last time freeing up the resources. + * + * + * void (*cleanup)(struct tty_struct * tty); + * + * This routine is called asynchronously when a particular tty device + * is closed for the last time freeing up the resources. This is + * actually the second part of shutdown for routines that might sleep. + * * * int (*write)(struct tty_struct * tty, * const unsigned char *buf, int count); @@ -233,6 +241,7 @@ struct tty_operations { int (*open)(struct tty_struct * tty, struct file * filp); void (*close)(struct tty_struct * tty, struct file * filp); void (*shutdown)(struct tty_struct *tty); + void (*cleanup)(struct tty_struct *tty); int (*write)(struct tty_struct * tty, const unsigned char *buf, int count); int (*put_char)(struct tty_struct *tty, unsigned char ch); |