| Message ID | 20111108214458.28884.86759.stgit@miche.sea.corp.google.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Tue, 08 Nov 2011 13:44:58 -0800, Miche Baker-Harvey <miche@google.com> wrote: > Some modifications of vtermno were not done under the spinlock. > > Moved assignment from vtermno and increment of vtermno together, > putting both under the spinlock. Revert vtermno on failure. > > Signed-off-by: Miche Baker-Harvey <miche@google.com> Does it matter? It's normal not to lock in a function called "init_XXX", since it's not exposed yet. Or is it? Thanks, Rusty.
On (Fri) 11 Nov 2011 [14:57:20], Rusty Russell wrote: > On Tue, 08 Nov 2011 13:44:58 -0800, Miche Baker-Harvey <miche@google.com> wrote: > > Some modifications of vtermno were not done under the spinlock. > > > > Moved assignment from vtermno and increment of vtermno together, > > putting both under the spinlock. Revert vtermno on failure. > > > > Signed-off-by: Miche Baker-Harvey <miche@google.com> > > Does it matter? It's normal not to lock in a function called > "init_XXX", since it's not exposed yet. > > Or is it? Slight misnomer, I suppose. We do this init_console_port() as part of add_port() if the port is a console port. Should it be named 'mark_console_port()'? Dunno, doesn't sound like the right name. init fits closest. Amit
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 8e3c46d..9722e76 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -987,18 +987,21 @@ int init_port_console(struct port *port) * pointers. The final argument is the output buffer size: we * can do any size, so we put PAGE_SIZE here. */ - port->cons.vtermno = pdrvdata.next_vtermno; + spin_lock_irq(&pdrvdata_lock); + port->cons.vtermno = pdrvdata.next_vtermno++; + spin_unlock_irq(&pdrvdata_lock); port->cons.hvc = hvc_alloc(port->cons.vtermno, 0, &hv_ops, PAGE_SIZE); + spin_lock_irq(&pdrvdata_lock); if (IS_ERR(port->cons.hvc)) { ret = PTR_ERR(port->cons.hvc); dev_err(port->dev, "error %d allocating hvc for port\n", ret); port->cons.hvc = NULL; + port->cons.vtermno = pdrvdata.next_vtermno--; + spin_unlock_irq(&pdrvdata_lock); return ret; } - spin_lock_irq(&pdrvdata_lock); - pdrvdata.next_vtermno++; list_add_tail(&port->cons.list, &pdrvdata.consoles); spin_unlock_irq(&pdrvdata_lock); port->guest_connected = true;
Some modifications of vtermno were not done under the spinlock. Moved assignment from vtermno and increment of vtermno together, putting both under the spinlock. Revert vtermno on failure. Signed-off-by: Miche Baker-Harvey <miche@google.com> --- drivers/char/virtio_console.c | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-)