Message ID | 20220506154612.c8ccdc2fe91f.Idae14b9661fd047f8cac695f0d95e321da70955b@changeid |
---|---|
State | Accepted |
Headers | show |
Series | um: line: use separate IRQs per line | expand |
On 06/05/2022 14:46, Johannes Berg wrote: > From: Johannes Berg <johannes.berg@intel.com> > > Today, all possible serial lines (ssl*=) as well as all > possible consoles (con*=) each share a single interrupt > (with a fixed number) with others of the same type. > > Now, if you have two lines, say ssl0 and ssl1, and one > of them is connected to an fd you cannot read (e.g. a > file), but the other gets a read interrupt, then both > of them get the interrupt since it's shared. Then, the > read() call will return EOF, since it's a file being > written and there's nothing to read (at least not at > the current offset, at the end). > > Unfortunately, this is treated as a read error, and we > close this line, losing all the possible output. > > It might be possible to work around this and make the > IRQ sharing work, however, now that we have dynamically > allocated IRQs that are easy to use, simply use that to > achieve separating between the events; then there's no > interrupt for that line and we never attempt the read > in the first place, thus not closing the line. > > This manifested itself in the wifi hostap/hwsim tests > where the parallel script communicates via one serial > console and the kernel messages go to another (a file) > and sending data on the communication console caused > the kernel messages to stop flowing into the file. > > Reported-by: Jouni Malinen <j@w1.fi> > Signed-off-by: Johannes Berg <johannes.berg@intel.com> > --- > arch/um/drivers/chan_kern.c | 10 +++++----- > arch/um/drivers/line.c | 22 +++++++++++++--------- > arch/um/drivers/line.h | 4 ++-- > arch/um/drivers/ssl.c | 2 -- > arch/um/drivers/stdio_console.c | 2 -- > arch/um/include/asm/irq.h | 22 +++++++++------------- > 6 files changed, 29 insertions(+), 33 deletions(-) > > diff --git a/arch/um/drivers/chan_kern.c b/arch/um/drivers/chan_kern.c > index 62997055c454..26a702a06515 100644 > --- a/arch/um/drivers/chan_kern.c > +++ b/arch/um/drivers/chan_kern.c > @@ -133,7 +133,7 @@ static void line_timer_cb(struct work_struct *work) > struct line *line = container_of(work, struct line, task.work); > > if (!line->throttled) > - chan_interrupt(line, line->driver->read_irq); > + chan_interrupt(line, line->read_irq); > } > > int enable_chan(struct line *line) > @@ -195,9 +195,9 @@ void free_irqs(void) > chan = list_entry(ele, struct chan, free_list); > > if (chan->input && chan->enabled) > - um_free_irq(chan->line->driver->read_irq, chan); > + um_free_irq(chan->line->read_irq, chan); > if (chan->output && chan->enabled) > - um_free_irq(chan->line->driver->write_irq, chan); > + um_free_irq(chan->line->write_irq, chan); > chan->enabled = 0; > } > } > @@ -215,9 +215,9 @@ static void close_one_chan(struct chan *chan, int delay_free_irq) > spin_unlock_irqrestore(&irqs_to_free_lock, flags); > } else { > if (chan->input && chan->enabled) > - um_free_irq(chan->line->driver->read_irq, chan); > + um_free_irq(chan->line->read_irq, chan); > if (chan->output && chan->enabled) > - um_free_irq(chan->line->driver->write_irq, chan); > + um_free_irq(chan->line->write_irq, chan); > chan->enabled = 0; > } > if (chan->ops->close != NULL) > diff --git a/arch/um/drivers/line.c b/arch/um/drivers/line.c > index 8febf95da96e..02b0befd6763 100644 > --- a/arch/um/drivers/line.c > +++ b/arch/um/drivers/line.c > @@ -139,7 +139,7 @@ static int flush_buffer(struct line *line) > count = line->buffer + LINE_BUFSIZE - line->head; > > n = write_chan(line->chan_out, line->head, count, > - line->driver->write_irq); > + line->write_irq); > if (n < 0) > return n; > if (n == count) { > @@ -156,7 +156,7 @@ static int flush_buffer(struct line *line) > > count = line->tail - line->head; > n = write_chan(line->chan_out, line->head, count, > - line->driver->write_irq); > + line->write_irq); > > if (n < 0) > return n; > @@ -195,7 +195,7 @@ int line_write(struct tty_struct *tty, const unsigned char *buf, int len) > ret = buffer_data(line, buf, len); > else { > n = write_chan(line->chan_out, buf, len, > - line->driver->write_irq); > + line->write_irq); > if (n < 0) { > ret = n; > goto out_up; > @@ -215,7 +215,7 @@ void line_throttle(struct tty_struct *tty) > { > struct line *line = tty->driver_data; > > - deactivate_chan(line->chan_in, line->driver->read_irq); > + deactivate_chan(line->chan_in, line->read_irq); > line->throttled = 1; > } > > @@ -224,7 +224,7 @@ void line_unthrottle(struct tty_struct *tty) > struct line *line = tty->driver_data; > > line->throttled = 0; > - chan_interrupt(line, line->driver->read_irq); > + chan_interrupt(line, line->read_irq); > } > > static irqreturn_t line_write_interrupt(int irq, void *data) > @@ -260,19 +260,23 @@ int line_setup_irq(int fd, int input, int output, struct line *line, void *data) > int err; > > if (input) { > - err = um_request_irq(driver->read_irq, fd, IRQ_READ, > - line_interrupt, IRQF_SHARED, > + err = um_request_irq(UM_IRQ_ALLOC, fd, IRQ_READ, > + line_interrupt, 0, > driver->read_irq_name, data); > if (err < 0) > return err; > + > + line->read_irq = err; > } > > if (output) { > - err = um_request_irq(driver->write_irq, fd, IRQ_WRITE, > - line_write_interrupt, IRQF_SHARED, > + err = um_request_irq(UM_IRQ_ALLOC, fd, IRQ_WRITE, > + line_write_interrupt, 0, > driver->write_irq_name, data); > if (err < 0) > return err; > + > + line->write_irq = err; > } > > return 0; > diff --git a/arch/um/drivers/line.h b/arch/um/drivers/line.h > index bdb16b96e76f..f15be75a3bf3 100644 > --- a/arch/um/drivers/line.h > +++ b/arch/um/drivers/line.h > @@ -23,9 +23,7 @@ struct line_driver { > const short minor_start; > const short type; > const short subtype; > - const int read_irq; > const char *read_irq_name; > - const int write_irq; > const char *write_irq_name; > struct mc_device mc; > struct tty_driver *driver; > @@ -35,6 +33,8 @@ struct line { > struct tty_port port; > int valid; > > + int read_irq, write_irq; > + > char *init_str; > struct list_head chan_list; > struct chan *chan_in, *chan_out; > diff --git a/arch/um/drivers/ssl.c b/arch/um/drivers/ssl.c > index 41eae2e8fb65..8514966778d5 100644 > --- a/arch/um/drivers/ssl.c > +++ b/arch/um/drivers/ssl.c > @@ -47,9 +47,7 @@ static struct line_driver driver = { > .minor_start = 64, > .type = TTY_DRIVER_TYPE_SERIAL, > .subtype = 0, > - .read_irq = SSL_IRQ, > .read_irq_name = "ssl", > - .write_irq = SSL_WRITE_IRQ, > .write_irq_name = "ssl-write", > .mc = { > .list = LIST_HEAD_INIT(driver.mc.list), > diff --git a/arch/um/drivers/stdio_console.c b/arch/um/drivers/stdio_console.c > index e8b762f4d8c2..489d5a746ed3 100644 > --- a/arch/um/drivers/stdio_console.c > +++ b/arch/um/drivers/stdio_console.c > @@ -53,9 +53,7 @@ static struct line_driver driver = { > .minor_start = 0, > .type = TTY_DRIVER_TYPE_CONSOLE, > .subtype = SYSTEM_TYPE_CONSOLE, > - .read_irq = CONSOLE_IRQ, > .read_irq_name = "console", > - .write_irq = CONSOLE_WRITE_IRQ, > .write_irq_name = "console-write", > .mc = { > .list = LIST_HEAD_INIT(driver.mc.list), > diff --git a/arch/um/include/asm/irq.h b/arch/um/include/asm/irq.h > index e187c789369d..749dfe8512e8 100644 > --- a/arch/um/include/asm/irq.h > +++ b/arch/um/include/asm/irq.h > @@ -4,19 +4,15 @@ > > #define TIMER_IRQ 0 > #define UMN_IRQ 1 > -#define CONSOLE_IRQ 2 > -#define CONSOLE_WRITE_IRQ 3 > -#define UBD_IRQ 4 > -#define UM_ETH_IRQ 5 > -#define SSL_IRQ 6 > -#define SSL_WRITE_IRQ 7 > -#define ACCEPT_IRQ 8 > -#define MCONSOLE_IRQ 9 > -#define WINCH_IRQ 10 > -#define SIGIO_WRITE_IRQ 11 > -#define TELNETD_IRQ 12 > -#define XTERM_IRQ 13 > -#define RANDOM_IRQ 14 > +#define UBD_IRQ 2 > +#define UM_ETH_IRQ 3 > +#define ACCEPT_IRQ 4 > +#define MCONSOLE_IRQ 5 > +#define WINCH_IRQ 6 > +#define SIGIO_WRITE_IRQ 7 > +#define TELNETD_IRQ 8 > +#define XTERM_IRQ 9 > +#define RANDOM_IRQ 10 > > #ifdef CONFIG_UML_NET_VECTOR > > Acked-By: anton ivanov <anton.ivanov@cambridgegreys.com>
diff --git a/arch/um/drivers/chan_kern.c b/arch/um/drivers/chan_kern.c index 62997055c454..26a702a06515 100644 --- a/arch/um/drivers/chan_kern.c +++ b/arch/um/drivers/chan_kern.c @@ -133,7 +133,7 @@ static void line_timer_cb(struct work_struct *work) struct line *line = container_of(work, struct line, task.work); if (!line->throttled) - chan_interrupt(line, line->driver->read_irq); + chan_interrupt(line, line->read_irq); } int enable_chan(struct line *line) @@ -195,9 +195,9 @@ void free_irqs(void) chan = list_entry(ele, struct chan, free_list); if (chan->input && chan->enabled) - um_free_irq(chan->line->driver->read_irq, chan); + um_free_irq(chan->line->read_irq, chan); if (chan->output && chan->enabled) - um_free_irq(chan->line->driver->write_irq, chan); + um_free_irq(chan->line->write_irq, chan); chan->enabled = 0; } } @@ -215,9 +215,9 @@ static void close_one_chan(struct chan *chan, int delay_free_irq) spin_unlock_irqrestore(&irqs_to_free_lock, flags); } else { if (chan->input && chan->enabled) - um_free_irq(chan->line->driver->read_irq, chan); + um_free_irq(chan->line->read_irq, chan); if (chan->output && chan->enabled) - um_free_irq(chan->line->driver->write_irq, chan); + um_free_irq(chan->line->write_irq, chan); chan->enabled = 0; } if (chan->ops->close != NULL) diff --git a/arch/um/drivers/line.c b/arch/um/drivers/line.c index 8febf95da96e..02b0befd6763 100644 --- a/arch/um/drivers/line.c +++ b/arch/um/drivers/line.c @@ -139,7 +139,7 @@ static int flush_buffer(struct line *line) count = line->buffer + LINE_BUFSIZE - line->head; n = write_chan(line->chan_out, line->head, count, - line->driver->write_irq); + line->write_irq); if (n < 0) return n; if (n == count) { @@ -156,7 +156,7 @@ static int flush_buffer(struct line *line) count = line->tail - line->head; n = write_chan(line->chan_out, line->head, count, - line->driver->write_irq); + line->write_irq); if (n < 0) return n; @@ -195,7 +195,7 @@ int line_write(struct tty_struct *tty, const unsigned char *buf, int len) ret = buffer_data(line, buf, len); else { n = write_chan(line->chan_out, buf, len, - line->driver->write_irq); + line->write_irq); if (n < 0) { ret = n; goto out_up; @@ -215,7 +215,7 @@ void line_throttle(struct tty_struct *tty) { struct line *line = tty->driver_data; - deactivate_chan(line->chan_in, line->driver->read_irq); + deactivate_chan(line->chan_in, line->read_irq); line->throttled = 1; } @@ -224,7 +224,7 @@ void line_unthrottle(struct tty_struct *tty) struct line *line = tty->driver_data; line->throttled = 0; - chan_interrupt(line, line->driver->read_irq); + chan_interrupt(line, line->read_irq); } static irqreturn_t line_write_interrupt(int irq, void *data) @@ -260,19 +260,23 @@ int line_setup_irq(int fd, int input, int output, struct line *line, void *data) int err; if (input) { - err = um_request_irq(driver->read_irq, fd, IRQ_READ, - line_interrupt, IRQF_SHARED, + err = um_request_irq(UM_IRQ_ALLOC, fd, IRQ_READ, + line_interrupt, 0, driver->read_irq_name, data); if (err < 0) return err; + + line->read_irq = err; } if (output) { - err = um_request_irq(driver->write_irq, fd, IRQ_WRITE, - line_write_interrupt, IRQF_SHARED, + err = um_request_irq(UM_IRQ_ALLOC, fd, IRQ_WRITE, + line_write_interrupt, 0, driver->write_irq_name, data); if (err < 0) return err; + + line->write_irq = err; } return 0; diff --git a/arch/um/drivers/line.h b/arch/um/drivers/line.h index bdb16b96e76f..f15be75a3bf3 100644 --- a/arch/um/drivers/line.h +++ b/arch/um/drivers/line.h @@ -23,9 +23,7 @@ struct line_driver { const short minor_start; const short type; const short subtype; - const int read_irq; const char *read_irq_name; - const int write_irq; const char *write_irq_name; struct mc_device mc; struct tty_driver *driver; @@ -35,6 +33,8 @@ struct line { struct tty_port port; int valid; + int read_irq, write_irq; + char *init_str; struct list_head chan_list; struct chan *chan_in, *chan_out; diff --git a/arch/um/drivers/ssl.c b/arch/um/drivers/ssl.c index 41eae2e8fb65..8514966778d5 100644 --- a/arch/um/drivers/ssl.c +++ b/arch/um/drivers/ssl.c @@ -47,9 +47,7 @@ static struct line_driver driver = { .minor_start = 64, .type = TTY_DRIVER_TYPE_SERIAL, .subtype = 0, - .read_irq = SSL_IRQ, .read_irq_name = "ssl", - .write_irq = SSL_WRITE_IRQ, .write_irq_name = "ssl-write", .mc = { .list = LIST_HEAD_INIT(driver.mc.list), diff --git a/arch/um/drivers/stdio_console.c b/arch/um/drivers/stdio_console.c index e8b762f4d8c2..489d5a746ed3 100644 --- a/arch/um/drivers/stdio_console.c +++ b/arch/um/drivers/stdio_console.c @@ -53,9 +53,7 @@ static struct line_driver driver = { .minor_start = 0, .type = TTY_DRIVER_TYPE_CONSOLE, .subtype = SYSTEM_TYPE_CONSOLE, - .read_irq = CONSOLE_IRQ, .read_irq_name = "console", - .write_irq = CONSOLE_WRITE_IRQ, .write_irq_name = "console-write", .mc = { .list = LIST_HEAD_INIT(driver.mc.list), diff --git a/arch/um/include/asm/irq.h b/arch/um/include/asm/irq.h index e187c789369d..749dfe8512e8 100644 --- a/arch/um/include/asm/irq.h +++ b/arch/um/include/asm/irq.h @@ -4,19 +4,15 @@ #define TIMER_IRQ 0 #define UMN_IRQ 1 -#define CONSOLE_IRQ 2 -#define CONSOLE_WRITE_IRQ 3 -#define UBD_IRQ 4 -#define UM_ETH_IRQ 5 -#define SSL_IRQ 6 -#define SSL_WRITE_IRQ 7 -#define ACCEPT_IRQ 8 -#define MCONSOLE_IRQ 9 -#define WINCH_IRQ 10 -#define SIGIO_WRITE_IRQ 11 -#define TELNETD_IRQ 12 -#define XTERM_IRQ 13 -#define RANDOM_IRQ 14 +#define UBD_IRQ 2 +#define UM_ETH_IRQ 3 +#define ACCEPT_IRQ 4 +#define MCONSOLE_IRQ 5 +#define WINCH_IRQ 6 +#define SIGIO_WRITE_IRQ 7 +#define TELNETD_IRQ 8 +#define XTERM_IRQ 9 +#define RANDOM_IRQ 10 #ifdef CONFIG_UML_NET_VECTOR