On 03/15/2012 12:40 AM, Richard Weinberger wrote:
> Am 14.03.2012 23:28, schrieb Boaz Harrosh:
>> But surly it needs to go upstream. I'll run with it out of tree for now.
>
> As of now it's not ready for upstream.
> I'll submit it as -stable patch as soon as possible.
>
Richard hi,
I've been running with your tty patch for a while now, and was able
to work. 3.3 Kernel
Now based on 3.4 tree the patch no longer merges, too many changes.
But am back to the same crashes as before. Do you have a rough
patch for 3.4 base I can use.
Attached is the original patch good for 3.3 based tree.
Thanks in advance
Boaz
From b69b3ee7a4e6c745a711777a9f9492977e95d063 Mon Sep 17 00:00:00 2001
From: Richard Weinberger <R@W,org>
Date: Wed, 14 Mar 2012 11:46:36 -0700
Subject: [PATCH] TTY: tty_port questions
Hi!
While moving UML's console driver to tty_port some strange things happened.
So, I have a few questions. :-)
The original driver did not implement tty_operations->hangup().
If I implement it and call tty_port_hangup(), as Alan suggested,
the login fails on all TTYs except tty0.
It fails because the opened TTY returns EIO upon read()/write() after /bin/login
called vhangup().
The call chain is:
vhangup() -> tty_vhangup_self() -> tty_vhangup() -> __tty_hangup()
Within __tty_hangup() something happens that I don't fully understand:
if (cons_filp) {
if (tty->ops->close)
for (n = 0; n < closecount; n++)
tty->ops->close(tty, cons_filp);
} else if (tty->ops->hangup)
(tty->ops->hangup)(tty);
Login on tty0 works because cons_filp is not NULL and tty->ops->close() is
called.
On the other hand login fails on every other TTY because cons_filp remains NULL
and the TTY hangs up.
Is there something missing in my hangup function?
If I omit tty_operations->hangup() and leave it, like the old driver, NULL
non-tty0 TTYs cannot
be opened. (getty terminates immediately because it cannot open any TTY.)
open() retuns -EIO because the TTY_CLOSING is set in tty->flags.
How can this be?
Another fun fact is that the above noted problems do not happen on Debian.
Because Debian's /bin/login does not call vhangup().
Thanks,
//richard
--
---
arch/um/drivers/line.c | 150 ++++++++++++++++++++--------------------
arch/um/drivers/line.h | 13 ++--
arch/um/drivers/ssl.c | 23 +++---
arch/um/drivers/stdio_console.c | 10 ++-
4 files changed, 101 insertions(+), 95 deletions(-)
diff --git a/arch/um/drivers/line.c b/arch/um/drivers/line.c
index c1cf220..24de04c 100644
--- a/arch/um/drivers/line.c
+++ b/arch/um/drivers/line.c
@@ -19,19 +19,29 @@ static irqreturn_t line_interrupt(int irq, void *data)
{
struct chan *chan = data;
struct line *line = chan->line;
+ struct tty_struct *tty;
+
+ if (line) {
+ tty = tty_port_tty_get(&line->port);
+ chan_interrupt(&line->chan_list, &line->task, tty, irq);
+ tty_kref_put(tty);
+ }
- if (line)
- chan_interrupt(&line->chan_list, &line->task, line->tty, irq);
return IRQ_HANDLED;
}
static void line_timer_cb(struct work_struct *work)
{
struct line *line = container_of(work, struct line, task.work);
+ struct tty_struct *tty;
- if (!line->throttled)
- chan_interrupt(&line->chan_list, &line->task, line->tty,
+ if (!line->throttled) {
+ tty = tty_port_tty_get(&line->port);
+ chan_interrupt(&line->chan_list, &line->task, tty,
line->driver->read_irq);
+
+ tty_kref_put(tty);
+ }
}
/*
@@ -343,7 +353,7 @@ static irqreturn_t line_write_interrupt(int irq, void *data)
{
struct chan *chan = data;
struct line *line = chan->line;
- struct tty_struct *tty = line->tty;
+ struct tty_struct *tty = tty_port_tty_get(&line->port);
int err;
/*
@@ -354,6 +364,7 @@ static irqreturn_t line_write_interrupt(int irq, void *data)
spin_lock(&line->lock);
err = flush_buffer(line);
if (err == 0) {
+ tty_kref_put(tty);
return IRQ_NONE;
} else if (err < 0) {
line->head = line->buffer;
@@ -365,9 +376,36 @@ static irqreturn_t line_write_interrupt(int irq, void
*data)
return IRQ_NONE;
tty_wakeup(tty);
+ tty_kref_put(tty);
return IRQ_HANDLED;
}
+static int line_activate(struct tty_port *port, struct tty_struct *tty)
+{
+ int ret;
+ struct line *line = tty->driver_data;
+
+ ret = enable_chan(line);
+ if (ret)
+ return ret;
+
+ INIT_DELAYED_WORK(&line->task, line_timer_cb);
+
+ if (!line->sigio) {
+ chan_enable_winch(&line->chan_list, tty);
+ line->sigio = 1;
+ }
+
+ chan_window_size(&line->chan_list, &tty->winsize.ws_row,
+ &tty->winsize.ws_col);
+
+ return 0;
+}
+
+static const struct tty_port_operations line_port_ops = {
+ .activate = line_activate,
+};
+
int line_setup_irq(int fd, int input, int output, struct line *line, void
*data)
{
const struct line_driver *driver = line->driver;
@@ -404,81 +442,49 @@ int line_setup_irq(int fd, int input, int output, struct
line *line, void *data)
* first open or last close. Otherwise, open and close just return.
*/
-int line_open(struct line *lines, struct tty_struct *tty)
+int line_open(struct tty_struct *tty, struct file *filp)
{
- struct line *line = &lines[tty->index];
- int err = -ENODEV;
+ struct line *line = tty->driver_data;
+ return tty_port_open(&line->port, tty, filp);
+}
- spin_lock(&line->count_lock);
- if (!line->valid)
- goto out_unlock;
+int line_install(struct tty_driver *driver, struct tty_struct *tty, struct
line *line)
+{
+ int ret = tty_init_termios(tty);
+ if (ret)
+ return ret;
- err = 0;
- if (line->count++)
- goto out_unlock;
+ tty_driver_kref_get(driver);
+ tty->count++;
+ driver->ttys[tty->index] = tty;
- BUG_ON(tty->driver_data);
tty->driver_data = line;
- line->tty = tty;
-
- spin_unlock(&line->count_lock);
- err = enable_chan(line);
- if (err) /* line_close() will be called by our caller */
- return err;
-
- INIT_DELAYED_WORK(&line->task, line_timer_cb);
-
- if (!line->sigio) {
- chan_enable_winch(&line->chan_list, tty);
- line->sigio = 1;
- }
-
- chan_window_size(&line->chan_list, &tty->winsize.ws_row,
- &tty->winsize.ws_col);
return 0;
-
-out_unlock:
- spin_unlock(&line->count_lock);
- return err;
}
static void unregister_winch(struct tty_struct *tty);
-void line_close(struct tty_struct *tty, struct file * filp)
+void line_cleanup(struct tty_struct *tty)
{
struct line *line = tty->driver_data;
- /*
- * If line_open fails (and tty->driver_data is never set),
- * tty_open will call line_close. So just return in this case.
- */
- if (line == NULL)
- return;
-
- /* We ignore the error anyway! */
- flush_buffer(line);
-
- spin_lock(&line->count_lock);
- BUG_ON(!line->valid);
-
- if (--line->count)
- goto out_unlock;
-
- line->tty = NULL;
- tty->driver_data = NULL;
-
- spin_unlock(&line->count_lock);
-
if (line->sigio) {
unregister_winch(tty);
line->sigio = 0;
}
+}
- return;
+void line_close(struct tty_struct *tty, struct file * filp)
+{
+ struct line *line = tty->driver_data;
+ tty_port_close(&line->port, tty, filp);
+}
-out_unlock:
- spin_unlock(&line->count_lock);
+void line_hangup(struct tty_struct *tty)
+{
+ struct line *line = tty->driver_data;
+ tty_port_hangup(&line->port);
}
void close_lines(struct line *lines, int nlines)
@@ -495,13 +501,6 @@ static int setup_one_line(struct line *lines, int n, char
*init, int init_prio,
struct line *line = &lines[n];
int err = -EINVAL;
- spin_lock(&line->count_lock);
-
- if (line->count) {
- *error_out = "Device is already open";
- goto out;
- }
-
if (line->init_pri <= init_prio) {
line->init_pri = init_prio;
if (!strcmp(init, "none"))
@@ -512,8 +511,7 @@ static int setup_one_line(struct line *lines, int n, char
*init, int init_prio,
}
}
err = 0;
-out:
- spin_unlock(&line->count_lock);
+
return err;
}
@@ -598,6 +596,7 @@ int line_get_config(char *name, struct line *lines,
unsigned int num, char *str,
struct line *line;
char *end;
int dev, n = 0;
+ struct tty_struct *tty;
dev = simple_strtoul(name, &end, 0);
if ((*end != '\0') || (end == name)) {
@@ -612,13 +611,15 @@ int line_get_config(char *name, struct line *lines,
unsigned int num, char *str,
line = &lines[dev];
- spin_lock(&line->count_lock);
+ tty = tty_port_tty_get(&line->port);
+
if (!line->valid)
CONFIG_CHUNK(str, size, n, "none", 1);
- else if (line->tty == NULL)
+ else if (tty == NULL)
CONFIG_CHUNK(str, size, n, line->init_str, 1);
else n = chan_config_string(&line->chan_list, str, size, error_out);
- spin_unlock(&line->count_lock);
+
+ tty_kref_put(tty);
return n;
}
@@ -678,8 +679,8 @@ struct tty_driver *register_lines(struct line_driver
*line_driver,
}
for(i = 0; i < nlines; i++) {
- if (!lines[i].valid)
- tty_unregister_device(driver, i);
+ tty_port_init(&lines[i].port);
+ lines[i].port.ops = &line_port_ops;
}
mconsole_register_dev(&line_driver->mc);
@@ -805,7 +806,6 @@ void register_winch_irq(int fd, int tty_fd, int pid, struct
tty_struct *tty,
.pid = pid,
.tty = tty,
.stack = stack });
-
if (um_request_irq(WINCH_IRQ, fd, IRQ_READ, winch_interrupt,
IRQF_DISABLED | IRQF_SHARED | IRQF_SAMPLE_RANDOM,
"winch", winch) < 0) {
diff --git a/arch/um/drivers/line.h b/arch/um/drivers/line.h
index 63df3ca..54adfc6 100644
--- a/arch/um/drivers/line.h
+++ b/arch/um/drivers/line.h
@@ -31,9 +31,8 @@ struct line_driver {
};
struct line {
- struct tty_struct *tty;
- spinlock_t count_lock;
- unsigned long count;
+ struct tty_port port;
+
int valid;
char *init_str;
@@ -59,15 +58,17 @@ struct line {
};
#define LINE_INIT(str, d) \
- { .count_lock = __SPIN_LOCK_UNLOCKED((str).count_lock), \
- .init_str = str, \
+ { .init_str = str, \
.init_pri = INIT_STATIC, \
.valid = 1, \
.lock = __SPIN_LOCK_UNLOCKED((str).lock), \
.driver = d }
extern void line_close(struct tty_struct *tty, struct file * filp);
-extern int line_open(struct line *lines, struct tty_struct *tty);
+extern int line_open(struct tty_struct *tty, struct file *filp);
+extern int line_install(struct tty_driver *driver, struct tty_struct *tty,
struct line *line);
+extern void line_cleanup(struct tty_struct *tty);
+extern void line_hangup(struct tty_struct *tty);
extern int line_setup(struct line *lines, unsigned int sizeof_lines,
char *init, char **error_out);
extern int line_write(struct tty_struct *tty, const unsigned char *buf,
diff --git a/arch/um/drivers/ssl.c b/arch/um/drivers/ssl.c
index 9d8c20a..89fdecb 100644
--- a/arch/um/drivers/ssl.c
+++ b/arch/um/drivers/ssl.c
@@ -92,17 +92,6 @@ static int ssl_remove(int n, char **error_out)
error_out);
}
-static int ssl_open(struct tty_struct *tty, struct file *filp)
-{
- int err = line_open(serial_lines, tty);
-
- if (err)
- printk(KERN_ERR "Failed to open serial line %d, err = %d\n",
- tty->index, err);
-
- return err;
-}
-
#if 0
static void ssl_flush_buffer(struct tty_struct *tty)
{
@@ -124,8 +113,13 @@ void ssl_hangup(struct tty_struct *tty)
}
#endif
+static int ssl_install(struct tty_driver *driver, struct tty_struct *tty)
+{
+ return line_install(driver, tty, &serial_lines[tty->index]);
+}
+
static const struct tty_operations ssl_ops = {
- .open = ssl_open,
+ .open = line_open,
.close = line_close,
.write = line_write,
.put_char = line_put_char,
@@ -134,9 +128,12 @@ static const struct tty_operations ssl_ops = {
.flush_buffer = line_flush_buffer,
.flush_chars = line_flush_chars,
.set_termios = line_set_termios,
- .ioctl = line_ioctl,
+ .ioctl = line_ioctl,
.throttle = line_throttle,
.unthrottle = line_unthrottle,
+ .install = ssl_install,
+ .cleanup = line_cleanup,
+ .hangup = line_hangup,
#if 0
.stop = ssl_stop,
.start = ssl_start,
diff --git a/arch/um/drivers/stdio_console.c b/arch/um/drivers/stdio_console.c
index 088776f..4c78c78 100644
--- a/arch/um/drivers/stdio_console.c
+++ b/arch/um/drivers/stdio_console.c
@@ -97,7 +97,7 @@ static int con_remove(int n, char **error_out)
static int con_open(struct tty_struct *tty, struct file *filp)
{
- int err = line_open(vts, tty);
+ int err = line_open(tty, filp);
if (err)
printk(KERN_ERR "Failed to open console %d, err = %d\n",
tty->index, err);
@@ -105,11 +105,17 @@ static int con_open(struct tty_struct *tty, struct file
*filp)
return err;
}
+static int con_install(struct tty_driver *driver, struct tty_struct *tty)
+{
+ return line_install(driver, tty, &vts[tty->index]);
+}
+
/* Set in an initcall, checked in an exitcall */
static int con_init_done = 0;
static const struct tty_operations console_ops = {
.open = con_open,
+ .install = con_install,
.close = line_close,
.write = line_write,
.put_char = line_put_char,
@@ -121,6 +127,8 @@ static const struct tty_operations console_ops = {
.ioctl = line_ioctl,
.throttle = line_throttle,
.unthrottle = line_unthrottle,
+ .cleanup = line_cleanup,
+ .hangup = line_hangup,
};
static void uml_console_write(struct console *console, const char *string,
--
1.7.10.2.677.gb6bc67f
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel
