On (Fri) 29 Mar 2013 [08:57:44], Wanlong Gao wrote:
> On 03/28/2013 07:28 PM, Amit Shah wrote:
> > When multiple ovq operations are being performed (lots of open/close
> > operations on virtio_console fds), the __send_control_msg() function can
> > get confused without locking.
> >
> > A simple recipe to cause badness is:
> > * create a QEMU VM with two virtio-serial ports
> > * in the guest, do
> > while true;do echo abc >/dev/vport0p1;done
> > while true;do echo edf >/dev/vport0p2;done
> >
> > In one run, this caused a panic in __send_control_msg(). In another, I
> > got
> >
> > virtio_console virtio0: control-o:id 0 is not a head!
> >
> > This also results repeated messages similar to these on the host:
> >
> > qemu-kvm: virtio-serial-bus: Unexpected port id 478762112 for device
> > virtio-serial-bus.0
> > qemu-kvm: virtio-serial-bus: Unexpected port id 478762368 for device
> > virtio-serial-bus.0
> >
> > Reported-by: FuXiangChun <[email protected]>
> > Signed-off-by: Amit Shah <[email protected]>
> > ---
> > drivers/char/virtio_console.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> > index 7e9bc1d..410866c 100644
> > --- a/drivers/char/virtio_console.c
> > +++ b/drivers/char/virtio_console.c
> > @@ -150,6 +150,7 @@ struct ports_device {
> >
> > /* To protect the vq operations for the control channel */
> > spinlock_t c_ivq_lock;
> > + spinlock_t c_ovq_lock;
> >
> > /* The current config space is stored here */
> > struct virtio_console_config config;
> > @@ -569,11 +570,14 @@ static ssize_t __send_control_msg(struct ports_device
> > *portdev, u32 port_id,
> > vq = portdev->c_ovq;
> >
> > sg_init_one(sg, &cpkt, sizeof(cpkt));
> > +
> > + spin_lock_irq(&portdev->c_ovq_lock);
> > if (virtqueue_add_buf(vq, sg, 1, 0, &cpkt, GFP_ATOMIC) == 0) {
> > virtqueue_kick(vq);
> > while (!virtqueue_get_buf(vq, &len))
> > cpu_relax();
> > }
> > + spin_unlock_irq(&portdev->c_ovq_lock);
>
> While you lock the irq, why don't we need to save and restore the irq flags
> here?
_irq isn't actually needed; I'll send a v2 with just spin_lock/unlock.
Thanks,
Amit
_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
