Tomasz Wroblewski
2013-Aug-27 09:03 UTC
[PATCH] Fix inactive timer list corruption on second S3 resume
init_timer cannot be safely called multiple times on same timer since it does memset(0) on the structure, erasing the auxiliary member used by linked list code. This breaks inactive timer list in common/timer.c. Moved resume_timer initialisation to ns16550_init_postirq, so it''s only done once. Signed-off-by: Tomasz Wroblewski <tomasz.wroblewski@citrix.com> --- xen/drivers/char/ns16550.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c index 6082c85..ce35df0 100644 --- a/xen/drivers/char/ns16550.c +++ b/xen/drivers/char/ns16550.c @@ -59,6 +59,8 @@ static struct ns16550 { u8 bar_idx; } ns16550_com[2] = { { 0 } }; +static void ns16550_delayed_resume(void *data); + static char ns_read_reg(struct ns16550 *uart, int reg) { if ( uart->remapped_io_base == NULL ) @@ -256,6 +258,7 @@ static void __init ns16550_init_postirq(struct serial_port *port) serial_async_transmit(port); init_timer(&uart->timer, ns16550_poll, port, 0); + init_timer(&uart->resume_timer, ns16550_delayed_resume, port, 0); /* Calculate time to fill RX FIFO and/or empty TX FIFO for polling. */ bits = uart->data_bits + uart->stop_bits + !!uart->parity; @@ -346,7 +349,6 @@ static void ns16550_resume(struct serial_port *port) if ( ns16550_ioport_invalid(uart) ) { delayed_resume_tries = RESUME_RETRIES; - init_timer(&uart->resume_timer, ns16550_delayed_resume, port, 0); set_timer(&uart->resume_timer, NOW() + RESUME_DELAY); } else -- 1.7.10.4
Jan Beulich
2013-Aug-27 09:09 UTC
Re: [PATCH] Fix inactive timer list corruption on second S3 resume
>>> On 27.08.13 at 11:03, Tomasz Wroblewski <tomasz.wroblewski@citrix.com> wrote: > init_timer cannot be safely called multiple times on same timer since it does > memset(0) > on the structure, erasing the auxiliary member used by linked list code. > This breaks > inactive timer list in common/timer.c. > > Moved resume_timer initialisation to ns16550_init_postirq, so it''s only done > once. > > Signed-off-by: Tomasz Wroblewski <tomasz.wroblewski@citrix.com>Reviewed-by: Jan Beulich <jbeulich@suse.com> You should also have Cc-ed Keir, who''ll need to approve the patch. Jan> --- > xen/drivers/char/ns16550.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c > index 6082c85..ce35df0 100644 > --- a/xen/drivers/char/ns16550.c > +++ b/xen/drivers/char/ns16550.c > @@ -59,6 +59,8 @@ static struct ns16550 { > u8 bar_idx; > } ns16550_com[2] = { { 0 } }; > > +static void ns16550_delayed_resume(void *data); > + > static char ns_read_reg(struct ns16550 *uart, int reg) > { > if ( uart->remapped_io_base == NULL ) > @@ -256,6 +258,7 @@ static void __init ns16550_init_postirq(struct serial_port *port) > serial_async_transmit(port); > > init_timer(&uart->timer, ns16550_poll, port, 0); > + init_timer(&uart->resume_timer, ns16550_delayed_resume, port, 0); > > /* Calculate time to fill RX FIFO and/or empty TX FIFO for polling. */ > bits = uart->data_bits + uart->stop_bits + !!uart->parity; > @@ -346,7 +349,6 @@ static void ns16550_resume(struct serial_port *port) > if ( ns16550_ioport_invalid(uart) ) > { > delayed_resume_tries = RESUME_RETRIES; > - init_timer(&uart->resume_timer, ns16550_delayed_resume, port, 0); > set_timer(&uart->resume_timer, NOW() + RESUME_DELAY); > } > else > -- > 1.7.10.4
Keir Fraser
2013-Aug-27 18:03 UTC
Re: [PATCH] Fix inactive timer list corruption on second S3 resume
On 27/08/2013 10:03, "Tomasz Wroblewski" <tomasz.wroblewski@citrix.com> wrote:> init_timer cannot be safely called multiple times on same timer since it does > memset(0) > on the structure, erasing the auxiliary member used by linked list code. This > breaks > inactive timer list in common/timer.c. > > Moved resume_timer initialisation to ns16550_init_postirq, so it''s only done > once. > > Signed-off-by: Tomasz Wroblewski <tomasz.wroblewski@citrix.com>Acked-by: Keir Fraser <keir@xen.org>> --- > xen/drivers/char/ns16550.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c > index 6082c85..ce35df0 100644 > --- a/xen/drivers/char/ns16550.c > +++ b/xen/drivers/char/ns16550.c > @@ -59,6 +59,8 @@ static struct ns16550 { > u8 bar_idx; > } ns16550_com[2] = { { 0 } }; > > +static void ns16550_delayed_resume(void *data); > + > static char ns_read_reg(struct ns16550 *uart, int reg) > { > if ( uart->remapped_io_base == NULL ) > @@ -256,6 +258,7 @@ static void __init ns16550_init_postirq(struct serial_port > *port) > serial_async_transmit(port); > > init_timer(&uart->timer, ns16550_poll, port, 0); > + init_timer(&uart->resume_timer, ns16550_delayed_resume, port, 0); > > /* Calculate time to fill RX FIFO and/or empty TX FIFO for polling. */ > bits = uart->data_bits + uart->stop_bits + !!uart->parity; > @@ -346,7 +349,6 @@ static void ns16550_resume(struct serial_port *port) > if ( ns16550_ioport_invalid(uart) ) > { > delayed_resume_tries = RESUME_RETRIES; > - init_timer(&uart->resume_timer, ns16550_delayed_resume, port, 0); > set_timer(&uart->resume_timer, NOW() + RESUME_DELAY); > } > else